Skip to content

fix: remove memory leak from retaining old DOM elements #11197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: remove memory leak from retaining old DOM elements
  • Loading branch information
trueadm committed Apr 16, 2024
commit 3c53da40ecfd66f9af3161bc445afa0e6a30c008
5 changes: 5 additions & 0 deletions .changeset/rich-plums-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: remove memory leak from retaining old DOM elements
17 changes: 14 additions & 3 deletions packages/svelte/src/internal/client/dom/blocks/html.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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 { hydrate_nodes, hydrating } from '../hydration.js';
import { create_fragment_from_html, remove } from '../reconciler.js';
import { push_template_node } from '../template.js';

/**
* @param {Element | Text | Comment} anchor
Expand All @@ -11,10 +12,12 @@ import { create_fragment_from_html, remove } from '../reconciler.js';
* @returns {void}
*/
export function html(anchor, get_value, svg) {
var 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, effect, get(value), svg);
effect = null;

if (dom) {
return () => remove(dom);
Expand All @@ -27,11 +30,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 + '';
Expand All @@ -49,6 +53,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;
}

Expand All @@ -62,5 +69,9 @@ function html_to_dom(target, value, svg) {
target.before(node);
}

if (effect !== null) {
push_template_node(effect, nodes);
}

return nodes;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
push_template_node(parent_effect, element);
}
});
}
Expand Down
88 changes: 69 additions & 19 deletions packages/svelte/src/internal/client/dom/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
};
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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;
};
}

Expand Down Expand Up @@ -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];

Expand All @@ -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);
Expand All @@ -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;
}