useChain does not work as expected for transitions

[martinkutter]

🐛 Bug Report

useChain does not chain animations correct, if Transition should animate after Spring.

To Reproduce

Spring first, than Transition -> does not work
See https://codesandbox.io/s/wizardly-leaf-b4v5g?file=/src/App.tsx

Transition first, than Spring -> does work
comment out line 38; comment in line 40

Expected behavior

Transition animation should run after Spring animation, not in sequence.

Environment

• react-spring v9.1.2
• react v17.0.1

[martinkutter]

What I have found out so far:
I think the issue is, that there are no controllers available for transitions in https://github.com/pmndrs/react-spring/blob/master/packages/core/src/hooks/useChain.ts#L15

[joshuaellis]

Firstly, I think you need to set an actual height on the inner divs the don't appear and it's confusing.
Secondly, and this is more so i understand, you've removed the data from the transition, but you're expecting the divs to stay there until the spring has changed background color and then the inner divs animate out?

[martinkutter]

Ups... :( I had forgotten to save the CSS file.
I think the Example is now much clearer.

https://codesandbox.io/s/wizardly-leaf-b4v5g?file=/src/App.tsx

https://user-images.githubusercontent.com/16589094/116391969-24844480-a820-11eb-84de-b9d48906838d.mp4
In the first part of the screen capture the animation starts parallel not chained. In the second part the chain starts with the transition and the animations runs chained as expected.

As you can see in the additional console logs, there is no controller for transition animations available from ref. So I think https://github.com/pmndrs/react-spring/blob/master/packages/core/src/hooks/useChain.ts#L15 can't work correct in this case.

[joshuaellis]

So the issue is the background color changes whilst the transitions are animating in. but actually, the background color should change then the divs animate in?

[martinkutter]

Yes correct!

[martinkutter]

I had the chance to take a closer look at the problem with the missing controller.

If I understand correct at this line the controller should be assigned to ref from props.

react-spring/packages/core/src/hooks/useTransition.tsx

Line 295 in 9568020

replaceRef(ctrl, payload.ref)

But payload.ref is not (and cannot be) defined. payload is initialized in

react-spring/packages/core/src/hooks/useTransition.tsx

Line 206 in 9568020

const payload: ControllerUpdate<UnknownProps> = {

defaultProps come from

react-spring/packages/core/src/hooks/useTransition.tsx

Line 168 in 9568020

const defaultProps = getDefaultProps<UseTransitionProps>(props)

But ref is not in DEFAULT_PROPS

react-spring/packages/core/src/helpers.ts

Lines 96 to 104 in 9568020

	export const DEFAULT_PROPS = [
	'config',
	'onProps',
	'onStart',
	'onChange',
	'onPause',
	'onResume',
	'onRest',
	] as const

[joshuaellis]

It sounds like you're gonna have a fix quicker than I, feel free to make a PR for this ⭐ although I will be looking for us to include tests to ensure refs that are passed are assigned and working.

[joshuaellis]

this has been released in 9.2.0-beta.1

[martinkutter]

Unfortunately my changes breaks some transitions completely if a ref is injected.
Compare 9.2.0-beta.0 and 9.2.0-beta.1 in https://codesandbox.io/s/flamboyant-violet-7nbv3?file=/src/App.tsx

I try to figure out why for some hours...
Now that a ref may exists there is something needed to adjust here:

react-spring/packages/core/src/hooks/useTransition.tsx

Lines 302 to 303 in 922f640

	// Postpone the update if an injected ref exists.
	ctrl[ctrl.ref ? 'update' : 'start'](payload)

Unfortunately I have no deeper insight here, maybe you can help?

[joshuaellis]

Okay I will revert your commit because we can't have a regression.

Regarding me looking at it, I will assign the ticket to myself to look at when I have time too.

[martinkutter]

Maybe a little support, it initially breaks with the SpringRef rework.

[martinkutter]

Hey @joshuaellis, I imagine you have a lot of work to do. Nevertheless, because my current project relies very heavily on chained animations, I would like to ask carefully if there is a forecast when you could look at this bug? Or maybe there is another contributor who could at least give me a hint where the problem should be solved? Thank you! 🙏

[joshuaellis]

Hi @martinkutter, unfortunately I can't tell you when i'll be able to work on this, if you're interested in enabling me to put more time into the project, feel free to sponsor me so I don't have to do as much client work. But it is on my radar for this weekend time permitting.

[joshuaellis]

Unfortunately my changes breaks some transitions completely if a ref is injected.
Compare 9.2.0-beta.0 and 9.2.0-beta.1 in https://codesandbox.io/s/flamboyant-violet-7nbv3?file=/src/App.tsx

I try to figure out why for some hours...
Now that a ref may exists there is something needed to adjust here:

react-spring/packages/core/src/hooks/useTransition.tsx

Lines 302 to 303 in 922f640

	// Postpone the update if an injected ref exists.
	ctrl[ctrl.ref ? 'update' : 'start'](payload)

Unfortunately I have no deeper insight here, maybe you can help?

In this example you're trying to pass a regular React Ref, which is no longer possible across the entire library. Was this the only issue? Or did I miss something. Because if it is, I think you did solve the issue...

[martinkutter]

Thanks for your time! Oh... that was something I changed while testing several versions. I updated the codesandbox and use SpringRef again. But as you can see, this simple transition works in 9.2.0-beta.0 but breaks in 9.2.0-beta.1.

By the way, sponsoring is a great idea. I will talk to my project owner next week.

[joshuaellis]

SO, I've had a look at it, yeah this line:

ctrl[ctrl.ref ? 'update' : 'start'](payload) 

is important, it's what keeps our API consistent, i.e. if you ever pass a ref, you must call ref.start to start the transition, here's a forked sandbox demonstrating this: https://codesandbox.io/s/goofy-allen-u4hml?file=/src/App.tsx

edit: see PR for reference. I've also updated a demo and when we go stable I will update the documentation.

[martinkutter]

Thank you very much for your time and your PR. While for me it is a little confusing that I have to call start() in this case, but not if I use useChain it's good to hear that it is only a docs and understanding issue. I will check if everything works fine in our project in the next days.

[joshuaellis]

Well useChain is designed to trigger animations, consistent with other hooks when you pass a ref, so imo it's better this way, i'll be releasing a beta once another PR is ready.

[joshuaellis]

This has been released on v9.2.0-beta.4