From 02f60fbebf7cdb036472d1aec8dc9d9f8215cd7a Mon Sep 17 00:00:00 2001 From: qinmu <magenta2127@gmail.com> Date: Mon, 4 Jul 2022 09:45:39 +0800 Subject: [PATCH] [fix]destroy empty component (#7492) * fix: destroy non-fragment element such as empty components * fix: fragment property of Empty Component is set as true in dev mode, inconsistent with production mode * chore: revert 'removal' of component.compile_options.dev * feat: add test for destroying empty component * chore: update typechecking callback * chore: revert fragment dev checks * chore: remove unnecessary comment * chore: update test for empty-component-destroy * fix: revert back the patching of console.log * use before_test and after_test Co-authored-by: qinmu <magenta2127@mail.com> Co-authored-by: tanhauhau <lhtan93@gmail.com> --- src/compiler/compile/render_dom/index.ts | 2 +- src/runtime/internal/transitions.ts | 4 ++- .../empty-component-destroy/Empty.svelte | 8 ++++++ .../empty-component-destroy/_config.js | 25 +++++++++++++++++++ .../empty-component-destroy/main.svelte | 8 ++++++ 5 files changed, 45 insertions(+), 2 deletions(-) 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/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 9d9699bdbf6c..5fdac9bce458 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.ts @@ -333,7 +333,7 @@ export default function dom( // $$props arg is still needed for unknown prop check args.push(x`$$props`); } - + // 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` diff --git a/src/runtime/internal/transitions.ts b/src/runtime/internal/transitions.ts index a0134668eff1..306a5e3793b4 100644 --- a/src/runtime/internal/transitions.ts +++ b/src/runtime/internal/transitions.ts @@ -79,6 +79,8 @@ export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, ca }); block.o(local); + } else if (callback) { + callback(); } } @@ -143,7 +145,7 @@ export function create_in_transition(node: Element & ElementCSSInlineStyle, fn: return { start() { if (started) return; - + started = true; delete_rule(node); 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..14551c069ad3 --- /dev/null +++ b/test/runtime/samples/empty-component-destroy/Empty.svelte @@ -0,0 +1,8 @@ + +<script> + import { onDestroy } from 'svelte'; + + onDestroy(() => { + console.log('destroy'); + }); +</script> 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..76b9ae4bb82e --- /dev/null +++ b/test/runtime/samples/empty-component-destroy/_config.js @@ -0,0 +1,25 @@ +let log; +export default { + html: ` + <button>destroy component</button> + `, + + 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 = []; + console.log = msg => messages.push(msg); + await button.dispatchEvent(event); + assert.htmlEqual(target.innerHTML, ` + <button>destroy component</button> + `); + 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 @@ +<script> + import Empty from './Empty.svelte'; + let active = true; +</script> + +<button on:click='{() => active = false }'>destroy component</button> + +<svelte:component this={active ? Empty : null} />