From 007d952e05c544125ac495f5739053059cdfbe4e Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 29 Apr 2022 15:41:36 +0800 Subject: [PATCH 01/10] fix: destroy non-fragment element such as empty components --- src/runtime/internal/transitions.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/internal/transitions.ts b/src/runtime/internal/transitions.ts index a0134668eff1..457d234087dc 100644 --- a/src/runtime/internal/transitions.ts +++ b/src/runtime/internal/transitions.ts @@ -79,6 +79,9 @@ export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, ca }); block.o(local); + // fix: destroy non-fragment element such as empty components. + } else if (typeof callback === 'function') { + callback(); } } @@ -143,7 +146,7 @@ export function create_in_transition(node: Element & ElementCSSInlineStyle, fn: return { start() { if (started) return; - + started = true; delete_rule(node); From 35da0f5a6bd1163dfa67047487576cc408d80d2d Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 29 Apr 2022 16:08:38 +0800 Subject: [PATCH 02/10] fix: fragment property of Empty Component is set as true in dev mode, inconsistent with production mode --- src/compiler/compile/render_dom/Block.ts | 5 ++++- src/compiler/compile/render_dom/index.ts | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/compiler/compile/render_dom/Block.ts b/src/compiler/compile/render_dom/Block.ts index 34c477480463..2d930c0a11d9 100644 --- a/src/compiler/compile/render_dom/Block.ts +++ b/src/compiler/compile/render_dom/Block.ts @@ -428,13 +428,16 @@ export default class Block { } has_content(): boolean { + // exclude "ThrowStatment" type node + // as in dev mode, 'ThrowStatement' type node will always be added, which will make has_content() always return true in dev mode + const validClaims = this.chunks.claim.filter(i => Array.isArray(i) && i[0] && i[0].type !== 'ThrowStatement'); return !!this.first || this.event_listeners.length > 0 || this.chunks.intro.length > 0 || this.chunks.outro.length > 0 || this.chunks.create.length > 0 || this.chunks.hydrate.length > 0 || - this.chunks.claim.length > 0 || + validClaims.length > 0 || this.chunks.mount.length > 0 || this.chunks.update.length > 0 || this.chunks.destroy.length > 0 || diff --git a/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 9d9699bdbf6c..6c1590d4d161 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.ts @@ -333,8 +333,10 @@ export default function dom( // $$props arg is still needed for unknown prop check args.push(x`$$props`); } - - const has_create_fragment = component.compile_options.dev || block.has_content(); + // fix: remove "component.compile_options.dev" condition, which + // will set has_create_fragment always be true in dev mode, + // inconsistent with the behavior in production mode. + const has_create_fragment = block.has_content(); if (has_create_fragment) { body.push(b` function create_fragment(#ctx) { @@ -593,7 +595,7 @@ export default function dom( constructor(options) { super(${options.dev && 'options'}); @init(this, options, ${definition}, ${has_create_fragment ? 'create_fragment' : 'null'}, ${not_equal}, ${prop_indexes}, ${optional_parameters}); - ${options.dev && b`@dispatch_dev("SvelteRegisterComponent", { component: this, tagName: "${name.name}", options, id: create_fragment.name });`} + ${options.dev && b`@dispatch_dev("SvelteRegisterComponent", { component: this, tagName: "${name.name}", options, id: ${has_create_fragment ? 'create_fragment.name' : 'this.name'} });`} ${dev_props_check} } From e57a97b2af8dd2e63f59693270cbb8837123720b Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 6 May 2022 14:19:25 +0800 Subject: [PATCH 03/10] chore: revert 'removal' of component.compile_options.dev --- src/compiler/compile/render_dom/index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 6c1590d4d161..5e9fe8b1bde2 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.ts @@ -333,10 +333,8 @@ export default function dom( // $$props arg is still needed for unknown prop check args.push(x`$$props`); } - // fix: remove "component.compile_options.dev" condition, which - // will set has_create_fragment always be true in dev mode, - // inconsistent with the behavior in production mode. - const has_create_fragment = block.has_content(); + // has_create_fragment is intentionally to be true in dev mode. + const has_create_fragment = component.compile_options.dev || block.has_content(); if (has_create_fragment) { body.push(b` function create_fragment(#ctx) { From 3d6cd65d940c2c147964d3fd64e405dd5521b8c3 Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 6 May 2022 14:27:41 +0800 Subject: [PATCH 04/10] feat: add test for destroying empty component --- .../empty-component-destroy/Empty.svelte | 8 ++++++++ .../samples/empty-component-destroy/_config.js | 17 +++++++++++++++++ .../samples/empty-component-destroy/main.svelte | 8 ++++++++ 3 files changed, 33 insertions(+) create mode 100644 test/runtime/samples/empty-component-destroy/Empty.svelte create mode 100644 test/runtime/samples/empty-component-destroy/_config.js create mode 100644 test/runtime/samples/empty-component-destroy/main.svelte diff --git a/test/runtime/samples/empty-component-destroy/Empty.svelte b/test/runtime/samples/empty-component-destroy/Empty.svelte new file mode 100644 index 000000000000..0c2a7a9c3b0d --- /dev/null +++ b/test/runtime/samples/empty-component-destroy/Empty.svelte @@ -0,0 +1,8 @@ + + diff --git a/test/runtime/samples/empty-component-destroy/_config.js b/test/runtime/samples/empty-component-destroy/_config.js new file mode 100644 index 000000000000..0334ab2e80b8 --- /dev/null +++ b/test/runtime/samples/empty-component-destroy/_config.js @@ -0,0 +1,17 @@ +export default { + html: ` + + `, + + async test({ assert, target, window }) { + const button = target.querySelector('button'); + const event = new window.MouseEvent('click'); + const messages = []; + console.log = msg => messages.push(msg); + await button.dispatchEvent(event); + assert.htmlEqual(target.innerHTML, ` + + `); + assert.deepEqual(messages, ['destroy']); + } +}; diff --git a/test/runtime/samples/empty-component-destroy/main.svelte b/test/runtime/samples/empty-component-destroy/main.svelte new file mode 100644 index 000000000000..0ee5ef4a04b0 --- /dev/null +++ b/test/runtime/samples/empty-component-destroy/main.svelte @@ -0,0 +1,8 @@ + + + + + From ff1ada7c1b266eae924480265d539889b67c8969 Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 6 May 2022 14:43:52 +0800 Subject: [PATCH 05/10] chore: update typechecking callback --- src/runtime/internal/transitions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/internal/transitions.ts b/src/runtime/internal/transitions.ts index 457d234087dc..4a101e5ce8bc 100644 --- a/src/runtime/internal/transitions.ts +++ b/src/runtime/internal/transitions.ts @@ -80,7 +80,7 @@ export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, ca block.o(local); // fix: destroy non-fragment element such as empty components. - } else if (typeof callback === 'function') { + } else if (callback) { callback(); } } From 09852ff033f2c5c873491d67ea92fa906d28c418 Mon Sep 17 00:00:00 2001 From: qinmu Date: Fri, 6 May 2022 17:14:09 +0800 Subject: [PATCH 06/10] chore: revert fragment dev checks --- src/compiler/compile/render_dom/Block.ts | 5 +---- src/compiler/compile/render_dom/index.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/compiler/compile/render_dom/Block.ts b/src/compiler/compile/render_dom/Block.ts index 2d930c0a11d9..34c477480463 100644 --- a/src/compiler/compile/render_dom/Block.ts +++ b/src/compiler/compile/render_dom/Block.ts @@ -428,16 +428,13 @@ export default class Block { } has_content(): boolean { - // exclude "ThrowStatment" type node - // as in dev mode, 'ThrowStatement' type node will always be added, which will make has_content() always return true in dev mode - const validClaims = this.chunks.claim.filter(i => Array.isArray(i) && i[0] && i[0].type !== 'ThrowStatement'); return !!this.first || this.event_listeners.length > 0 || this.chunks.intro.length > 0 || this.chunks.outro.length > 0 || this.chunks.create.length > 0 || this.chunks.hydrate.length > 0 || - validClaims.length > 0 || + this.chunks.claim.length > 0 || this.chunks.mount.length > 0 || this.chunks.update.length > 0 || this.chunks.destroy.length > 0 || diff --git a/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 5e9fe8b1bde2..5fdac9bce458 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.ts @@ -593,7 +593,7 @@ export default function dom( constructor(options) { super(${options.dev && 'options'}); @init(this, options, ${definition}, ${has_create_fragment ? 'create_fragment' : 'null'}, ${not_equal}, ${prop_indexes}, ${optional_parameters}); - ${options.dev && b`@dispatch_dev("SvelteRegisterComponent", { component: this, tagName: "${name.name}", options, id: ${has_create_fragment ? 'create_fragment.name' : 'this.name'} });`} + ${options.dev && b`@dispatch_dev("SvelteRegisterComponent", { component: this, tagName: "${name.name}", options, id: create_fragment.name });`} ${dev_props_check} } From 26d1853e88f203fda195fbc56ee18ae888daf084 Mon Sep 17 00:00:00 2001 From: qinmu Date: Sat, 7 May 2022 19:23:18 +0800 Subject: [PATCH 07/10] chore: remove unnecessary comment --- src/runtime/internal/transitions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/internal/transitions.ts b/src/runtime/internal/transitions.ts index 4a101e5ce8bc..306a5e3793b4 100644 --- a/src/runtime/internal/transitions.ts +++ b/src/runtime/internal/transitions.ts @@ -79,7 +79,6 @@ export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, ca }); block.o(local); - // fix: destroy non-fragment element such as empty components. } else if (callback) { callback(); } From 42b7683b7eed39748244e80ea6a9a883075cbe0e Mon Sep 17 00:00:00 2001 From: qinmu Date: Sat, 7 May 2022 19:26:30 +0800 Subject: [PATCH 08/10] chore: update test for empty-component-destroy --- test/runtime/samples/empty-component-destroy/Empty.svelte | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/runtime/samples/empty-component-destroy/Empty.svelte b/test/runtime/samples/empty-component-destroy/Empty.svelte index 0c2a7a9c3b0d..14551c069ad3 100644 --- a/test/runtime/samples/empty-component-destroy/Empty.svelte +++ b/test/runtime/samples/empty-component-destroy/Empty.svelte @@ -1,8 +1,8 @@ From e285f446d67d849692c984e4825bf0d70d2302a9 Mon Sep 17 00:00:00 2001 From: qinmu Date: Sun, 3 Jul 2022 16:16:56 +0800 Subject: [PATCH 09/10] fix: revert back the patching of console.log --- test/runtime/samples/empty-component-destroy/_config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/runtime/samples/empty-component-destroy/_config.js b/test/runtime/samples/empty-component-destroy/_config.js index 0334ab2e80b8..ca47a00bafc5 100644 --- a/test/runtime/samples/empty-component-destroy/_config.js +++ b/test/runtime/samples/empty-component-destroy/_config.js @@ -7,11 +7,13 @@ export default { const button = target.querySelector('button'); const event = new window.MouseEvent('click'); const messages = []; + const log = console.log; console.log = msg => messages.push(msg); await button.dispatchEvent(event); assert.htmlEqual(target.innerHTML, ` `); assert.deepEqual(messages, ['destroy']); + console.log = log; } }; From 21a01f5dddbf734ab2b694855e2e71dff11c156e Mon Sep 17 00:00:00 2001 From: tanhauhau Date: Mon, 4 Jul 2022 00:14:15 +0800 Subject: [PATCH 10/10] use before_test and after_test --- .../runtime/samples/empty-component-destroy/_config.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/runtime/samples/empty-component-destroy/_config.js b/test/runtime/samples/empty-component-destroy/_config.js index ca47a00bafc5..76b9ae4bb82e 100644 --- a/test/runtime/samples/empty-component-destroy/_config.js +++ b/test/runtime/samples/empty-component-destroy/_config.js @@ -1,19 +1,25 @@ +let log; export default { html: ` `, + before_test() { + log = console.log; + }, + after_test() { + console.log = log; + }, + async test({ assert, target, window }) { const button = target.querySelector('button'); const event = new window.MouseEvent('click'); const messages = []; - const log = console.log; console.log = msg => messages.push(msg); await button.dispatchEvent(event); assert.htmlEqual(target.innerHTML, ` `); assert.deepEqual(messages, ['destroy']); - console.log = log; } };