Skip to content
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

fix: properly handle proxied array length mutations #13026

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/khaki-donkeys-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: properly handle proxied array length mutations
42 changes: 27 additions & 15 deletions packages/svelte/src/internal/client/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,17 @@ export function proxy(value, parent = null, prev) {
return value;
}

/** @type {Map<any, Source<any>>} */
var sources = new Map();
var is_proxied_array = is_array(value);
var version = source(0);

if (is_proxied_array) {
// 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));
}

/** @type {ProxyMetadata} */
var metadata;

Expand Down Expand Up @@ -187,6 +194,22 @@ export function proxy(value, parent = null, prev) {
var s = sources.get(prop);
var has = prop in target;

// variable.length = value -> clear all signals with index >= value
if (is_proxied_array && prop === 'length') {
for (var i = value; i < /** @type {Source<number>} */ (s).v; i += 1) {
var other_s = sources.get(i + '');
if (other_s !== undefined) {
set(other_s, UNINITIALIZED);
} else if (i in target) {
// If the item exists in the original, we need to create a uninitialized source,
// else a later read of the property would result in a source being created with
// the value of the original item at that index.
other_s = source(UNINITIALIZED);
sources.set(i + '', other_s);
}
}
}

// If we haven't yet created a source for this property, we need to ensure
// we do so otherwise if we read it later, then the write won't be tracked and
// the heuristics of effects will be different vs if we had read the proxied
Expand All @@ -211,14 +234,6 @@ export function proxy(value, parent = null, prev) {
check_ownership(metadata);
}

// variable.length = value -> clear all signals with index >= value
if (is_proxied_array && prop === 'length') {
for (var i = value; i < target.length; i += 1) {
var other_s = sources.get(i + '');
if (other_s !== undefined) set(other_s, UNINITIALIZED);
}
}

var descriptor = Reflect.getOwnPropertyDescriptor(target, prop);

// Set the new value before updating any signals so that any listeners get the new value
Expand All @@ -232,14 +247,11 @@ export function proxy(value, parent = null, prev) {
// to ensure that iterating over the array as a result of a metadata update
// will not cause the length to be out of sync.
if (is_proxied_array && typeof prop === 'string') {
var ls = sources.get('length');

if (ls !== undefined) {
var n = Number(prop);
var ls = /** @type {Source<number>} */ (sources.get('length'));
var n = Number(prop);

if (Number.isInteger(n) && n >= ls.v) {
set(ls, n + 1);
}
if (Number.isInteger(n) && n >= ls.v) {
set(ls, n + 1);
}
}

Expand Down
35 changes: 35 additions & 0 deletions packages/svelte/src/internal/client/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,38 @@ test('deletes a property', () => {
// deleting a non-existent property should succeed
delete state.c;
});

test('handles array.push', () => {
const original = [1, 2, 3];
const state = proxy(original);

state.push(4);
assert.deepEqual(original.length, 3);
assert.deepEqual(original, [1, 2, 3]);
assert.deepEqual(state.length, 4);
assert.deepEqual(state, [1, 2, 3, 4]);
});

test('handles array mutation', () => {
const original = [1, 2, 3];
const state = proxy(original);

state[3] = 4;
assert.deepEqual(original.length, 3);
assert.deepEqual(original, [1, 2, 3]);
assert.deepEqual(state.length, 4);
assert.deepEqual(state, [1, 2, 3, 4]);
});

test('handles array length mutation', () => {
const original = [1, 2, 3];
const state = proxy(original);

state.length = 0;
assert.deepEqual(original.length, 3);
assert.deepEqual(original, [1, 2, 3]);
assert.deepEqual(original[0], 1);
assert.deepEqual(state.length, 0);
assert.deepEqual(state, []);
assert.deepEqual(state[0], undefined);
});
Loading