Store updates break transitions and components

[jakobrosenberg]

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.

[jhwheeler]

Encountering the same bug. All components in the first page are not destroyed when transitioning to the next page, resulting in both pages' components rendering on top of each other.

If I change the <div transition:fade> to <div in:fade>, it works fine. But then of course there are no out fades when that component changes :/

[jakobrosenberg]

@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

[jhwheeler]

@jakobrosenberg Thanks for the tips! What I've done for now is to replace transition:fade with in:fade, which is obviously not ideal, but at least nothing breaks. I'll also try out some of your workarounds; although as you point out, they are duct-tape solutions and it would be great if we can get the root cause fixed.

[antony]

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).

[halfnelson]

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?

[stalkerg]

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

Probably yes, but it provides some API changes. Actually, I have a similar problem recently... now I know why.

[chuckrector]

I hit this recently as well in Sapper. It's pernicious when you've built up a complex app and then decide you'd like to add a little flair, later to find that pages have begun intermittently stacking atop one another.

I spent the last few hours root causing this myself and had whittled down a minimal Sapper repro: https://github.com/chuckrector/destructionanigans I discovered that this had already been filed as a bug here only after digging deep enough to understand what I was dealing with. 😅

I would add that this only affects object stores -- primitive stores seem to be immune!

[christianheine]

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 Transitions break basic functionality #3202 (and Transition prevents {#if}{:else}{/if} from being exclusive #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.

[cdock1029]

Probably related, #await blocks and transitions have similar issues

#1591

#2629

[Conduitry]

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

[thienpow]

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.

[jhwheeler]

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

[muxcmux]

I'm still hitting this, did anyone manage to work out how to fix it?

[braebo]

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?

[jaskaransarkaria]

Agreed this needs to be re-opened. Although my particular scenario didn't involve any store updates, the on:outroended would not fired if I navigated away from the page and then returned. The our transitions generally were creating a lot of random bugs in my app (showing components on wrong pages, components showing where they were not supposed to and components not behaving correctly). The threshold of 240ms mentioned by @jhwheeler made no difference for me.

My solution was to remove the out transitions and my bugs disappeared.