Skip to content

Commit

Permalink
fix: address regressed memory leak (#11832)
Browse files Browse the repository at this point in the history
* fix: address regressed memory leak

* fix: address regressed memory leak
  • Loading branch information
trueadm authored May 29, 2024
1 parent bcb3193 commit 22e2aec
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/sour-geese-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: address regressed memory leak
13 changes: 11 additions & 2 deletions packages/svelte/src/internal/client/dom/blocks/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 + '';
Expand All @@ -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;
}

Expand All @@ -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;
}
36 changes: 32 additions & 4 deletions packages/svelte/src/internal/client/dom/blocks/svelte-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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} */
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
});
}

Expand Down
56 changes: 42 additions & 14 deletions packages/svelte/src/internal/client/dom/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

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

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

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

Expand All @@ -201,6 +232,7 @@ export function comment() {
var frag = document.createDocumentFragment();
var anchor = empty();
frag.append(anchor);
push_template_node([anchor]);

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

0 comments on commit 22e2aec

Please sign in to comment.