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

[fix] When intro was triggered before the outro, that outro was canceled #7300

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

bfanger
Copy link
Contributor

@bfanger bfanger commented Feb 21, 2022

The issue

  1. When the block.i (intro) is called it registers a callback (via add_render_callback)
  2. Then the block.o (outro) is called and start the outro and adds a callback to detach on outroend
  3. The render callback from the intro is executed, starts the intro and cancels the outro animation

This causes components that should've been destroyed to stay on the page.

#6152
#6812

The fix in this PR

Inside the intro render callback it checks if it is still current and if it isn't (because an outro was triggered) it won't start the intro animation.

Copy link
Member

@tanhauhau tanhauhau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a test case to test this?

@bfanger
Copy link
Contributor Author

bfanger commented Apr 18, 2022

I've tried creating a unittest but I was not able to reproduce the timing defect inside the test environment.

The bug is clearly visible in the REPL

@bfanger bfanger force-pushed the fix/intro-outro-detach branch from 12fa1c4 to 35266a1 Compare July 16, 2022 11:02
@induratized
Copy link

Issue : REPL
Fix by using '|local modifier : REPL

@bfanger bfanger force-pushed the fix/intro-outro-detach branch from 35266a1 to 2bd3f26 Compare February 22, 2023 07:56
@vercel
Copy link

vercel bot commented Feb 22, 2023

@bfanger is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also unable to create a test for this specific case, probably because of the way the animation frame is patched, but to me this change looks good

@dummdidumm dummdidumm merged commit fb5f281 into sveltejs:master Mar 14, 2023
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 this pull request may close these issues.

4 participants