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

Sync updates shouldn't cancel pending async updates entirely #624

Closed
aleclarson opened this issue Apr 6, 2019 · 2 comments
Closed

Sync updates shouldn't cancel pending async updates entirely #624

aleclarson opened this issue Apr 6, 2019 · 2 comments
Milestone

Comments

@aleclarson
Copy link
Contributor

Currently, when the queue is flushed, any pending async updates are cancelled.
To ensure that to props are never lost, the controller merges them into its merged object, which is iterated in the diff method.

Merging has the consequence of immediately starting the pending async updates, which I believe is rarely what the user wants. For example, let's say useTransition is running enter animations on its items along with the trail prop (creating a staggered effect). When useTransition is then passed an update, the trail prop is effectively ignored and all enter animations begin at the same time.

I think we can do better.

One idea is to attach a timestamp to every flush. Then we never need to cancel anything on flush. Instead, when a pending async update is ready to diff, it checks the timestamp of each animation it wants to update. If the animation has a greater timestamp, skip updating it. By never cancelling on flush, we ensure that end values are never lost. By attaching a timestamp, we ensure that newer updates take precedence over async updates.

Feedback wanted. Thanks!

@drcmda
Copy link
Member

drcmda commented Apr 6, 2019

By never cancelling on flush, we ensure that end values are never lost.

I think that's super important. If timestamps can guarantee that values aren't lost, so that we stay deterministic, then i would say let's go for it!

Another way could be that updates check if there are outstanding queue chunks, merge their values to the next one, or if none exists, start a new diff/update cycle.

Would you want to create a small test for this? We could bombard it with complex usecases and try timetags out.

@aleclarson
Copy link
Contributor Author

Would you want to create a small test for this? We could bombard it with complex usecases and try timetags out.

I would appreciate if you did, and I'll get the timestamps implemented. :)

@aleclarson aleclarson changed the title Sync updates shouldn't affect pending async updates Sync updates shouldn't cancel pending async updates entirely Apr 10, 2019
@aleclarson aleclarson mentioned this issue Apr 10, 2019
5 tasks
@aleclarson aleclarson added this to the v9.0.0 milestone Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants