From 777527b5a3358fe0a9e05fa68fef7363150d57e8 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 17 Apr 2024 10:02:18 +0100 Subject: [PATCH] fix: remove memory leak from retaining old DOM elements (#11197) * fix: remove memory leak from retaining old DOM elements * missing logic * fix dynamic html bug --- .changeset/rich-plums-thank.md | 5 ++ .../src/internal/client/dom/blocks/html.js | 44 +++++++++- .../client/dom/blocks/svelte-element.js | 3 + .../src/internal/client/dom/template.js | 88 +++++++++++++++---- .../samples/each-dynamic-html/_config.js | 39 ++++++++ .../samples/each-dynamic-html/main.svelte | 30 +++++++ 6 files changed, 186 insertions(+), 23 deletions(-) create mode 100644 .changeset/rich-plums-thank.md create mode 100644 packages/svelte/tests/runtime-runes/samples/each-dynamic-html/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/each-dynamic-html/main.svelte diff --git a/.changeset/rich-plums-thank.md b/.changeset/rich-plums-thank.md new file mode 100644 index 0000000000..0c96191f92 --- /dev/null +++ b/.changeset/rich-plums-thank.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: remove memory leak from retaining old DOM elements diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index ff96617885..40583e09f2 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -1,8 +1,30 @@ import { derived } from '../../reactivity/deriveds.js'; import { render_effect } from '../../reactivity/effects.js'; -import { get } from '../../runtime.js'; +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 + * @param {(Element | Comment | Text)[]} to_remove + * @returns {void} + */ +function remove_from_parent_effect(effect, to_remove) { + const dom = effect.dom; + + if (is_array(dom)) { + for (let i = dom.length - 1; i >= 0; i--) { + if (to_remove.includes(dom[i])) { + dom.splice(i, 1); + break; + } + } + } else if (dom !== null && to_remove.includes(dom)) { + effect.dom = null; + } +} /** * @param {Element | Text | Comment} anchor @@ -11,13 +33,19 @@ import { create_fragment_from_html, remove } from '../reconciler.js'; * @returns {void} */ export function html(anchor, get_value, svg) { + const parent_effect = anchor.parentNode !== current_effect?.dom ? current_effect : null; let value = derived(get_value); render_effect(() => { - var dom = html_to_dom(anchor, get(value), svg); + var dom = html_to_dom(anchor, parent_effect, get(value), svg); if (dom) { - return () => remove(dom); + return () => { + if (parent_effect !== null) { + remove_from_parent_effect(parent_effect, is_array(dom) ? dom : [dom]); + } + remove(dom); + }; } }); } @@ -27,11 +55,12 @@ export function html(anchor, get_value, svg) { * 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 * @returns {Element | Comment | (Element | Comment | Text)[]} */ -function html_to_dom(target, value, svg) { +function html_to_dom(target, effect, value, svg) { if (hydrating) return hydrate_nodes; var html = value + ''; @@ -49,6 +78,9 @@ function html_to_dom(target, value, svg) { if (node.childNodes.length === 1) { var child = /** @type {Text | Element | Comment} */ (node.firstChild); target.before(child); + if (effect !== null) { + push_template_node(effect, child); + } return child; } @@ -62,5 +94,9 @@ function html_to_dom(target, value, svg) { target.before(node); } + if (effect !== null) { + push_template_node(effect, nodes); + } + 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 a0c3cc6d65..c7daff227b 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -13,6 +13,7 @@ import { is_array } from '../../utils.js'; import { set_should_intro } from '../../render.js'; import { current_each_item, set_current_each_item } from './each.js'; import { current_effect } from '../../runtime.js'; +import { push_template_node } from '../template.js'; /** * @param {import('#client').Effect} effect @@ -131,6 +132,8 @@ export function element(anchor, get_tag, is_svg, render_fn) { if (prev_element) { swap_block_dom(parent_effect, prev_element, element); prev_element.remove(); + } else if (!hydrating) { + push_template_node(parent_effect, element); } }); } diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 8e1f51c8eb..5baeb2f313 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -4,6 +4,36 @@ 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'; + +/** + * @param {import("#client").Effect} effect + * @param {import("#client").TemplateNode | import("#client").TemplateNode[]} dom + */ +export function push_template_node(effect, dom) { + var current_dom = effect.dom; + if (current_dom === null) { + effect.dom = dom; + } else { + if (!is_array(current_dom)) { + current_dom = effect.dom = [current_dom]; + } + var anchor; + // If we're working with an anchor, then remove it and put it at the end. + if (current_dom[0].nodeType === 8) { + anchor = current_dom.pop(); + } + if (is_array(dom)) { + current_dom.push(...dom); + } else { + current_dom.push(dom); + } + if (anchor !== undefined) { + current_dom.push(anchor); + } + } + return dom; +} /** * @param {string} content @@ -19,16 +49,31 @@ export function template(content, flags) { var node; return () => { + var effect = /** @type {import('#client').Effect} */ (current_effect); if (hydrating) { - return is_fragment ? hydrate_nodes : /** @type {Node} */ (hydrate_nodes[0]); + var hydration_content = push_template_node( + effect, + is_fragment ? hydrate_nodes : hydrate_nodes[0] + ); + return /** @type {Node} */ (hydration_content); } if (!node) { node = create_fragment_from_html(content); if (!is_fragment) node = /** @type {Node} */ (node.firstChild); } + var clone = use_import_node ? document.importNode(node, true) : clone_node(node, true); + + if (is_fragment) { + push_template_node( + effect, + /** @type {import('#client').TemplateNode[]} */ ([...clone.childNodes]) + ); + } else { + push_template_node(effect, /** @type {import('#client').TemplateNode} */ (clone)); + } - return use_import_node ? document.importNode(node, true) : clone_node(node, true); + return clone; }; } @@ -70,8 +115,13 @@ export function svg_template(content, flags) { var node; return () => { + var effect = /** @type {import('#client').Effect} */ (current_effect); if (hydrating) { - return is_fragment ? hydrate_nodes : /** @type {Node} */ (hydrate_nodes[0]); + var hydration_content = push_template_node( + effect, + is_fragment ? hydrate_nodes : hydrate_nodes[0] + ); + return /** @type {Node} */ (hydration_content); } if (!node) { @@ -87,7 +137,18 @@ export function svg_template(content, flags) { } } - return clone_node(node, true); + var clone = clone_node(node, true); + + if (is_fragment) { + push_template_node( + effect, + /** @type {import('#client').TemplateNode[]} */ ([...clone.childNodes]) + ); + } else { + push_template_node(effect, /** @type {import('#client').TemplateNode} */ (clone)); + } + + return clone; }; } @@ -152,7 +213,8 @@ function run_scripts(node) { */ /*#__NO_SIDE_EFFECTS__*/ export function text(anchor) { - if (!hydrating) return empty(); + var effect = /** @type {import('#client').Effect} */ (current_effect); + if (!hydrating) return push_template_node(effect, empty()); var node = hydrate_nodes[0]; @@ -162,7 +224,7 @@ export function text(anchor) { anchor.before((node = empty())); } - return node; + return push_template_node(effect, node); } export const comment = template('', TEMPLATE_FRAGMENT); @@ -174,19 +236,7 @@ export const comment = template('', TEMPLATE_FRAGMENT); * @param {import('#client').Dom} dom */ export function append(anchor, dom) { - var current = dom; - if (!hydrating) { - var node = /** @type {Node} */ (dom); - - if (node.nodeType === 11) { - // if hydrating, `dom` is already an array of nodes, but if not then - // we need to create an array to store it on the current effect - current = /** @type {import('#client').Dom} */ ([...node.childNodes]); - } - - anchor.before(node); + anchor.before(/** @type {Node} */ (dom)); } - - /** @type {import('#client').Effect} */ (current_effect).dom = current; } diff --git a/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/_config.js b/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/_config.js new file mode 100644 index 0000000000..93f3702491 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/_config.js @@ -0,0 +1,39 @@ +import { flushSync } from '../../../../src/index-client'; +import { test } from '../../test'; + +export default test({ + html: ``, + + async test({ assert, target }) { + const [btn1, btn2, btn3] = target.querySelectorAll('button'); + + flushSync(() => { + btn1?.click(); + btn1?.click(); + btn1?.click(); + }); + + assert.htmlEqual( + target.innerHTML, + `
Item 1
Item 2
Item 3
` + ); + + flushSync(() => { + btn2?.click(); + }); + + assert.htmlEqual( + target.innerHTML, + `Item 1Item 2Item 3` + ); + + flushSync(() => { + btn3?.click(); + }); + + assert.htmlEqual( + target.innerHTML, + `Item 3Item 2Item 1` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/main.svelte new file mode 100644 index 0000000000..350cbf0d4a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-dynamic-html/main.svelte @@ -0,0 +1,30 @@ + + + + + + +{#each items as item (item.id)} + {@html item.html} +{/each}