From 52032bef8f2f900014b214d8b0dafa85d7fad72c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jun 2018 16:52:01 -0400 Subject: [PATCH 1/3] failing test for #1527 --- .../Span.html | 1 + .../_config.js | 26 +++++++++++++++++++ .../main.html | 14 ++++++++++ 3 files changed, 41 insertions(+) create mode 100644 test/runtime/samples/nested-transition-if-block-not-remounted/Span.html create mode 100644 test/runtime/samples/nested-transition-if-block-not-remounted/_config.js create mode 100644 test/runtime/samples/nested-transition-if-block-not-remounted/main.html diff --git a/test/runtime/samples/nested-transition-if-block-not-remounted/Span.html b/test/runtime/samples/nested-transition-if-block-not-remounted/Span.html new file mode 100644 index 0000000000..b16b370950 --- /dev/null +++ b/test/runtime/samples/nested-transition-if-block-not-remounted/Span.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/runtime/samples/nested-transition-if-block-not-remounted/_config.js b/test/runtime/samples/nested-transition-if-block-not-remounted/_config.js new file mode 100644 index 0000000000..128a518134 --- /dev/null +++ b/test/runtime/samples/nested-transition-if-block-not-remounted/_config.js @@ -0,0 +1,26 @@ +export default { + data: { + x: true, + value: 'one' + }, + + html: ` +
+ + x +
+ `, + + nestedTransitions: true, + + test(assert, component, target, window, raf) { + const div = target.querySelector('div'); + const { appendChild, insertBefore } = div; + + div.appendChild = div.insertBefore = () => { + throw new Error('DOM was mutated'); + }; + + component.set({ value: 'two' }); + }, +}; diff --git a/test/runtime/samples/nested-transition-if-block-not-remounted/main.html b/test/runtime/samples/nested-transition-if-block-not-remounted/main.html new file mode 100644 index 0000000000..ad8b2454a8 --- /dev/null +++ b/test/runtime/samples/nested-transition-if-block-not-remounted/main.html @@ -0,0 +1,14 @@ +
+ {#if x} + + x + {/if} +
+ + \ No newline at end of file From 1cd55d1f2bc1b933d299250c869eb6fa83127937 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jun 2018 16:52:27 -0400 Subject: [PATCH 2/3] avoid double intro --- src/compile/dom/Block.ts | 51 +++++++++++----------------------------- 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/src/compile/dom/Block.ts b/src/compile/dom/Block.ts index 6b753ed1aa..0b16d1502a 100644 --- a/src/compile/dom/Block.ts +++ b/src/compile/dom/Block.ts @@ -166,18 +166,11 @@ export default class Block { toString() { const { dev } = this.compiler.options; - let introing; - const hasIntros = !this.builders.intro.isEmpty(); - if (hasIntros) { - introing = this.getUniqueName('introing'); - this.addVariable(introing); - } - - let outroing; - const hasOutros = !this.builders.outro.isEmpty(); - if (hasOutros) { - outroing = this.alias('outroing'); - this.addVariable(outroing); + let current; + if (this.hasIntroMethod || this.hasOutroMethod) { + current = this.getUniqueName('current'); + this.addVariable(current); + this.builders.mount.addLine(`${current} = true;`); } if (this.autofocus) { @@ -275,46 +268,30 @@ export default class Block { } if (this.hasIntroMethod || this.hasOutroMethod) { - if (hasIntros) { + if (this.builders.mount.isEmpty() && this.builders.outro.isEmpty()) { + properties.addBlock(`i: @noop,`); + properties.addBlock(`o: @run,`); + } else { properties.addBlock(deindent` ${dev ? 'i: function intro' : 'i'}(#target, anchor) { - if (${introing}) return; - ${introing} = true; - ${hasOutros && `${outroing} = false;`} - + console.trace("intro", #component.constructor.name, ${current}); + if (${current}) return; ${this.builders.intro} - this.m(#target, anchor); }, `); - } else { - if (this.builders.mount.isEmpty()) { - properties.addBlock(`i: @noop,`); - } else { - properties.addBlock(deindent` - ${dev ? 'i: function intro' : 'i'}(#target, anchor) { - this.m(#target, anchor); - }, - `); - } - } - if (hasOutros) { properties.addBlock(deindent` ${dev ? 'o: function outro' : 'o'}(#outrocallback) { - if (${outroing}) return; - ${outroing} = true; - ${hasIntros && `${introing} = false;`} + console.trace("outro", #component.constructor.name, ${current}); + if (!${current}) return; + ${current} = false; ${this.outros > 1 && `#outrocallback = @callAfter(#outrocallback, ${this.outros});`} ${this.builders.outro} }, `); - } else { - properties.addBlock(deindent` - o: @run, - `); } } From 3b928b36a93fb98d90db2a2244db5f044905bbfb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jun 2018 17:23:35 -0400 Subject: [PATCH 3/3] fix the bugs --- src/compile/dom/Block.ts | 31 ++++++++++++++++++++----------- src/compile/nodes/Attribute.ts | 4 ++-- src/compile/nodes/Title.ts | 2 +- src/compile/nodes/shared/Tag.ts | 2 +- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/compile/dom/Block.ts b/src/compile/dom/Block.ts index 0b16d1502a..977486bd8d 100644 --- a/src/compile/dom/Block.ts +++ b/src/compile/dom/Block.ts @@ -142,6 +142,10 @@ export default class Block { } addVariable(name: string, init?: string) { + if (name[0] === '#') { + name = this.alias(name.slice(1)); + } + if (this.variables.has(name) && this.variables.get(name) !== init) { throw new Error( `Variable '${name}' already initialised with a different value` @@ -166,11 +170,16 @@ export default class Block { toString() { const { dev } = this.compiler.options; - let current; if (this.hasIntroMethod || this.hasOutroMethod) { - current = this.getUniqueName('current'); - this.addVariable(current); - this.builders.mount.addLine(`${current} = true;`); + this.addVariable('#current'); + + if (!this.builders.mount.isEmpty()) { + this.builders.mount.addLine(`#current = true;`); + } + + if (!this.builders.outro.isEmpty()) { + this.builders.outro.addLine(`#current = false;`); + } } if (this.autofocus) { @@ -268,24 +277,24 @@ export default class Block { } if (this.hasIntroMethod || this.hasOutroMethod) { - if (this.builders.mount.isEmpty() && this.builders.outro.isEmpty()) { + if (this.builders.mount.isEmpty()) { properties.addBlock(`i: @noop,`); - properties.addBlock(`o: @run,`); } else { properties.addBlock(deindent` ${dev ? 'i: function intro' : 'i'}(#target, anchor) { - console.trace("intro", #component.constructor.name, ${current}); - if (${current}) return; + if (#current) return; ${this.builders.intro} this.m(#target, anchor); }, `); + } + if (this.builders.outro.isEmpty()) { + properties.addBlock(`o: @run,`); + } else { properties.addBlock(deindent` ${dev ? 'o: function outro' : 'o'}(#outrocallback) { - console.trace("outro", #component.constructor.name, ${current}); - if (!${current}) return; - ${current} = false; + if (!#current) return; ${this.outros > 1 && `#outrocallback = @callAfter(#outrocallback, ${this.outros});`} diff --git a/src/compile/nodes/Attribute.ts b/src/compile/nodes/Attribute.ts index ac1cbeda6c..53996469e9 100644 --- a/src/compile/nodes/Attribute.ts +++ b/src/compile/nodes/Attribute.ts @@ -232,7 +232,7 @@ export default class Attribute extends Node { if (this.dependencies.size || isSelectValueAttribute) { const dependencies = Array.from(this.dependencies); const changedCheck = ( - ( block.hasOutros ? `#outroing || ` : '' ) + + (block.hasOutros ? `!#current || ` : '') + dependencies.map(dependency => `changed.${dependency}`).join(' || ') ); @@ -308,7 +308,7 @@ export default class Attribute extends Node { if (propDependencies.size) { const dependencies = Array.from(propDependencies); const condition = ( - (block.hasOutros ? `#outroing || ` : '') + + (block.hasOutros ? `!#current || ` : '') + dependencies.map(dependency => `changed.${dependency}`).join(' || ') ); diff --git a/src/compile/nodes/Title.ts b/src/compile/nodes/Title.ts index 8be46bd53b..080d122535 100644 --- a/src/compile/nodes/Title.ts +++ b/src/compile/nodes/Title.ts @@ -81,7 +81,7 @@ export default class Title extends Node { if (allDependencies.size) { const dependencies = Array.from(allDependencies); const changedCheck = ( - ( block.hasOutros ? `#outroing || ` : '' ) + + ( block.hasOutros ? `!#current || ` : '' ) + dependencies.map(dependency => `changed.${dependency}`).join(' || ') ); diff --git a/src/compile/nodes/shared/Tag.ts b/src/compile/nodes/shared/Tag.ts index 82b7123798..df17a1f9c0 100644 --- a/src/compile/nodes/shared/Tag.ts +++ b/src/compile/nodes/shared/Tag.ts @@ -35,7 +35,7 @@ export default class Tag extends Node { if (dependencies.size) { const changedCheck = ( - (block.hasOutros ? `#outroing || ` : '') + + (block.hasOutros ? `!#current || ` : '') + [...dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ') );