From 6cf3a193428f08a1c15b403dad681627425c0f1d Mon Sep 17 00:00:00 2001 From: 7nik Date: Wed, 16 Jul 2025 16:55:15 +0300 Subject: [PATCH] fix: better handle $inspect on array mutations (#16389) * fix: better handle $inspect on array mutations * increase stack trace limit, revert test change * second changeset --------- Co-authored-by: Rich Harris --- .changeset/curvy-houses-jog.md | 5 ++ .changeset/metal-coats-thank.md | 5 ++ packages/svelte/src/internal/client/proxy.js | 60 +++++++++++++++---- .../src/internal/client/reactivity/sources.js | 40 ++++++++----- .../samples/array-delete-item/_config.js | 13 ++++ .../samples/array-delete-item/main.svelte | 8 +++ .../samples/inspect-deep-array/_config.js | 13 +--- packages/svelte/tests/suite.ts | 3 + 8 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 .changeset/curvy-houses-jog.md create mode 100644 .changeset/metal-coats-thank.md create mode 100644 packages/svelte/tests/runtime-runes/samples/array-delete-item/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/array-delete-item/main.svelte diff --git a/.changeset/curvy-houses-jog.md b/.changeset/curvy-houses-jog.md new file mode 100644 index 0000000000..0a2323d8e4 --- /dev/null +++ b/.changeset/curvy-houses-jog.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: better handle $inspect on array mutations diff --git a/.changeset/metal-coats-thank.md b/.changeset/metal-coats-thank.md new file mode 100644 index 0000000000..b307fd3d45 --- /dev/null +++ b/.changeset/metal-coats-thank.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: leave proxied array `length` untouched when deleting properties diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 5da1b7e188..3ae4b87ed5 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -15,7 +15,13 @@ import { is_array, object_prototype } from '../shared/utils.js'; -import { state as source, set, increment } from './reactivity/sources.js'; +import { + state as source, + set, + increment, + flush_inspect_effects, + set_inspect_effects_deferred +} from './reactivity/sources.js'; import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; @@ -80,6 +86,9 @@ export function proxy(value) { // We need to create the length source eagerly to ensure that // mutations to the array are properly synced with our proxy sources.set('length', source(/** @type {any[]} */ (value).length, stack)); + if (DEV) { + value = /** @type {any} */ (inspectable_array(/** @type {any[]} */ (value))); + } } /** Used in dev for $inspect.trace() */ @@ -142,16 +151,6 @@ export function proxy(value) { } } } else { - // When working with arrays, we need to also ensure we update the length when removing - // an indexed property - if (is_proxied_array && typeof prop === 'string') { - var ls = /** @type {Source} */ (sources.get('length')); - var n = Number(prop); - - if (Number.isInteger(n) && n < ls.v) { - set(ls, n); - } - } set(s, UNINITIALIZED); increment(version); } @@ -388,3 +387,42 @@ export function get_proxied_value(value) { export function is(a, b) { return Object.is(get_proxied_value(a), get_proxied_value(b)); } + +const ARRAY_MUTATING_METHODS = new Set([ + 'copyWithin', + 'fill', + 'pop', + 'push', + 'reverse', + 'shift', + 'sort', + 'splice', + 'unshift' +]); + +/** + * Wrap array mutating methods so $inspect is triggered only once and + * to prevent logging an array in intermediate state (e.g. with an empty slot) + * @param {any[]} array + */ +function inspectable_array(array) { + return new Proxy(array, { + get(target, prop, receiver) { + var value = Reflect.get(target, prop, receiver); + if (!ARRAY_MUTATING_METHODS.has(/** @type {string} */ (prop))) { + return value; + } + + /** + * @this {any[]} + * @param {any[]} args + */ + return function (...args) { + set_inspect_effects_deferred(); + var result = value.apply(this, args); + flush_inspect_effects(); + return result; + }; + } + }); +} diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 29c657bdd0..bd55b9d935 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -49,6 +49,12 @@ export function set_inspect_effects(v) { inspect_effects = v; } +let inspect_effects_deferred = false; + +export function set_inspect_effects_deferred() { + inspect_effects_deferred = true; +} + /** * @template V * @param {V} v @@ -213,26 +219,32 @@ export function internal_set(source, value) { } } - if (DEV && inspect_effects.size > 0) { - const inspects = Array.from(inspect_effects); + if (DEV && inspect_effects.size > 0 && !inspect_effects_deferred) { + flush_inspect_effects(); + } + } + + return value; +} - for (const effect of inspects) { - // Mark clean inspect-effects as maybe dirty and then check their dirtiness - // instead of just updating the effects - this way we avoid overfiring. - if ((effect.f & CLEAN) !== 0) { - set_signal_status(effect, MAYBE_DIRTY); - } +export function flush_inspect_effects() { + inspect_effects_deferred = false; - if (is_dirty(effect)) { - update_effect(effect); - } - } + const inspects = Array.from(inspect_effects); - inspect_effects.clear(); + for (const effect of inspects) { + // Mark clean inspect-effects as maybe dirty and then check their dirtiness + // instead of just updating the effects - this way we avoid overfiring. + if ((effect.f & CLEAN) !== 0) { + set_signal_status(effect, MAYBE_DIRTY); + } + + if (is_dirty(effect)) { + update_effect(effect); } } - return value; + inspect_effects.clear(); } /** diff --git a/packages/svelte/tests/runtime-runes/samples/array-delete-item/_config.js b/packages/svelte/tests/runtime-runes/samples/array-delete-item/_config.js new file mode 100644 index 0000000000..7134117067 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/array-delete-item/_config.js @@ -0,0 +1,13 @@ +import { ok, test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + mode: ['client'], + async test({ target, assert, logs }) { + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + + assert.deepEqual(logs[0], [0, , 2]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/array-delete-item/main.svelte b/packages/svelte/tests/runtime-runes/samples/array-delete-item/main.svelte new file mode 100644 index 0000000000..ca00a85491 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/array-delete-item/main.svelte @@ -0,0 +1,8 @@ + + + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-deep-array/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-deep-array/_config.js index 0e6b12508b..49f1b5de41 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-deep-array/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-deep-array/_config.js @@ -13,17 +13,6 @@ export default test({ button?.click(); }); - assert.deepEqual(logs, [ - 'init', - [1, 2, 3, 7], - 'update', - [2, 2, 3, 7], - 'update', - [2, 3, 3, 7], - 'update', - [2, 3, 7, 7], - 'update', - [2, 3, 7] - ]); + assert.deepEqual(logs, ['init', [1, 2, 3, 7], 'update', [2, 3, 7]]); } }); diff --git a/packages/svelte/tests/suite.ts b/packages/svelte/tests/suite.ts index 6954b8b683..bbd252b8e1 100644 --- a/packages/svelte/tests/suite.ts +++ b/packages/svelte/tests/suite.ts @@ -20,6 +20,9 @@ const filter = process.env.FILTER ) : /./; +// this defaults to 10, which is too low for some of our tests +Error.stackTraceLimit = 100; + export function suite(fn: (config: Test, test_dir: string) => void) { return { test: (config: Test) => config,