From 316a341c79057854d4ec5bd98e69174b1fd632f5 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 23 Jan 2025 23:05:19 +0100 Subject: [PATCH] fix: make it work properly with reassigned references --- packages/svelte/src/internal/client/proxy.js | 99 ++++++++++++++----- .../_config.js | 12 ++- .../main.svelte | 12 +++ 3 files changed, 99 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 406b279df3..81dbbfda23 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -1,6 +1,7 @@ /** @import { ProxyMetadata, ProxyStateObject, Source, ValueOptions } from '#client' */ import { DEV } from 'esm-env'; -import { get, component_context, active_effect } from './runtime.js'; +import { UNINITIALIZED } from '../../constants.js'; +import { tracing_mode_flag } from '../flags/index.js'; import { array_prototype, get_descriptor, @@ -8,16 +9,27 @@ import { is_array, object_prototype } from '../shared/utils.js'; -import { check_ownership, widen_ownership } from './dev/ownership.js'; -import { source, set, state, batch_onchange } from './reactivity/sources.js'; import { PROXY_ONCHANGE_SYMBOL, STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js'; -import { UNINITIALIZED } from '../../constants.js'; -import * as e from './errors.js'; +import { check_ownership, widen_ownership } from './dev/ownership.js'; import { get_stack } from './dev/tracing.js'; -import { tracing_mode_flag } from '../flags/index.js'; +import * as e from './errors.js'; +import { batch_onchange, set, source, state } from './reactivity/sources.js'; +import { active_effect, component_context, get } from './runtime.js'; const array_methods = ['push', 'pop', 'shift', 'unshift', 'splice', 'reverse', 'sort']; +/** + * @param {ValueOptions | undefined} options + * @returns {ValueOptions | undefined} + */ +function clone_options(options) { + return options != null + ? { + onchange: options.onchange + } + : undefined; +} + /** * @template T * @param {T} value @@ -39,10 +51,21 @@ export function proxy(value, options, parent = null, prev) { if (STATE_SYMBOL in value) { // @ts-ignore - value[PROXY_ONCHANGE_SYMBOL] = options?.onchange; + value[PROXY_ONCHANGE_SYMBOL](options?.onchange); return value; } + if (options?.onchange) { + // if there's an onchange we actually store that but override the value + // to store every other onchange that new proxies might add + var onchanges = new Set([options.onchange]); + options.onchange = () => { + for (let onchange of onchanges) { + onchange(); + } + }; + } + const prototype = get_prototype_of(value); if (prototype !== object_prototype && prototype !== array_prototype) { @@ -103,7 +126,7 @@ export function proxy(value, options, parent = null, prev) { var s = sources.get(prop); if (s === undefined) { - s = source(descriptor.value, options, stack); + s = source(descriptor.value, clone_options(options), stack); sources.set(prop, s); } else { set(s, proxy(descriptor.value, options, metadata)); @@ -117,7 +140,7 @@ export function proxy(value, options, parent = null, prev) { if (s === undefined) { if (prop in target) { - sources.set(prop, source(UNINITIALIZED, options, stack)); + sources.set(prop, source(UNINITIALIZED, clone_options(options), stack)); } } else { // When working with arrays, we need to also ensure we update the length when removing @@ -130,6 +153,11 @@ export function proxy(value, options, parent = null, prev) { set(ls, n); } } + // when we delete a property if the source is a proxy we remove the current onchange from + // the proxy `onchanges` so that it doesn't trigger it anymore + if (typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) { + s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + } set(s, UNINITIALIZED); update_version(version); } @@ -146,12 +174,38 @@ export function proxy(value, options, parent = null, prev) { return value; } + if (prop === PROXY_ONCHANGE_SYMBOL) { + return ( + /** @type {(() => unknown) | undefined} */ value, + /** @type {boolean} */ remove + ) => { + // we either add or remove the passed in value + // to the onchanges array or we set every source onchange + // to the passed in value (if it's undefined it will make the chain stop) + if (options?.onchange != null && value && !remove) { + onchanges.add(value); + } else if (options?.onchange != null && value) { + onchanges.delete(value); + } else { + options = { + onchange: value + }; + for (let [, s] of sources) { + if (s.o) { + s.o.onchange = value; + } + } + } + }; + } + var s = sources.get(prop); var exists = prop in target; // 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 = source(proxy(exists ? target[prop] : UNINITIALIZED, options, metadata), options, stack); + let opt = clone_options(options); + s = source(proxy(exists ? target[prop] : UNINITIALIZED, opt, metadata), opt, stack); sources.set(prop, s); } @@ -229,7 +283,8 @@ export function proxy(value, options, parent = null, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = source(has ? proxy(target[prop], options, metadata) : UNINITIALIZED, options, stack); + let opt = clone_options(options); + s = source(has ? proxy(target[prop], opt, metadata) : UNINITIALIZED, opt, stack); sources.set(prop, s); } @@ -243,14 +298,6 @@ export function proxy(value, options, parent = null, prev) { }, set(target, prop, value, receiver) { - if (prop === PROXY_ONCHANGE_SYMBOL && options?.onchange != null) { - const old_onchange = options.onchange; - options.onchange = () => { - old_onchange(); - value(); - }; - return true; - } var s = sources.get(prop); var has = prop in target; @@ -264,7 +311,7 @@ export function proxy(value, options, parent = null, prev) { // 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, options, stack); + other_s = source(UNINITIALIZED, clone_options(options), stack); sources.set(i + '', other_s); } } @@ -276,13 +323,19 @@ export function proxy(value, options, parent = null, prev) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - s = source(undefined, options, stack); - set(s, proxy(value, options, metadata)); + const opt = clone_options(options); + s = source(undefined, opt, stack); + set(s, proxy(value, opt, metadata)); sources.set(prop, s); } } else { has = s.v !== UNINITIALIZED; - set(s, proxy(value, options, metadata)); + // when we set a property if the source is a proxy we remove the current onchange from + // the proxy `onchanges` so that it doesn't trigger it anymore + if (typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) { + s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + } + set(s, proxy(value, clone_options(options), metadata)); } if (DEV) { diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js index 27f3a73187..cc0bfbb1e3 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js @@ -3,7 +3,7 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { - const [btn, btn2, btn3, btn4] = target.querySelectorAll('button'); + const [btn, btn2, btn3, btn4, btn5, btn6] = target.querySelectorAll('button'); logs.length = 0; flushSync(() => { @@ -18,6 +18,16 @@ export default test({ flushSync(() => { btn4.click(); }); + flushSync(() => { + btn5.click(); + }); assert.deepEqual(logs, []); + flushSync(() => { + btn6.click(); + }); + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['arr', 'arr']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte index 3a039bbd23..4d586c7707 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte @@ -23,9 +23,21 @@ const values = [...Object.values(obj)]; delete obj.key; + + let arr_2 = $state([{ count: 1 }, { count: 1 }], { + onchange(){ + console.log("arr_2"); + } + }); + + const item_4 = arr_2[0]; + + arr_2.length = 0; + +