From 56b2d2ce9e8938afe88fe4fdfe1e95014b48635e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 20 Apr 2024 15:02:55 -0400 Subject: [PATCH 1/5] fix: only destroy snippets when they have changed --- .../src/internal/client/dom/blocks/snippet.js | 25 ++++++++++++------ .../client/dom/elements/transitions.js | 26 ++++++++++++------- .../src/internal/client/reactivity/effects.js | 9 ++++--- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index e88544091082..f7d999da2b44 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -1,5 +1,5 @@ import { EFFECT_TRANSPARENT } from '../../constants.js'; -import { branch, render_effect } from '../../reactivity/effects.js'; +import { branch, block, destroy_effect } from '../../reactivity/effects.js'; /** * @template {(node: import('#client').TemplateNode, ...args: any[]) => import('#client').Dom} SnippetFn @@ -12,13 +12,22 @@ export function snippet(get_snippet, node, ...args) { /** @type {SnippetFn | null | undefined} */ var snippet; - var effect = render_effect(() => { - if (snippet === (snippet = get_snippet())) return; + /** @type {import('#client').Effect | null} */ + var snippet_effect; - if (snippet) { - branch(() => /** @type {SnippetFn} */ (snippet)(node, ...args)); - } - }); + block( + () => { + if (snippet === (snippet = get_snippet())) return; - effect.f |= EFFECT_TRANSPARENT; + if (snippet_effect) { + destroy_effect(snippet_effect); + snippet_effect = null; + } + + if (snippet) { + snippet_effect = branch(() => /** @type {SnippetFn} */ (snippet)(node, ...args)); + } + }, + EFFECT_TRANSPARENT | (1 << 20) + ); } diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index a9f55d940fcf..a3a20921891c 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -7,7 +7,7 @@ import { should_intro } from '../../render.js'; import { is_function } from '../../utils.js'; import { current_each_item } from '../blocks/each.js'; import { TRANSITION_GLOBAL, TRANSITION_IN, TRANSITION_OUT } from '../../../../constants.js'; -import { BLOCK_EFFECT, EFFECT_RAN } from '../../constants.js'; +import { BLOCK_EFFECT, EFFECT_RAN, EFFECT_TRANSPARENT } from '../../constants.js'; /** * @template T @@ -241,18 +241,26 @@ export function transition(flags, element, get_fn, get_params) { (e.transitions ??= []).push(transition); - // if this is a local transition, we only want to run it if the parent (block) effect's - // parent (branch) effect is where the state change happened. we can determine that by - // looking at whether the branch effect is currently initializing + // if this is a local transition, we only want to run it if the parent (branch) effect's + // parent (block) effect is where the state change happened. we can determine that by + // looking at whether the block effect is currently initializing if (is_intro && should_intro) { - var parent = /** @type {import('#client').Effect} */ (e.parent); + let run = is_global; - // e.g snippets are implemented as render effects — keep going until we find the parent block - while ((parent.f & BLOCK_EFFECT) === 0 && parent.parent) { - parent = parent.parent; + if (!run) { + var block = /** @type {import('#client').Effect} */ (e.parent); + + // skip over transparent blocks (e.g. snippets, else-if blocks) + while (block && (block.f & EFFECT_TRANSPARENT) !== 0) { + while ((block = /** @type {import('#client').Effect} */ (block.parent))) { + if ((block.f & BLOCK_EFFECT) !== 0) break; + } + } + + run = (block.f & EFFECT_RAN) !== 0; } - if (is_global || (parent.f & EFFECT_RAN) !== 0) { + if (run) { effect(() => { untrack(() => transition.in()); }); diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 3b0b97ade50e..515d13136eb1 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -230,9 +230,12 @@ export function render_effect(fn) { return create_effect(RENDER_EFFECT, fn, true); } -/** @param {(() => void)} fn */ -export function block(fn) { - return create_effect(RENDER_EFFECT | BLOCK_EFFECT, fn, true); +/** + * @param {(() => void)} fn + * @param {number} flags + */ +export function block(fn, flags = 0) { + return create_effect(RENDER_EFFECT | BLOCK_EFFECT | flags, fn, true); } /** @param {(() => void)} fn */ From 5b8b99e95bafd18f9d780b19fd12c3dcbafb1020 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 20 Apr 2024 15:15:34 -0400 Subject: [PATCH 2/5] tidy up --- .../src/internal/client/dom/blocks/if.js | 8 ++----- .../src/internal/client/dom/blocks/snippet.js | 23 ++++++++----------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index c432943c1a6a..3295d6db9107 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -28,7 +28,7 @@ export function if_block( /** @type {boolean | null} */ let condition = null; - const effect = block(() => { + block(() => { if (condition === (condition = !!get_condition())) return; /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ @@ -76,9 +76,5 @@ export function if_block( // continue in hydration mode set_hydrating(true); } - }); - - if (elseif) { - effect.f |= EFFECT_TRANSPARENT; - } + }, EFFECT_TRANSPARENT); } diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index f7d999da2b44..c5097ee2c3c6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -15,19 +15,16 @@ export function snippet(get_snippet, node, ...args) { /** @type {import('#client').Effect | null} */ var snippet_effect; - block( - () => { - if (snippet === (snippet = get_snippet())) return; + block(() => { + if (snippet === (snippet = get_snippet())) return; - if (snippet_effect) { - destroy_effect(snippet_effect); - snippet_effect = null; - } + if (snippet_effect) { + destroy_effect(snippet_effect); + snippet_effect = null; + } - if (snippet) { - snippet_effect = branch(() => /** @type {SnippetFn} */ (snippet)(node, ...args)); - } - }, - EFFECT_TRANSPARENT | (1 << 20) - ); + if (snippet) { + snippet_effect = branch(() => /** @type {SnippetFn} */ (snippet)(node, ...args)); + } + }, EFFECT_TRANSPARENT); } From 57f50eb8ae7456993e805b342a3ccacf7dee69be Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 20 Apr 2024 15:17:36 -0400 Subject: [PATCH 3/5] changeset --- .changeset/shiny-mayflies-clean.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shiny-mayflies-clean.md diff --git a/.changeset/shiny-mayflies-clean.md b/.changeset/shiny-mayflies-clean.md new file mode 100644 index 000000000000..3adaa3dec85a --- /dev/null +++ b/.changeset/shiny-mayflies-clean.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: only destroy snippets when they have changed From f41556c7ba8e109ab57ac2ec3d945ee49be30f4a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 20 Apr 2024 15:36:20 -0400 Subject: [PATCH 4/5] oops --- .../src/internal/client/dom/blocks/if.js | 81 ++++++++++--------- .../client/dom/elements/transitions.js | 6 +- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index 3295d6db9107..0fe8e8fda633 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -28,53 +28,56 @@ export function if_block( /** @type {boolean | null} */ let condition = null; - block(() => { - if (condition === (condition = !!get_condition())) return; + block( + () => { + if (condition === (condition = !!get_condition())) return; - /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ - let mismatch = false; + /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ + let mismatch = false; - if (hydrating) { - const is_else = anchor.data === HYDRATION_END_ELSE; + if (hydrating) { + const is_else = anchor.data === HYDRATION_END_ELSE; - if (condition === is_else) { - // Hydration mismatch: remove everything inside the anchor and start fresh. - // This could happen with `{#if browser}...{/if}`, for example - remove(hydrate_nodes); - set_hydrating(false); - mismatch = true; + if (condition === is_else) { + // Hydration mismatch: remove everything inside the anchor and start fresh. + // This could happen with `{#if browser}...{/if}`, for example + remove(hydrate_nodes); + set_hydrating(false); + mismatch = true; + } } - } - if (condition) { - if (consequent_effect) { - resume_effect(consequent_effect); + if (condition) { + if (consequent_effect) { + resume_effect(consequent_effect); + } else { + consequent_effect = branch(() => consequent_fn(anchor)); + } + + if (alternate_effect) { + pause_effect(alternate_effect, () => { + alternate_effect = null; + }); + } } else { - consequent_effect = branch(() => consequent_fn(anchor)); - } + if (alternate_effect) { + resume_effect(alternate_effect); + } else if (alternate_fn) { + alternate_effect = branch(() => alternate_fn(anchor)); + } - if (alternate_effect) { - pause_effect(alternate_effect, () => { - alternate_effect = null; - }); - } - } else { - if (alternate_effect) { - resume_effect(alternate_effect); - } else if (alternate_fn) { - alternate_effect = branch(() => alternate_fn(anchor)); + if (consequent_effect) { + pause_effect(consequent_effect, () => { + consequent_effect = null; + }); + } } - if (consequent_effect) { - pause_effect(consequent_effect, () => { - consequent_effect = null; - }); + if (mismatch) { + // continue in hydration mode + set_hydrating(true); } - } - - if (mismatch) { - // continue in hydration mode - set_hydrating(true); - } - }, EFFECT_TRANSPARENT); + }, + elseif ? EFFECT_TRANSPARENT : 0 + ); } diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index a3a20921891c..3551a093da1d 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -248,16 +248,16 @@ export function transition(flags, element, get_fn, get_params) { let run = is_global; if (!run) { - var block = /** @type {import('#client').Effect} */ (e.parent); + var block = /** @type {import('#client').Effect | null} */ (e.parent); // skip over transparent blocks (e.g. snippets, else-if blocks) while (block && (block.f & EFFECT_TRANSPARENT) !== 0) { - while ((block = /** @type {import('#client').Effect} */ (block.parent))) { + while ((block = block.parent)) { if ((block.f & BLOCK_EFFECT) !== 0) break; } } - run = (block.f & EFFECT_RAN) !== 0; + run = !block || (block.f & EFFECT_RAN) !== 0; } if (run) { From 33667764894c3bbe35110de54db8f03c8734749c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 22 Apr 2024 07:50:20 -0400 Subject: [PATCH 5/5] tidy up --- .../src/internal/client/dom/blocks/if.js | 89 +++++++++---------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index 0fe8e8fda633..80ed8c09f4f6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -20,64 +20,63 @@ export function if_block( elseif = false ) { /** @type {import('#client').Effect | null} */ - let consequent_effect = null; + var consequent_effect = null; /** @type {import('#client').Effect | null} */ - let alternate_effect = null; + var alternate_effect = null; /** @type {boolean | null} */ - let condition = null; + var condition = null; - block( - () => { - if (condition === (condition = !!get_condition())) return; + var flags = elseif ? EFFECT_TRANSPARENT : 0; - /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ - let mismatch = false; + block(() => { + if (condition === (condition = !!get_condition())) return; - if (hydrating) { - const is_else = anchor.data === HYDRATION_END_ELSE; + /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ + let mismatch = false; - if (condition === is_else) { - // Hydration mismatch: remove everything inside the anchor and start fresh. - // This could happen with `{#if browser}...{/if}`, for example - remove(hydrate_nodes); - set_hydrating(false); - mismatch = true; - } - } + if (hydrating) { + const is_else = anchor.data === HYDRATION_END_ELSE; - if (condition) { - if (consequent_effect) { - resume_effect(consequent_effect); - } else { - consequent_effect = branch(() => consequent_fn(anchor)); - } + if (condition === is_else) { + // Hydration mismatch: remove everything inside the anchor and start fresh. + // This could happen with `{#if browser}...{/if}`, for example + remove(hydrate_nodes); + set_hydrating(false); + mismatch = true; + } + } - if (alternate_effect) { - pause_effect(alternate_effect, () => { - alternate_effect = null; - }); - } + if (condition) { + if (consequent_effect) { + resume_effect(consequent_effect); } else { - if (alternate_effect) { - resume_effect(alternate_effect); - } else if (alternate_fn) { - alternate_effect = branch(() => alternate_fn(anchor)); - } + consequent_effect = branch(() => consequent_fn(anchor)); + } - if (consequent_effect) { - pause_effect(consequent_effect, () => { - consequent_effect = null; - }); - } + if (alternate_effect) { + pause_effect(alternate_effect, () => { + alternate_effect = null; + }); + } + } else { + if (alternate_effect) { + resume_effect(alternate_effect); + } else if (alternate_fn) { + alternate_effect = branch(() => alternate_fn(anchor)); } - if (mismatch) { - // continue in hydration mode - set_hydrating(true); + if (consequent_effect) { + pause_effect(consequent_effect, () => { + consequent_effect = null; + }); } - }, - elseif ? EFFECT_TRANSPARENT : 0 - ); + } + + if (mismatch) { + // continue in hydration mode + set_hydrating(true); + } + }, flags); }