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

Tweened store updates with 0 duration leave store in corrupted state #4846

Closed
tivac opened this issue May 16, 2020 · 4 comments · Fixed by #4847
Closed

Tweened store updates with 0 duration leave store in corrupted state #4846

tivac opened this issue May 16, 2020 · 4 comments · Fixed by #4847

Comments

@tivac
Copy link
Contributor

tivac commented May 16, 2020

Describe the bug
When a tweened store is changed with a duration : 0 set it early-outs and immediately sets the value after #4766, but that PR missed an important detail. When the store is tweening, it needs to have the value variable updated each time it's set like it does for the early-out if value == null. The change #4766 doesn't do that, so whenever that codepath is taken value gets out of date and any later tweens are broken.

value === null path

if (value == null) {
store.set(value = new_value);
return Promise.resolve();
}

duration === 0 path

if (duration === 0) {
store.set(target_value);
return Promise.resolve();
}

Line 97 needs to be

store.set(value = target_value);

To Reproduce
https://svelte.dev/repl/b1c9c041de7048f2bd733f686d0d9482?version=3.22.2

Expected behavior
Store would immediately and synchronously get set to 10, then start tweening from 10 to 20.

Actual behavior

Store immediately set to 10, then starts tweening from 0 to 20.

Information about your Svelte project:

Severity
This breaks any tweened updates to the store after a value is set with duration : 0, so that seems pretty bad to me.

Additional context
I'm sorry I'm a bad person and introduced this bug. 😞

@tivac
Copy link
Contributor Author

tivac commented May 16, 2020

It also needs to cancel any tweening in progress, using the same previous_task.abort() logic that exists within the loop().

@pushkine
Copy link
Contributor

duplicate #4799

@tivac
Copy link
Contributor Author

tivac commented May 17, 2020

Whoops, thanks @pushkine!

@tivac tivac closed this as completed May 17, 2020
@Conduitry
Copy link
Member

Released your fix in 3.22.3 - https://svelte.dev/repl/b1c9c041de7048f2bd733f686d0d9482?version=3.22.3 - thanks!

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.

3 participants