Canary 808.14 - spring and decay don't play well together when spring has a velocity set in config

[dbismut]

🐛 Bug Report

When a spring take over a decay, the animated value jumps erratically for a short time.

To Reproduce

The sandbox allows you to drag the viewport and "fake scroll" its content. When the mouse is down, the content should scroll along your pointer. When you release the mouse, the content decays.

If you interrupt the decay movement, here's a weird jump as shown in the following gif.

Expected behavior

See v8 sandbox.

Link to repro

• using react-spring v8: https://codesandbox.io/s/decay-v8-zduqt
• using react-spring v9: https://codesandbox.io/s/decay-v9-mtkwn

Environment

• react-spring v9.0.0-canary.808.14
• react v16.9.0

[aleclarson]

Don't know when I'll get to this. I tried writing a test in SpringValue.test.ts that reproduces the bug, but I didn't figure it out in time. I haven't looked closely at your sandbox code yet, so the test is probably missing an important part that triggers the bug.

I created the i887 branch with my test. Feel free to try reproducing the bug with it. If you figure it out, you can push to that branch and open a PR targeting the feat/spring-class branch.

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.test.ts#L55-L76

[dbismut]

@aleclarson I think I've somewhat narrowed down the bug: it has to do with the velocity of the spring.

// before
if(down) set({ y: my, config: { decay: false } })
else set({ y: my, config: { decay: true, velocity: vy } })

// after
if(down) set({ y: my, config: { decay: false, velocity: 0 } }) // <-- add velocity: 0
else set({ y: my, config: { decay: true, velocity: vy } })

This solves my use case but I believe it still remains a bug since the jump shouldn't happen even with a velocity set in config.

As a side note, I think that this is a bit counterintuitive having to reset the last config but we can discuss this separately.

[dbismut]

@aleclarson sorry for the previous message I deleted. This time I think I've isolated the issue.

Here is the code I'm using to trigger the bug:

const [{ y }, set] = useSpring(() => ({ y: 0 }))

const bind = useDrag(
  ({ down, movement: [, my], vxvy: [, vy] }) => {
      if (down) set({ y: my, config: { decay: false } })
      else  set({ to: my, config: { decay: true, velocity: vy } }) 
    },
  { initial: () => [0, y.get()] }
)

What happens

So what the code does is while the mouse is down follow it with a spring, then when it's released, decay with the last velocity of the gesture. The bug occurs when you interrupt the decay gesture by pressing the mouse. From the capture below, it seems like the spring animates with an erroneous from position.

Looking at the code

Looking at SpringValue.ts it looks like from values are set in the _reset() function:

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.ts#L779

This function is called from the _merge() function unless the started variable is false.

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.ts#L670-L676

When a spring gets interrupted, it seems like started is true and the _reset() function gets called, therefore from is correctly set to the last known position. But when decay is interrupted, started is false.

Here is how started gets calculated:

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.ts#L601-L610

My observation is that things happen L604: changed is indeed true, but with decay value and to are equal (which shouldn't matter since as explained below, decay animation doesn't rely on the to value).

Then there is the second check occurring on L609 where started is set to true if lastVelocity is different from config.velocity. Well, lastVelocity is defined L531:

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.ts#L531

So there's hardly a chance both can be different. I'm not sure what should be the right check, but I think this is where the problem is.

---

Side observation about decay

Decay shouldn't even need a to argument since its ending position only depends on where it starts (from) and initial velocity.

Also there is this line in the decay loop which I'm not sure of the effect since to doesn't seem to be used anywhere else:

https://github.com/react-spring/react-spring/blob/c26c8c03e87bf0b06083ed3998a6b5c44c169026/packages/core/src/SpringValue.ts#L185

[aleclarson]

I think the goal value of your spring animation is missing an offset based off the y position when the drag starts.

Decay shouldn't even need a to argument since its ending position only depends on where it starts (from) and initial velocity.

Yup, this one was on my radar. It should be fixed in the next canary (releasing today after I verify this issue is not react-spring's fault). I think 10e5e93 is what fixed it.

[aleclarson]

Oh nevermind my first sentence. Just noticed the initial array. 😂

edit: Hmm, in my tests, it seems like the initial array doesn't get updated. Is that right? Seems like it should be updated when down turns from false to true.

[dbismut]

@aleclarson hey Alec thanks for taking the time. Actually the initial array gets updated every time the gesture starts.

Again, this works with v8, I think you can reproduce the issue by following these steps:

• start a decay animation
• interrupt it with a spring animation.

The spring animation should start from the current position of the value (so the current value of decay should be the implicit from prop). My observation is that the spring animation will keep the same from value as the initial decay animation.

[aleclarson]

The from prop isn't used unless reset: true is passed or it's the first ever animation. I'll take another look soon. I must have an older version of react-use-gesture installed on accident.

[dbismut]

I'll have another look as well, I just tried to reproduce the issue and can't do it steadily. But I still think my observation is valid in the Looking at the code section especially line 609!

[aleclarson]

Be sure to pull the feat/spring-class branch before testing, so you have my latest changes. 👍

[dbismut]

Wow, this looks like it's also fixed! Updated sandbox: https://codesandbox.io/s/decay-v9-mtkwn

[aleclarson]

Nice, I didn't even try to fix this one :)

edit: I mean.. I did with 874897d, but I don't think that worked at the time.. 😋

[sahil87]

I am facing this problem in the latest version (9.1.0).

This probably got reintroduced here.

According to the discussion above decay animations shouldn't worry about to. But here, at the end of the animation, node.setValue gets sent anim.to. The result of this a "jump" at the end of the decay animation.

[joshuaellis]

@sahil87 would you be able to open a new issue please and follow the template? You could reference this issue too, that way it'll be open and on our radar. Please ensure it's not a duplicate of #1026 though ⭐