From c745945e9ec92f6e2809b11f4e68594cee6dd36f Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 10:45:41 -0700 Subject: [PATCH 01/10] fix: avoid recursion error when tagging circular references --- .changeset/six-shirts-scream.md | 5 +++ .../svelte/src/internal/client/dev/tracing.js | 7 +++-- packages/svelte/src/internal/client/proxy.js | 31 ++++++++++++++----- .../src/internal/client/reactivity/sources.js | 2 +- 4 files changed, 34 insertions(+), 11 deletions(-) create mode 100644 .changeset/six-shirts-scream.md diff --git a/.changeset/six-shirts-scream.md b/.changeset/six-shirts-scream.md new file mode 100644 index 000000000000..ac4d1834741b --- /dev/null +++ b/.changeset/six-shirts-scream.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid recursion error when tagging circular references diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index 673a710faca6..06b0055cb9ab 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -179,7 +179,7 @@ export function get_stack(label) { */ export function tag(source, label) { source.label = label; - tag_proxy(source.v, label); + tag_proxy(source.v, label, source); return source; } @@ -187,10 +187,11 @@ export function tag(source, label) { /** * @param {unknown} value * @param {string} label + * @param {Value} source */ -export function tag_proxy(value, label) { +export function tag_proxy(value, label, source) { // @ts-expect-error - value?.[PROXY_PATH_SYMBOL]?.(label); + value?.[PROXY_PATH_SYMBOL]?.(label, source); return value; } diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 3ae4b87ed5d6..e39b8d610627 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -34,9 +34,13 @@ const regex_is_valid_identifier = /^[a-zA-Z_$][a-zA-Z_$0-9]*$/; /** * @template T * @param {T} value + * @param {WeakSet} [owned_sources] * @returns {T} */ -export function proxy(value) { +export function proxy(value, owned_sources) { + if (DEV) { + owned_sources ??= new WeakSet(); + } // if non-proxyable, or is already a proxy, return `value` if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) { return value; @@ -94,8 +98,15 @@ export function proxy(value) { /** Used in dev for $inspect.trace() */ var path = ''; - /** @param {string} new_path */ - function update_path(new_path) { + /** + * @param {string} new_path + * @param {Source} updater the source causing the path update + */ + function update_path(new_path, updater) { + // there's a circular reference somewhere, don't update the path to avoid recursion + if (owned_sources?.has(updater)) { + return; + } path = new_path; tag(version, `${path} version`); @@ -127,6 +138,7 @@ export function proxy(value) { sources.set(prop, s); if (DEV && typeof prop === 'string') { tag(s, get_label(path, prop)); + owned_sources?.add(s); } return s; }); @@ -148,6 +160,7 @@ export function proxy(value) { if (DEV) { tag(s, get_label(path, prop)); + owned_sources?.add(s); } } } else { @@ -173,11 +186,12 @@ export function proxy(value) { // create a source, but only if it's an own property and not a prototype property if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) { s = with_parent(() => { - var p = proxy(exists ? target[prop] : UNINITIALIZED); + var p = proxy(exists ? target[prop] : UNINITIALIZED, DEV ? owned_sources : undefined); var s = source(p, stack); if (DEV) { tag(s, get_label(path, prop)); + owned_sources?.add(s); } return s; @@ -231,11 +245,12 @@ export function proxy(value) { ) { if (s === undefined) { s = with_parent(() => { - var p = has ? proxy(target[prop]) : UNINITIALIZED; + var p = has ? proxy(target[prop], DEV ? owned_sources : undefined) : UNINITIALIZED; var s = source(p, stack); if (DEV) { tag(s, get_label(path, prop)); + owned_sources?.add(s); } return s; @@ -272,6 +287,7 @@ export function proxy(value) { if (DEV) { tag(other_s, get_label(path, i)); + owned_sources?.add(other_s); } } } @@ -284,18 +300,19 @@ export function proxy(value) { if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { s = with_parent(() => source(undefined, stack)); - set(s, proxy(value)); + set(s, proxy(value, DEV ? owned_sources : undefined)); sources.set(prop, s); if (DEV) { tag(s, get_label(path, prop)); + owned_sources?.add(s); } } } else { has = s.v !== UNINITIALIZED; - var p = with_parent(() => proxy(value)); + var p = with_parent(() => proxy(value, DEV ? owned_sources : undefined)); set(s, p); } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7fb3135708d3..45582d4f9f65 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -156,7 +156,7 @@ export function set(source, value, should_proxy = false) { let new_value = should_proxy ? proxy(value) : value; if (DEV) { - tag_proxy(new_value, /** @type {string} */ (source.label)); + tag_proxy(new_value, /** @type {string} */ (source.label), source); } return internal_set(source, new_value); From e862e5e044cb8d8cd143dfb058bdb9047c106f65 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 11:40:39 -0700 Subject: [PATCH 02/10] try suggestion --- .../svelte/src/internal/client/dev/tracing.js | 7 ++-- packages/svelte/src/internal/client/proxy.js | 36 ++++++------------- .../src/internal/client/reactivity/sources.js | 2 +- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index 06b0055cb9ab..673a710faca6 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -179,7 +179,7 @@ export function get_stack(label) { */ export function tag(source, label) { source.label = label; - tag_proxy(source.v, label, source); + tag_proxy(source.v, label); return source; } @@ -187,11 +187,10 @@ export function tag(source, label) { /** * @param {unknown} value * @param {string} label - * @param {Value} source */ -export function tag_proxy(value, label, source) { +export function tag_proxy(value, label) { // @ts-expect-error - value?.[PROXY_PATH_SYMBOL]?.(label, source); + value?.[PROXY_PATH_SYMBOL]?.(label); return value; } diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index e39b8d610627..1c72ebc16f51 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -34,13 +34,9 @@ const regex_is_valid_identifier = /^[a-zA-Z_$][a-zA-Z_$0-9]*$/; /** * @template T * @param {T} value - * @param {WeakSet} [owned_sources] * @returns {T} */ -export function proxy(value, owned_sources) { - if (DEV) { - owned_sources ??= new WeakSet(); - } +export function proxy(value) { // if non-proxyable, or is already a proxy, return `value` if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) { return value; @@ -97,16 +93,11 @@ export function proxy(value, owned_sources) { /** Used in dev for $inspect.trace() */ var path = ''; - - /** - * @param {string} new_path - * @param {Source} updater the source causing the path update - */ - function update_path(new_path, updater) { - // there's a circular reference somewhere, don't update the path to avoid recursion - if (owned_sources?.has(updater)) { - return; - } + let updating = false; + /** @param {string} new_path */ + function update_path(new_path) { + if (updating) return; + updating = true; path = new_path; tag(version, `${path} version`); @@ -115,6 +106,7 @@ export function proxy(value, owned_sources) { for (const [prop, source] of sources) { tag(source, get_label(path, prop)); } + updating = false; } return new Proxy(/** @type {any} */ (value), { @@ -138,7 +130,6 @@ export function proxy(value, owned_sources) { sources.set(prop, s); if (DEV && typeof prop === 'string') { tag(s, get_label(path, prop)); - owned_sources?.add(s); } return s; }); @@ -160,7 +151,6 @@ export function proxy(value, owned_sources) { if (DEV) { tag(s, get_label(path, prop)); - owned_sources?.add(s); } } } else { @@ -186,12 +176,11 @@ export function proxy(value, owned_sources) { // create a source, but only if it's an own property and not a prototype property if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) { s = with_parent(() => { - var p = proxy(exists ? target[prop] : UNINITIALIZED, DEV ? owned_sources : undefined); + var p = proxy(exists ? target[prop] : UNINITIALIZED); var s = source(p, stack); if (DEV) { tag(s, get_label(path, prop)); - owned_sources?.add(s); } return s; @@ -245,12 +234,11 @@ export function proxy(value, owned_sources) { ) { if (s === undefined) { s = with_parent(() => { - var p = has ? proxy(target[prop], DEV ? owned_sources : undefined) : UNINITIALIZED; + var p = has ? proxy(target[prop]) : UNINITIALIZED; var s = source(p, stack); if (DEV) { tag(s, get_label(path, prop)); - owned_sources?.add(s); } return s; @@ -287,7 +275,6 @@ export function proxy(value, owned_sources) { if (DEV) { tag(other_s, get_label(path, i)); - owned_sources?.add(other_s); } } } @@ -300,19 +287,18 @@ export function proxy(value, owned_sources) { if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { s = with_parent(() => source(undefined, stack)); - set(s, proxy(value, DEV ? owned_sources : undefined)); + set(s, proxy(value)); sources.set(prop, s); if (DEV) { tag(s, get_label(path, prop)); - owned_sources?.add(s); } } } else { has = s.v !== UNINITIALIZED; - var p = with_parent(() => proxy(value, DEV ? owned_sources : undefined)); + var p = with_parent(() => proxy(value)); set(s, p); } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 45582d4f9f65..7fb3135708d3 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -156,7 +156,7 @@ export function set(source, value, should_proxy = false) { let new_value = should_proxy ? proxy(value) : value; if (DEV) { - tag_proxy(new_value, /** @type {string} */ (source.label), source); + tag_proxy(new_value, /** @type {string} */ (source.label)); } return internal_set(source, new_value); From b8ad73faf609b72e8d81a3f33491dd31fed2fa78 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:14:19 -0700 Subject: [PATCH 03/10] add some logging --- packages/svelte/src/internal/client/proxy.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 1c72ebc16f51..c6676a28ebb9 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -96,6 +96,7 @@ export function proxy(value) { let updating = false; /** @param {string} new_path */ function update_path(new_path) { + console.log(new_path); if (updating) return; updating = true; path = new_path; From 713ae9eb9de0ae416a6c7a18587de37eaf7b8b74 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:17:37 -0700 Subject: [PATCH 04/10] make logging clearer --- packages/svelte/src/internal/client/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index c6676a28ebb9..fffa1b1d3099 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -96,7 +96,7 @@ export function proxy(value) { let updating = false; /** @param {string} new_path */ function update_path(new_path) { - console.log(new_path); + console.log({new_path}); if (updating) return; updating = true; path = new_path; From cb6c018ee09c8540545a94ad016858dade082a34 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:22:05 -0700 Subject: [PATCH 05/10] more --- packages/svelte/src/internal/client/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index fffa1b1d3099..b0581c255992 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -96,7 +96,7 @@ export function proxy(value) { let updating = false; /** @param {string} new_path */ function update_path(new_path) { - console.log({new_path}); + console.log({ new_path, updating, stack: new Error().stack }); if (updating) return; updating = true; path = new_path; From 2f4974c771b21f75d8d9561e3b218f31a7278ca5 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:40:28 -0700 Subject: [PATCH 06/10] try this --- packages/svelte/src/internal/client/proxy.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index b0581c255992..dae3791eb0c7 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -96,7 +96,6 @@ export function proxy(value) { let updating = false; /** @param {string} new_path */ function update_path(new_path) { - console.log({ new_path, updating, stack: new Error().stack }); if (updating) return; updating = true; path = new_path; @@ -288,13 +287,13 @@ export function proxy(value) { if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { s = with_parent(() => source(undefined, stack)); - set(s, proxy(value)); - - sources.set(prop, s); if (DEV) { tag(s, get_label(path, prop)); } + set(s, proxy(value)); + + sources.set(prop, s); } } else { has = s.v !== UNINITIALIZED; From a77f485f205b63b1a3cac0d8508179b5835221bc Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:48:25 -0700 Subject: [PATCH 07/10] add test --- .../_config.js | 22 +++++++++++++++++++ .../main.svelte | 12 ++++++++++ 2 files changed, 34 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js new file mode 100644 index 000000000000..fa671e5b2847 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js @@ -0,0 +1,22 @@ +import { test } from '../../test'; +import { normalise_trace_logs } from '../../../helpers.js'; + +export default test({ + compileOptions: { + dev: true + }, + + test({ assert, logs }) { + assert.deepEqual(normalise_trace_logs(logs), [ + { log: '$state', highlighted: true }, + { log: 'filesState.files', highlighted: false }, + { log: { id: 1, items: [{ id: 2, items: [{ id: 3 }, { id: 4 }] }] } }, + { log: '$state', highlighted: true }, + { log: 'filesState.files.items[0].parent', highlighted: false }, + { log: { id: 1, items: [{ id: 2, items: [{ id: 3 }, { id: 4 }] }] } }, + { log: '$state', highlighted: true }, + { log: 'filesState.files.items[0].parent.items[0]', highlighted: false }, + { log: { id: 2, items: [{ id: 3 }, { id: 4 }] } } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte new file mode 100644 index 000000000000..ba9fcb3e0974 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte @@ -0,0 +1,12 @@ + \ No newline at end of file From 62964abd364784d0e060ad8ae34cdabdacce3803 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:49:32 -0700 Subject: [PATCH 08/10] tweak --- .../samples/inspect-trace-circular-reference/main.svelte | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte index ba9fcb3e0974..7640d48f7705 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/main.svelte @@ -4,9 +4,7 @@ filesState.files = nodes; function test() { $inspect.trace(); - console.log('before assignment'); filesState.files.items[0].parent = filesState.files; - console.log('after assignment'); } $effect(test); \ No newline at end of file From e738d6ad2c0012ce456bb1cab15fc018b1521ad1 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:54:31 -0700 Subject: [PATCH 09/10] fix? --- .../_config.js | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js index fa671e5b2847..aed274fb764c 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js @@ -2,21 +2,25 @@ import { test } from '../../test'; import { normalise_trace_logs } from '../../../helpers.js'; export default test({ - compileOptions: { - dev: true - }, + compileOptions: { + dev: true + }, - test({ assert, logs }) { - assert.deepEqual(normalise_trace_logs(logs), [ - { log: '$state', highlighted: true }, - { log: 'filesState.files', highlighted: false }, - { log: { id: 1, items: [{ id: 2, items: [{ id: 3 }, { id: 4 }] }] } }, - { log: '$state', highlighted: true }, - { log: 'filesState.files.items[0].parent', highlighted: false }, - { log: { id: 1, items: [{ id: 2, items: [{ id: 3 }, { id: 4 }] }] } }, - { log: '$state', highlighted: true }, - { log: 'filesState.files.items[0].parent.items[0]', highlighted: false }, - { log: { id: 2, items: [{ id: 3 }, { id: 4 }] } } - ]); - } + test({ assert, logs }) { + const files = { id: 1, items: [{ id: 2, items: [{ id: 3 }, { id: 4 }] }] }; + // @ts-expect-error + files.items[0].parent = files; + assert.deepEqual(normalise_trace_logs(logs), [ + { log: 'test (main.svelte:5:4)' }, + { log: '$state', highlighted: true }, + { log: 'filesState.files', highlighted: false }, + { log: files }, + { log: '$state', highlighted: true }, + { log: 'filesState.files.items[0].parent', highlighted: false }, + { log: files }, + { log: '$state', highlighted: true }, + { log: 'filesState.files.items[0].parent.items[0]', highlighted: false }, + { log: files.items[0] } + ]); + } }); From 710274c78fb7c79f8dbf7ffbd0c66d723f2c8f49 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:58:46 -0700 Subject: [PATCH 10/10] fix?? --- .../samples/inspect-trace-circular-reference/_config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js index aed274fb764c..ca81c7854a29 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-circular-reference/_config.js @@ -16,8 +16,8 @@ export default test({ { log: 'filesState.files', highlighted: false }, { log: files }, { log: '$state', highlighted: true }, - { log: 'filesState.files.items[0].parent', highlighted: false }, - { log: files }, + { log: 'filesState.files.items[0].parent.items', highlighted: false }, + { log: files.items }, { log: '$state', highlighted: true }, { log: 'filesState.files.items[0].parent.items[0]', highlighted: false }, { log: files.items[0] }