Skip to content
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

[useSpring] onStart callback triggered on each frame #335

Closed
dbismut opened this issue Nov 23, 2018 · 7 comments
Closed

[useSpring] onStart callback triggered on each frame #335

dbismut opened this issue Nov 23, 2018 · 7 comments
Labels
kind: bug Something isn't working
Milestone

Comments

@dbismut
Copy link
Contributor

dbismut commented Nov 23, 2018

Hey! Thanks for this fantastic library.
I'm trying the useSpring hook which I understand is experimental, and maybe this is by design or maybe this is me not using useCallback or misusing hooks, but I find onStart being triggered on each frame.

Here is the isolated codesandbox showing the issue: https://codesandbox.io/s/oj9y6j6z9q
As you can see, dragging the box triggers a console.log at each frame.
If you uncomment the commented onStart function which increments a state counter, the onStart function is run continuously...

@ruggedy
Copy link
Contributor

ruggedy commented Nov 24, 2018

the problem is useSpring updates the internal animation controller on every rerender(i.e every time you call setState). there should be some sore of diffing of props, i.e if the props passed down are the same, then skip update.

@ruggedy
Copy link
Contributor

ruggedy commented Nov 24, 2018

a partial working example here. the problem now is if you pass a value that is an object or array, that is created in the params function i.e useSpring({x: 0, from: {x:0}), then the reference changes everytime which re-triggers the update. @drcmda let me know what you think about this?

@drcmda
Copy link
Member

drcmda commented Nov 24, 2018

That's interesting. onStart in hooks, i have to think about it. It's true that if you update, it theoretically "starts" - whatever this means, this is where the lines between declarative and imperative get blurry.

@dbismut
Copy link
Contributor Author

dbismut commented Nov 25, 2018

@ruggedy it stopped running onStart continuously when updating state from it (which is already a great thing!), but it still renders on every render generated by useGesture — which as you said in your first comment is expected since useSpring updates on every re-render.

So I tried using the transient mode in useGesture and update the spring without triggering a render: https://codesandbox.io/s/1rnp87kp1j

In this case, the component is not re-rendered on drag, so I wouldn't expect useSpring to be updated, but still, the onStart is run at every dragged frame as you can observe from the logs.

I'm sure there is a logic explanation for all this, and maybe we're reaching the limits of hooks there I don't know 😅

@ruggedy
Copy link
Contributor

ruggedy commented Nov 25, 2018

@dbismut yh it is actually working correctly, because now you are updating the internal animation controller everytime useGesture calls onAction, which technically starts a new animation everytime.

just a suggestion, as I havent used react-with-gesture yet, but maybe instead of listening to mousedown event, you can listen to the mouseup event, and animate from that position back to zero.

this is what I assume the code might look like.

onAction: ({ up, xDelta, xVelocity }) => {
      up && set({ from: { x: xDelta}, x: 0 })
}

just a wild guess.

edit: just checked and realised that you were using react spring to set the translate based on the mousemove xDelta. so the above will not work. but I will keep looking into this and let you know once I have something.

@aleclarson aleclarson added the kind: bug Something isn't working label Apr 1, 2019
@aleclarson aleclarson mentioned this issue Apr 10, 2019
5 tasks
@aleclarson
Copy link
Contributor

This is fixed in v9, which you can now try: yarn add react-spring@next

Let us know how it goes: #642

@aleclarson aleclarson added this to the v9.0.0 milestone Apr 23, 2019
@dbismut
Copy link
Contributor Author

dbismut commented Apr 23, 2019

Yes this does work, thanks @aleclarson!

@dbismut dbismut closed this as completed Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants