Fix ordering of deleted items

[aocenas]

Fixes: #461

The logic recreating the ordering of the items requires the deleted items to have consistent ordering. This seems to be already fixed in Transition.js.

Btw there seems to be other differences between Transition.js and useTransition.js even though they probably should be deduplicated.

[drcmda]

@aocenas super! did you test it against the examples? is all looking good?

[aocenas]

@drcmda Ah did not know about the examples, at least not that they are set up for testing. But yeah I tested it just now, they seem to be ok as far as I can tell. I also added a test example for this particular usecase pmndrs/react-spring-examples#1.

[drcmda]

Sorry for the long wait! I'll make a patch release

[drcmda]

@aocenas oh no, i saw there's still a bug hidden somewhere. The notifications demo breaks. That demo was broken before, but the order of the elements was always correct. Could you have another look?

[drcmda]

Had to revert for now, could you open another PR?

[aocenas]

@drcmda Yeah I see it now, weird. Sorry for that. I will take a look and try to fix it.

[aleclarson]

Possibly related: In this example, click "Banana" at the end to restart from the beginning. But before you do that, set a conditional breakpoint on this line with out.length > 3 as the expression. Notice how the deleted.forEach loop is adding in items when it shouldn't be.

edit: Nevermind, that's expected behavior in non-unique mode.

[aocenas]

@aleclarson thanks for the tip but honestly not sure if having duplicate items is ok or not in this case. Seems like items can have duplicates at some point. Would have to look more into the code.

@drcmda One sure thing is that the order restoration code https://github.com/react-spring/react-spring/blob/master/src/useTransition.js#L253 does not work very well. It works only for specific ordering of items and deletions. Only thing that comes to my mind as a fix would be to change it to something like this:

while (deleted.length) {
  d = deleted.pop()
  inserted = false
  out.forEach((o) => {
    // try find a sibling in current out array
    if (o.key == d.left || o.key == d.right) {
      // we found sibling so push into 'out' and break from the forEach
      inserted = true
      return
    }
  })
  if (!inserted) {
    // we did not find where it should be inserted, probably sibling is in deleted array, so put it back on stack and try later 
    deleted.push(d)
  }
}

I would hope this would not get into infinite loop. It shouldn't if items does not disappear and if they do there can be safety hatch that if there is full traversal of deleted and nothing was inserted just bail. This seems a bit lot of looping around but I am not seeing other quicker solution, apart from some heavier refactoring of how items are tracked.

If you do not have better idea I will try to implement this later.

[drcmda]

@aocenas looks good to me. item tracking is such an evil thing, when i started i had no idea how complex this was going to be with all the permutations going on. Maybe looping isn't optimal, but it also won't be the bottleneck, since all this stuff is being done before the animation starts.