From 22e2aeca76b3049c2b249712885a98d148d74c4a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 29 May 2024 22:13:49 +0100 Subject: [PATCH] fix: address regressed memory leak (#11832) * fix: address regressed memory leak * fix: address regressed memory leak --- .changeset/sour-geese-listen.md | 5 ++ .../src/internal/client/dom/blocks/html.js | 13 ++++- .../client/dom/blocks/svelte-element.js | 36 ++++++++++-- .../src/internal/client/dom/template.js | 56 ++++++++++++++----- 4 files changed, 90 insertions(+), 20 deletions(-) create mode 100644 .changeset/sour-geese-listen.md diff --git a/.changeset/sour-geese-listen.md b/.changeset/sour-geese-listen.md new file mode 100644 index 0000000000..d96cbcecda --- /dev/null +++ b/.changeset/sour-geese-listen.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: address regressed memory leak diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index 103bf7c046..6873adf70c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -4,6 +4,7 @@ import { current_effect, get } from '../../runtime.js'; import { is_array } from '../../utils.js'; import { hydrate_nodes, hydrating } from '../hydration.js'; import { create_fragment_from_html, remove } from '../reconciler.js'; +import { push_template_node } from '../template.js'; /** * @param {import('#client').Effect} effect @@ -37,7 +38,7 @@ export function html(anchor, get_value, svg, mathml) { let value = derived(get_value); render_effect(() => { - var dom = html_to_dom(anchor, get(value), svg, mathml); + var dom = html_to_dom(anchor, parent_effect, get(value), svg, mathml); if (dom) { return () => { @@ -55,12 +56,13 @@ export function html(anchor, get_value, svg, mathml) { * inserts it before the target anchor and returns the new nodes. * @template V * @param {Element | Text | Comment} target + * @param {import('#client').Effect | null} effect * @param {V} value * @param {boolean} svg * @param {boolean} mathml * @returns {Element | Comment | (Element | Comment | Text)[]} */ -function html_to_dom(target, value, svg, mathml) { +function html_to_dom(target, effect, value, svg, mathml) { if (hydrating) return hydrate_nodes; var html = value + ''; @@ -79,6 +81,9 @@ function html_to_dom(target, value, svg, mathml) { if (node.childNodes.length === 1) { var child = /** @type {Text | Element | Comment} */ (node.firstChild); target.before(child); + if (effect !== null) { + push_template_node(child, effect); + } return child; } @@ -92,5 +97,9 @@ function html_to_dom(target, value, svg, mathml) { target.before(node); } + if (effect !== null) { + push_template_node(nodes, effect); + } + return nodes; } diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index 49d6ddeff7..df607fe54a 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -10,8 +10,31 @@ import { } from '../../reactivity/effects.js'; import { set_should_intro } from '../../render.js'; import { current_each_item, set_current_each_item } from './each.js'; -import { current_component_context } from '../../runtime.js'; +import { current_component_context, current_effect } from '../../runtime.js'; import { DEV } from 'esm-env'; +import { is_array } from '../../utils.js'; +import { push_template_node } from '../template.js'; + +/** + * @param {import('#client').Effect} effect + * @param {Element} from + * @param {Element} to + * @returns {void} + */ +function swap_block_dom(effect, from, to) { + const dom = effect.dom; + + if (is_array(dom)) { + for (let i = 0; i < dom.length; i++) { + if (dom[i] === from) { + dom[i] = to; + break; + } + } + } else if (dom === from) { + effect.dom = to; + } +} /** * @param {Comment} anchor @@ -23,6 +46,7 @@ import { DEV } from 'esm-env'; * @returns {void} */ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, location) { + const parent_effect = /** @type {import('#client').Effect} */ (current_effect); const filename = DEV && location && current_component_context?.function.filename; /** @type {string | null} */ @@ -78,6 +102,7 @@ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, locat if (next_tag && next_tag !== current_tag) { effect = branch(() => { + const prev_element = element; element = hydrating ? /** @type {Element} */ (hydrate_start) : ns @@ -111,9 +136,12 @@ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, locat anchor.before(element); - return () => { - element?.remove(); - }; + if (prev_element) { + swap_block_dom(parent_effect, prev_element, element); + prev_element.remove(); + } else if (!hydrating) { + push_template_node(element, parent_effect); + } }); } diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 6458dc65cd..85d6d5f852 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -4,17 +4,32 @@ import { create_fragment_from_html } from './reconciler.js'; import { current_effect } from '../runtime.js'; import { TEMPLATE_FRAGMENT, TEMPLATE_USE_IMPORT_NODE } from '../../../constants.js'; import { effect } from '../reactivity/effects.js'; +import { is_array } from '../utils.js'; /** * @template {import("#client").TemplateNode | import("#client").TemplateNode[]} T * @param {T} dom + * @param {import("#client").Effect} effect */ -function push_template_node(dom) { - var effect = /** @type {import('#client').Effect} */ (current_effect); - - if (effect.dom === null) { +export function push_template_node( + dom, + effect = /** @type {import('#client').Effect} */ (current_effect) +) { + var current_dom = effect.dom; + if (current_dom === null) { effect.dom = dom; + } else { + if (!is_array(current_dom)) { + current_dom = effect.dom = [current_dom]; + } + + if (is_array(dom)) { + current_dom.push(...dom); + } else { + current_dom.push(dom); + } } + return dom; } /** @@ -41,7 +56,15 @@ export function template(content, flags) { if (!is_fragment) node = /** @type {Node} */ (node.firstChild); } - return use_import_node ? document.importNode(node, true) : node.cloneNode(true); + var clone = use_import_node ? document.importNode(node, true) : node.cloneNode(true); + + push_template_node( + is_fragment + ? /** @type {import('#client').TemplateNode[]} */ ([...clone.childNodes]) + : /** @type {import('#client').TemplateNode} */ (clone) + ); + + return clone; }; } @@ -102,7 +125,15 @@ export function ns_template(content, flags, ns = 'svg') { } } - return node.cloneNode(true); + var clone = node.cloneNode(true); + + push_template_node( + is_fragment + ? /** @type {import('#client').TemplateNode[]} */ ([...clone.childNodes]) + : /** @type {import('#client').TemplateNode} */ (clone) + ); + + return clone; }; } @@ -177,7 +208,7 @@ function run_scripts(node) { */ /*#__NO_SIDE_EFFECTS__*/ export function text(anchor) { - if (!hydrating) return empty(); + if (!hydrating) return push_template_node(empty()); var node = hydrate_start; @@ -201,6 +232,7 @@ export function comment() { var frag = document.createDocumentFragment(); var anchor = empty(); frag.append(anchor); + push_template_node([anchor]); return frag; } @@ -213,13 +245,9 @@ export function comment() { */ export function append(anchor, dom) { if (hydrating) return; - - var effect = /** @type {import('#client').Effect} */ (current_effect); - - effect.dom = - dom.nodeType === 11 - ? /** @type {import('#client').TemplateNode[]} */ ([...dom.childNodes]) - : /** @type {Element | Comment} */ (dom); + // We intentionally do not assign the `dom` property of the effect here because it's far too + // late. If we try, we will capture additional DOM elements that we cannot control the lifecycle + // for and will inevitably cause memory leaks. See https://github.com/sveltejs/svelte/pull/11832 anchor.before(/** @type {Node} */ (dom)); }