Skip to content

Commit 6cf3a19

Browse files
7nikRich-Harris
andauthored
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 <[email protected]>
1 parent 8e73fd4 commit 6cf3a19

File tree

8 files changed

+110
-37
lines changed

8 files changed

+110
-37
lines changed

.changeset/curvy-houses-jog.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: better handle $inspect on array mutations

.changeset/metal-coats-thank.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: leave proxied array `length` untouched when deleting properties

packages/svelte/src/internal/client/proxy.js

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,13 @@ import {
1515
is_array,
1616
object_prototype
1717
} from '../shared/utils.js';
18-
import { state as source, set, increment } from './reactivity/sources.js';
18+
import {
19+
state as source,
20+
set,
21+
increment,
22+
flush_inspect_effects,
23+
set_inspect_effects_deferred
24+
} from './reactivity/sources.js';
1925
import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants';
2026
import { UNINITIALIZED } from '../../constants.js';
2127
import * as e from './errors.js';
@@ -80,6 +86,9 @@ export function proxy(value) {
8086
// We need to create the length source eagerly to ensure that
8187
// mutations to the array are properly synced with our proxy
8288
sources.set('length', source(/** @type {any[]} */ (value).length, stack));
89+
if (DEV) {
90+
value = /** @type {any} */ (inspectable_array(/** @type {any[]} */ (value)));
91+
}
8392
}
8493

8594
/** Used in dev for $inspect.trace() */
@@ -142,16 +151,6 @@ export function proxy(value) {
142151
}
143152
}
144153
} else {
145-
// When working with arrays, we need to also ensure we update the length when removing
146-
// an indexed property
147-
if (is_proxied_array && typeof prop === 'string') {
148-
var ls = /** @type {Source<number>} */ (sources.get('length'));
149-
var n = Number(prop);
150-
151-
if (Number.isInteger(n) && n < ls.v) {
152-
set(ls, n);
153-
}
154-
}
155154
set(s, UNINITIALIZED);
156155
increment(version);
157156
}
@@ -388,3 +387,42 @@ export function get_proxied_value(value) {
388387
export function is(a, b) {
389388
return Object.is(get_proxied_value(a), get_proxied_value(b));
390389
}
390+
391+
const ARRAY_MUTATING_METHODS = new Set([
392+
'copyWithin',
393+
'fill',
394+
'pop',
395+
'push',
396+
'reverse',
397+
'shift',
398+
'sort',
399+
'splice',
400+
'unshift'
401+
]);
402+
403+
/**
404+
* Wrap array mutating methods so $inspect is triggered only once and
405+
* to prevent logging an array in intermediate state (e.g. with an empty slot)
406+
* @param {any[]} array
407+
*/
408+
function inspectable_array(array) {
409+
return new Proxy(array, {
410+
get(target, prop, receiver) {
411+
var value = Reflect.get(target, prop, receiver);
412+
if (!ARRAY_MUTATING_METHODS.has(/** @type {string} */ (prop))) {
413+
return value;
414+
}
415+
416+
/**
417+
* @this {any[]}
418+
* @param {any[]} args
419+
*/
420+
return function (...args) {
421+
set_inspect_effects_deferred();
422+
var result = value.apply(this, args);
423+
flush_inspect_effects();
424+
return result;
425+
};
426+
}
427+
});
428+
}

packages/svelte/src/internal/client/reactivity/sources.js

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ export function set_inspect_effects(v) {
4949
inspect_effects = v;
5050
}
5151

52+
let inspect_effects_deferred = false;
53+
54+
export function set_inspect_effects_deferred() {
55+
inspect_effects_deferred = true;
56+
}
57+
5258
/**
5359
* @template V
5460
* @param {V} v
@@ -213,26 +219,32 @@ export function internal_set(source, value) {
213219
}
214220
}
215221

216-
if (DEV && inspect_effects.size > 0) {
217-
const inspects = Array.from(inspect_effects);
222+
if (DEV && inspect_effects.size > 0 && !inspect_effects_deferred) {
223+
flush_inspect_effects();
224+
}
225+
}
226+
227+
return value;
228+
}
218229

219-
for (const effect of inspects) {
220-
// Mark clean inspect-effects as maybe dirty and then check their dirtiness
221-
// instead of just updating the effects - this way we avoid overfiring.
222-
if ((effect.f & CLEAN) !== 0) {
223-
set_signal_status(effect, MAYBE_DIRTY);
224-
}
230+
export function flush_inspect_effects() {
231+
inspect_effects_deferred = false;
225232

226-
if (is_dirty(effect)) {
227-
update_effect(effect);
228-
}
229-
}
233+
const inspects = Array.from(inspect_effects);
230234

231-
inspect_effects.clear();
235+
for (const effect of inspects) {
236+
// Mark clean inspect-effects as maybe dirty and then check their dirtiness
237+
// instead of just updating the effects - this way we avoid overfiring.
238+
if ((effect.f & CLEAN) !== 0) {
239+
set_signal_status(effect, MAYBE_DIRTY);
240+
}
241+
242+
if (is_dirty(effect)) {
243+
update_effect(effect);
232244
}
233245
}
234246

235-
return value;
247+
inspect_effects.clear();
236248
}
237249

238250
/**
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { ok, test } from '../../test';
2+
import { flushSync } from 'svelte';
3+
4+
export default test({
5+
mode: ['client'],
6+
async test({ target, assert, logs }) {
7+
const btn = target.querySelector('button');
8+
9+
flushSync(() => btn?.click());
10+
11+
assert.deepEqual(logs[0], [0, , 2]);
12+
}
13+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
const arr = [0, 1, 2];
3+
</script>
4+
5+
<button onclick={() => {
6+
delete arr[1];
7+
console.log(arr);
8+
}}>del</button>

packages/svelte/tests/runtime-runes/samples/inspect-deep-array/_config.js

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,6 @@ export default test({
1313
button?.click();
1414
});
1515

16-
assert.deepEqual(logs, [
17-
'init',
18-
[1, 2, 3, 7],
19-
'update',
20-
[2, 2, 3, 7],
21-
'update',
22-
[2, 3, 3, 7],
23-
'update',
24-
[2, 3, 7, 7],
25-
'update',
26-
[2, 3, 7]
27-
]);
16+
assert.deepEqual(logs, ['init', [1, 2, 3, 7], 'update', [2, 3, 7]]);
2817
}
2918
});

packages/svelte/tests/suite.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const filter = process.env.FILTER
2020
)
2121
: /./;
2222

23+
// this defaults to 10, which is too low for some of our tests
24+
Error.stackTraceLimit = 100;
25+
2326
export function suite<Test extends BaseTest>(fn: (config: Test, test_dir: string) => void) {
2427
return {
2528
test: (config: Test) => config,

0 commit comments

Comments
 (0)