useTransition: Fix deleted items handing

[dminkovsky]

I am bumping up against this line https://github.com/react-spring/react-spring/blob/66087e18b64d6c6d5fd75f6d84e85ed59714769d/src/useTransition.js#L256. I have a list of items, and when consecutive items are removed from the list, this line of code causes all items that had their previous sibling removed to jump to the top of the list. For example:

This can get really jarring if lots of items are moved to the top.

This PR simplifies the behavior by sorting the resulting list relative to the previous list. References to left and right are removed. right was not being used already, and left is left is removed due to this PR.

After this PR:

[aleclarson]

Did you try v9 or the latest canary?

[dminkovsky]

Nope, forgot to check. Will do that right now, thanks!

Also, I think this patch might break added items.

[aleclarson]

The useTransition hook is completely rewritten in #809, but that's only available in the latest canary version for now. Let me know if you have any questions.

[dminkovsky]

Thanks. I've got the canary installed and can confirm it doesn't have this issue.

Some things I experienced during the upgrade:

• Interpolations are more strict:

I had to change

    from: {transform: 'translateX(100%)'},
    enter: {transform: 'translateX(0)'},
    leave: {transform: 'translateX(100%)'},

to

    from: {transform: 'translateX(100%)'},
    enter: {transform: 'translateX(0%)'},  <-- percent sign
    leave: {transform: 'translateX(100%)'},

or else the animation wouldn't work.

• This pattern:

    const transition = useTransition(
        isOpen, // a bool
        config,
    );

    {transition((props, item) => {
          return item ? (
              <Overlay style={props} />
          ) : null;
    })}

No longer works as before. The component doesn't unmount when null. But that makes sense given the new API?

Thank you!

[aleclarson]

No longer works as before. The component doesn't unmount when null. But that makes sense given the new API?

Not sure what you mean. Make a minimal sandbox?

[dminkovsky]

Sorry, that was confusing. Here is

• a CS with 8.27: https://codesandbox.io/s/affectionate-https-wq6km.
• a CS with the latest canary: https://codesandbox.io/s/solitary-waterfall-b0l1i

In 8.27, .Overlay is removed from the DOM after animation.

In the canary, the animation works but .Overlay is not removed from the DOM.

[aleclarson]

@dminkovsky By default, useTransition won't dismount anything unless you re-render your component in some other way. To get around this, set expires: 0 to make useTransition force a re-render whenever a leave transition finishes. This default behavior may change before v9 is released.

[dminkovsky]

Ah yes, that's it. Now I see it explained over in #809 too. Thanks a lot!

[aleclarson]

By default, dismounting is postponed until (1) next render, or (2) all transitions are resting.

The sentence above is from the OP in #809. It seems that your example should fit the 2nd condition, so I think this is a bug. I might have looked into this before, but I don't remember. I'll definitely look into it again before v9 is released. There's a good chance your example won't need expires: 0 in the future.

[dminkovsky]

That sounds great. However it shakes out, much appreciated.

[dminkovsky]

I think there is an issue with useSpring in this latest canary wherein initial values returned from fn in useSpring(fn) are re-applied after subsequent animations come to rest. Has this been reported?

EDIT not able to replicate in a sandbox.

[dminkovsky]

Not sure why I'm not replicating in code sandbox but the issue appears to be here:

https://github.com/react-spring/react-spring/blob/7e75a6758ffb78f5c149deb1c14317ecaa6bd68d/packages/core/src/useSpring.ts#L66.

I have a call like useSpring(fn), and when it gets to that line, useSprings is called with deps undefined. Subsequently, at

https://github.com/react-spring/react-spring/blob/7e75a6758ffb78f5c149deb1c14317ecaa6bd68d/packages/core/src/useSprings.ts#L81-L82

arguments.length is 3 so deps remains undefined and the memo constantly calculates. Updating the original useSpring call to include the empty deps array fixes this problem.

[aleclarson]

@dminkovsky Whoops, thanks for pointing that out! Should be fixed in the latest canary. 👍

[dminkovsky]

Awesome, thank you.

Is Spectrum the best way to keep up with the canaries? Should I open new issues here for canary-related things?

[aleclarson]

Is Spectrum the best way to keep up with the canaries?

Yes

Should I open new issues here for canary-related things?

Sure, or you could comment in #808 for the canaries that contain .808. in their version.