sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.78k stars 4.23k forks source link

Nested transitions: dynamic components in EachBlock prevent parent from being removed #1512

Closed jacwright closed 6 years ago

jacwright commented 6 years ago

When nested transitions is turned on and you have dynamic components in an EachBlock the parent block is never removed. See: https://svelte.technology/repl?version=2.7.0&gist=41168eab5f3ed4c781ab1453eb6900f4

stalkerg commented 6 years ago

Sorry, I was wrong, I have totally same case.

stalkerg commented 6 years ago

I found why it happens - if outro has already passed:

o: function outro(outrocallback) {
    if (outroing) return;
    outroing = true;
    introing = false;
    if (!div_transition) div_transition = wrapTransition(component, div, fade, {}, false);
    div_transition.run(0, () => {
        outrocallback();
        div_transition = null;
    });
},

and upper:

o: function outro(outrocallback) {
    if (outroing) return;
    outroing = true;

    if (if_block_7) if_block_7.o(outrocallback);
    else outrocallback();
},

in my case, outroing have already true, because Modal window hide by itself and after emit the event to they have already hide and parent component can remove it. Looks like if "outroing" have already true we can just remove this block.

stalkerg commented 6 years ago

Ohhh I found extra problems even I have no any "transition" inside.

stalkerg commented 6 years ago

Fast hack way obviously didn't help me:

diff --git a/src/compile/dom/Block.ts b/src/compile/dom/Block.ts
index 6b753ed1..1313b896 100644
--- a/src/compile/dom/Block.ts
+++ b/src/compile/dom/Block.ts
@@ -302,13 +302,14 @@ export default class Block {
                        if (hasOutros) {
                                properties.addBlock(deindent`
                                        ${dev ? 'o: function outro' : 'o'}(#outrocallback) {
-                                               if (${outroing}) return;
+                                               if (${outroing}) return false;
                                                ${outroing} = true;
                                                ${hasIntros && `${introing} = false;`}

                                                ${this.outros > 1 && `#outrocallback = @callAfter(#outrocallback, ${this.outros});`}

                                                ${this.builders.outro}
+                                               return true;
                                        },
                                `);
                        } else {
diff --git a/src/compile/nodes/IfBlock.ts b/src/compile/nodes/IfBlock.ts
index ee5cb41a..b00a7cc6 100644
--- a/src/compile/nodes/IfBlock.ts
+++ b/src/compile/nodes/IfBlock.ts
@@ -148,8 +148,7 @@ export default class IfBlock extends Node {

                        if (hasOutros && this.compiler.options.nestedTransitions) {
                                block.builders.outro.addBlock(deindent`
-                                       if (${name}) ${name}.o(#outrocallback);
-                                       else #outrocallback();
+                                       if (!(${name} && ${name}.o(#outrocallback))) #outrocallback();
                                `);
                        }
                }
Rich-Harris commented 6 years ago

Came here while looking into #1660. It seems that this specific issue was fixed a while back, so I'll close this. Thanks