Skip to content

fix: ordering of deleted items #626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions src/renderprops/Transition.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'
import Spring from './Spring'
import Keyframes from './Keyframes'
import { callProp, toArray, interpolateTo } from './shared/helpers'
import { reconcileDeleted } from '../shared/helpers'

let guid = 0
let get = props => {
Expand Down Expand Up @@ -160,20 +161,7 @@ export default class Transition extends React.PureComponent {

// This tries to restore order for deleted items by finding their last known siblings
let out = keys.map(key => current[key])
deleted.forEach(({ left, right, ...transition }) => {
let pos
// Was it the element on the left, if yes, move there ...
if ((pos = out.findIndex(t => t.originalKey === left)) !== -1) pos += 1
// Or how about the element on the right ...
if (pos === -1) pos = out.findIndex(t => t.originalKey === right)
// Maybe we'll find it in the list of deleted items
if (pos === -1) pos = deleted.findIndex(t => t.originalKey === left)
// Checking right side as well
if (pos === -1) pos = deleted.findIndex(t => t.originalKey === right)
// And if nothing else helps, move it to the start ¯\_(ツ)_/¯
pos = Math.max(0, pos)
out = [...out.slice(0, pos), transition, ...out.slice(pos)]
})
out = reconcileDeleted(deleted, out)

return {
first: first && added.length === 0,
Expand Down
56 changes: 55 additions & 1 deletion src/shared/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { interpolateTo } from './helpers'
import { interpolateTo, reconcileDeleted } from './helpers'

describe('helpers', () => {
it('interpolateTo', () => {
Expand Down Expand Up @@ -37,4 +37,58 @@ describe('helpers', () => {
...excludeProps,
})
})

describe('reconcileDeleted', () => {
it('should handle simple cases', () => {
expect(reconcileWrapper('1:2:', '1')).toEqual([1, 2])
expect(reconcileWrapper(':2:1', '1')).toEqual([2, 1])
})

it('should handle multiple items', () => {
expect(reconcileWrapper('1:4:|4:5:', '1|2|3')).toEqual([1, 4, 5, 2, 3])
expect(reconcileWrapper('1:4:|3:5:', '1|2|3')).toEqual([1, 4, 2, 3, 5])
expect(reconcileWrapper('1:4:|4:5:|5:6:|:7:1', '1')).toEqual([
7,
1,
4,
5,
6,
])
})

it('should handle weird ordering of items', () => {
expect(reconcileWrapper('4:5:|1:4:', '1|2|3')).toEqual([1, 4, 5, 2, 3])
expect(reconcileWrapper('1:4:|:5:4', '1|2|3')).toEqual([1, 5, 4, 2, 3])
})

it('should handle interupted chain', () => {
expect(reconcileWrapper('9:4:|4:5:', '1|2|3')).toEqual([4, 5, 1, 2, 3])
})

it('should handle empty arrays', () => {
expect(reconcileWrapper('1:4:|4:5:', '')).toEqual([4, 5])
expect(reconcileWrapper('', '1|2')).toEqual([1, 2])
expect(reconcileWrapper('', '')).toEqual([])
})
})
})

const reconcileWrapper = (
deletedString: string,
outString: string
): number[] => {
return reconcileDeleted(makeItems(deletedString), makeItems(outString)).map(
i => i.originalKey
)
}

const makeItems = (input: string): any[] => {
if (input === '') return []
return input.split('|').map(item => {
let [left, originalKey, right] = item.split(':').map(n => parseInt(n, 10))
if (originalKey === undefined) {
return { originalKey: left }
}
return { left, originalKey, right }
})
}
63 changes: 63 additions & 0 deletions src/shared/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,66 @@ export function handleRef<T>(ref: T, forward: Ref<T>) {
}
return ref
}

/**
* This tries to put deleted items back into out list in correct order. Deleted
* items need to have a left and right property with id of their sibling which
* is used to find the correct placement.
* @param deleted
* @param out
*/
export function reconcileDeleted(
deleted: { left?: number; right?: number }[],
out: { originalKey: number }[]
): any[] {
// Copy as we will be mutating the arrays
deleted = [...deleted]
let result: any[] = [...out]

// Keep track of how many times we were not able to insert an item
let failedTries = 0

// Either try to insert all deleted items or bail if we went through whole
// list and did not insert single item. Bailing means the chain was
// interrupted somewhere and we cannot recreate the ordering.
while (deleted.length && failedTries < deleted.length) {
const d = deleted.shift()!
let indexToInsert = null

result.forEach((item, index) => {
// try find a sibling in out array
if (item.originalKey == d.left) {
indexToInsert = index + 1
return
}

if (item.originalKey == d.right) {
indexToInsert = index
return
}
})

if (indexToInsert === null) {
// we did not find where it should be inserted, probably the sibling is
// in deleted array and we did not insert it yet so put it back on stack
// and try later
deleted.push(d)
failedTries += 1
} else {
result.splice(Math.max(indexToInsert, 0), 0, d)
indexToInsert = null
failedTries = 0
}
}

// We were not able to recreate the ordering just put them in the beginning.
// We assume deleted item are already ordered properly. There are some
// (not sure if bugs or not) cases where we get here, for example items without
// siblings have left set to their own key so if items are added one by one
// they won't be linked
if (deleted.length) {
result = [...deleted, ...result]
}

return result
}
22 changes: 9 additions & 13 deletions src/useTransition.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import {
useCallback,
} from 'react'
import Ctrl from './animated/Controller'
import { is, toArray, callProp, useForceUpdate } from './shared/helpers'
import {
is,
toArray,
callProp,
useForceUpdate,
reconcileDeleted,
} from './shared/helpers'
import { requestFrame } from './animated/Globals'

/** API
Expand Down Expand Up @@ -213,7 +219,7 @@ function diffItems({ first, prevProps, ...state }, props) {
const keyIndex = _keys.indexOf(key)
const item = _items[keyIndex]
const slot = LEAVE
deleted.unshift({
deleted.push({
...current[key],
slot,
destroyed: true,
Expand Down Expand Up @@ -246,17 +252,7 @@ function diffItems({ first, prevProps, ...state }, props) {
}
}
let out = keys.map(key => current[key])

// This tries to restore order for deleted items by finding their last known siblings
// only using the left sibling to keep order placement consistent for all deleted items
deleted.forEach(({ left, right, ...item }) => {
let pos
// Was it the element on the left, if yes, move there ...
if ((pos = out.findIndex(t => t.originalKey === left)) !== -1) pos += 1
// And if nothing else helps, move it to the start ¯\_(ツ)_/¯
pos = Math.max(0, pos)
out = [...out.slice(0, pos), item, ...out.slice(pos)]
})
out = reconcileDeleted(deleted, out)

return {
...state,
Expand Down