svelte: Store updates break transitions and components

Describe the bug Components with an out:[transition] and a store subscription don’t get destroyed if the store is updated while out:[transition] is in progress.

To Reproduce https://svelte.dev/repl/00d38994d0364ea3a3ae629937142ea6?version=3.12.1

  1. Create a store
  2. Subscribe to it in component A
  3. Create an element with out:fade in component A
  4. Destroy component A and trigger a store update before the animation has completed Result: component A never got destroyed.

Expected behavior Components should be destroyed

Severity Very. It breaks apps without a single warning or error. This means that the more complex/nested your app is, the harder it is to troubleshoot.

Additional context Any app that relies on URL params to update the store is highly susceptible to this bug. I ran into it with Sapper as well.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 15
  • Comments: 15 (6 by maintainers)

Most upvoted comments

Fixed in 3.21.0 - https://svelte.dev/repl/00d38994d0364ea3a3ae629937142ea6?version=3.21.0

not really fixed, if the duration of transition for out:send is more than 100ms it will likely to stuck again, if duration is 600ms or 1000ms… it will surely stuck.

I can confirm this is still an issue as described, except the threshold I found was 240ms. Anything above that and this bug occurs; 240ms or below, it works as expected.

Here you can see an example REPL: https://svelte.dev/repl/c2ec0888fcf044f78b0ab05c446d5787?version=3.24.1

I am having the same issue in a rather large scale app. When I add out-transitions, parts of the app are not unmounted anymore.

Today I finally took some time (well, more or less the whole day…) to debug this.

Based on the excellent example above I built a modified, further simplified version (here as a REPL - based on the initial one above). No store and only one child.

Findings:

  • The issue is definitely not just related to stores. This is more general. The issue can be replicated even when a component only updates its internal state
  • It’s definitely serious since it virtually breaks the app (or puts it in an inconsistent state)
  • This issue seems to be the same as #3202 (and #3410 might also be related to this)
  • I agree with @halfnelson about what is causing the issue

But: I am not sure if it requires to add a global/local classifier. When running is set to false, the Set “outroing” still contains two entries. Could this be used to prevent cancelling the overall outro, or pick up at the remaining entries?

I am most probably missing something, but this worked for me (both the the simple example as well as the one from above. Also tried this out in a large scale app):

end(reset) {
                if (reset && config.tick) {
                    config.tick(1, 0);
                }
                if (running) {
                    if (animation_name)
                        delete_rule(node, animation_name);
                    running = (outroing.size !== 0);  // <-- changed from false
                }
            }

Penny for your thoughts…


Edit: As I expected I missed something. After a good night of sleep I had a second look at this and realized that outroing obviously also contains all other outros… Will try to find some more time for this later this week.

I have done some digging and discovered the following during a trace: The outros are all scheduled as usual then an update kicks in and the if block is transitioned back in

p: function update(ctx, [dirty]) {
    			if (/*show*/ ctx[0]) {  //<-- show is true 
    				if (!if_block) {   
    					if_block = create_if_block(ctx);
    					if_block.c();
    					transition_in(if_block, 1);
    					if_block.m(t0.parentNode, t0);
    				} else {
    					transition_in(if_block, 1);  // <-- so this runs, 
    				}
    			} else if (if_block) {
.....
    			}

This goes through the motions and ends up finding the header which is transitioning out and cancels that outro (since it is coming back in now)

	i: function intro(local) {
    			if (current) return;
    			if (h3_outro) h3_outro.end(1);  <-- The header outro which is still running is ended
    			current = true;
    		},

the end sets running= false for the entire outro group

       end(reset) {
                if (reset && config.tick) {
                    config.tick(1, 0);
                }
                if (running) {
                    if (animation_name)
                        delete_rule(node, animation_name);
                    running = false; // <-- no outro callbacks and destroy calls for h3 && if block && page component
                }
            }

So I think the root cause is that the outro is being cancelled and it shouldn’t be. I think this is a case of a local intro canceling a global outro.

We can see that the intro call doesn’t have enough information, it can’t tell if the outro was created for a local transition or not.

Do we need to store the type (local|global) as well as the transition?

I can confirm this is still an issue as described, except the threshold I found was 240ms. Anything above that and this bug occurs; 240ms or below, it works as expected

This explains my current struggles. Does anyone know of a temporary fix for this? (how to forcefully remove after transition). I tried triggering a {#key} on:outroend but it doesn’t always work.

@Conduitry Could we re-open this issue?

This also affects Sapper, where any transition on the page whose elements are provided by the contents of a store (such as an each) cause navigation to break (and then the app to break entirely).

@jhwheeler there are a few workarounds. They’re all duct tape solutions though.

  1. Split up the store, so the update doesn’t affect the store on the exit-page.
  2. Import the store further down the tree and pass it to components through props.
  3. Delay the store update on the enter-page.
  4. Use the local directive when possible (out:fade|local)

EDIT: Added no. 4