From d7876af34bd25a9b92cd30e1f63a45b7689a2365 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 20 Jan 2025 22:57:35 +0100 Subject: [PATCH 01/50] feat: add `onchange` option to `$state` --- .changeset/red-rules-share.md | 5 +++ packages/svelte/src/ambient.d.ts | 1 + .../2-analyze/visitors/CallExpression.js | 4 +- .../3-transform/client/transform-client.js | 5 ++- .../phases/3-transform/client/utils.js | 4 +- .../3-transform/client/visitors/ClassBody.js | 23 +++++++++- .../client/visitors/VariableDeclaration.js | 21 ++++++--- packages/svelte/src/index.d.ts | 2 + packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/client/proxy.js | 25 ++++++----- .../src/internal/client/reactivity/sources.js | 22 +++++++--- .../src/internal/client/reactivity/types.d.ts | 6 +++ .../svelte/src/internal/client/types.d.ts | 2 +- .../samples/runes-wrong-state-args/_config.js | 2 +- .../samples/state-onchange/_config.js | 44 +++++++++++++++++++ .../samples/state-onchange/main.svelte | 36 +++++++++++++++ .../_expected/client/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 4 +- .../_expected/client/index.svelte.js | 2 +- packages/svelte/types/index.d.ts | 10 +++-- 21 files changed, 185 insertions(+), 39 deletions(-) create mode 100644 .changeset/red-rules-share.md create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte diff --git a/.changeset/red-rules-share.md b/.changeset/red-rules-share.md new file mode 100644 index 000000000000..2a4d29b7985e --- /dev/null +++ b/.changeset/red-rules-share.md @@ -0,0 +1,5 @@ +--- +'svelte': minor +--- + +feat: add `onchange` option to `$state` diff --git a/packages/svelte/src/ambient.d.ts b/packages/svelte/src/ambient.d.ts index fbcecba8e47c..211064c535da 100644 --- a/packages/svelte/src/ambient.d.ts +++ b/packages/svelte/src/ambient.d.ts @@ -20,6 +20,7 @@ declare module '*.svelte' { * * @param initial The initial value */ +declare function $state(initial?: T, options?: import('svelte').StateOptions): T; declare function $state(initial: T): T; declare function $state(): T | undefined; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 9f51cd61de6d..2eff18cd51ae 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -87,8 +87,8 @@ export function CallExpression(node, context) { if ((rune === '$derived' || rune === '$derived.by') && node.arguments.length !== 1) { e.rune_invalid_arguments_length(node, rune, 'exactly one argument'); - } else if (rune === '$state' && node.arguments.length > 1) { - e.rune_invalid_arguments_length(node, rune, 'zero or one arguments'); + } else if (rune === '$state' && node.arguments.length > 2) { + e.rune_invalid_arguments_length(node, rune, 'zero, one or two arguments'); } break; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index f4a6c9a4147b..e7ac6a9653c9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -292,7 +292,10 @@ export function client_component(analysis, options) { } if (binding?.kind === 'state' || binding?.kind === 'raw_state') { - const value = binding.kind === 'state' ? b.call('$.proxy', b.id('$$value')) : b.id('$$value'); + const value = + binding.kind === 'state' + ? b.call('$.proxy', b.id('$$value'), b.call('$.get_options', b.id(name))) + : b.id('$$value'); return [getter, b.set(alias ?? name, [b.stmt(b.call('$.set', b.id(name), value))])]; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index c59a5544dfb2..4256b84962a7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -50,7 +50,9 @@ export function build_getter(node, state) { * @param {Expression} previous */ export function build_proxy_reassignment(value, previous) { - return dev ? b.call('$.proxy', value, b.null, previous) : b.call('$.proxy', value); + return dev + ? b.call('$.proxy', value, b.call('$.get_options', previous), b.null, previous) + : b.call('$.proxy', value, b.call('$.get_options', previous)); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 7b3a9a4d0e29..1ade3508985a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -116,11 +116,32 @@ export function ClassBody(node, context) { context.visit(definition.value.arguments[0], child_state) ); + let options = + definition.value.arguments.length === 2 + ? /** @type {Expression} **/ ( + context.visit(definition.value.arguments[1], child_state) + ) + : undefined; + + let proxied = should_proxy(init, context.state.scope); + + if (field.kind === 'state' && proxied && options != null) { + let generated = 'state_options'; + let i = 0; + while (private_ids.includes(generated)) { + generated = `state_options_${i++}`; + } + private_ids.push(generated); + body.push(b.prop_def(b.private_id(generated), options)); + options = b.member(b.this, `#${generated}`); + } + value = field.kind === 'state' ? b.call( '$.state', - should_proxy(init, context.state.scope) ? b.call('$.proxy', init) : init + should_proxy(init, context.state.scope) ? b.call('$.proxy', init, options) : init, + options ) : field.kind === 'raw_state' ? b.call('$.state', init) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index afb90bbec7f9..6c266fde210b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -113,28 +113,37 @@ export function VariableDeclaration(node, context) { const args = /** @type {CallExpression} */ (init).arguments; const value = args.length === 0 ? b.id('undefined') : /** @type {Expression} */ (context.visit(args[0])); + let options = + args.length === 2 ? /** @type {Expression} */ (context.visit(args[1])) : undefined; if (rune === '$state' || rune === '$state.raw') { /** * @param {Identifier} id * @param {Expression} value + * @param {Expression} [options] */ - const create_state_declarator = (id, value) => { + const create_state_declarator = (id, value, options) => { const binding = /** @type {import('#compiler').Binding} */ ( context.state.scope.get(id.name) ); - if (rune === '$state' && should_proxy(value, context.state.scope)) { - value = b.call('$.proxy', value); + const proxied = rune === '$state' && should_proxy(value, context.state.scope); + if (proxied) { + if (options != null) { + const generated = context.state.scope.generate('state_options'); + declarations.push(b.declarator(generated, options)); + options = b.id(generated); + } + value = b.call('$.proxy', value, options); } if (is_state_source(binding, context.state.analysis)) { - value = b.call('$.state', value); + value = b.call('$.state', value, options); } return value; }; if (declarator.id.type === 'Identifier') { declarations.push( - b.declarator(declarator.id, create_state_declarator(declarator.id, value)) + b.declarator(declarator.id, create_state_declarator(declarator.id, value, options)) ); } else { const tmp = context.state.scope.generate('tmp'); @@ -147,7 +156,7 @@ export function VariableDeclaration(node, context) { return b.declarator( path.node, binding?.kind === 'state' || binding?.kind === 'raw_state' - ? create_state_declarator(binding.node, value) + ? create_state_declarator(binding.node, value, options) : value ); }) diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index 554510542e2e..f94e6b5425b5 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -351,4 +351,6 @@ export type MountOptions = Record props: Props; }); +export { ValueOptions as StateOptions } from './internal/client/types.js'; + export * from './index-client.js'; diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 2bf58c51f75d..0251b8e47b20 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -109,7 +109,7 @@ export { user_effect, user_pre_effect } from './reactivity/effects.js'; -export { mutable_state, mutate, set, state } from './reactivity/sources.js'; +export { mutable_state, mutate, set, state, get_options } from './reactivity/sources.js'; export { prop, rest_props, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 6cbd6394df3a..9a2e88cc8533 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -1,4 +1,4 @@ -/** @import { ProxyMetadata, ProxyStateObject, Source } from '#client' */ +/** @import { ProxyMetadata, ProxyStateObject, Source, ValueOptions } from '#client' */ import { DEV } from 'esm-env'; import { get, component_context, active_effect } from './runtime.js'; import { @@ -19,11 +19,12 @@ import { tracing_mode_flag } from '../flags/index.js'; /** * @template T * @param {T} value + * @param {ValueOptions} [options] * @param {ProxyMetadata | null} [parent] * @param {Source} [prev] dev mode only * @returns {T} */ -export function proxy(value, parent = null, prev) { +export function proxy(value, options, parent = null, prev) { /** @type {Error | null} */ var stack = null; if (DEV && tracing_mode_flag) { @@ -48,7 +49,7 @@ export function proxy(value, parent = null, prev) { 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, stack)); + sources.set('length', source(/** @type {any[]} */ (value).length, options, stack)); } /** @type {ProxyMetadata} */ @@ -94,10 +95,10 @@ export function proxy(value, parent = null, prev) { var s = sources.get(prop); if (s === undefined) { - s = source(descriptor.value, stack); + s = source(descriptor.value, options, stack); sources.set(prop, s); } else { - set(s, proxy(descriptor.value, metadata)); + set(s, proxy(descriptor.value, options, metadata)); } return true; @@ -108,7 +109,7 @@ export function proxy(value, parent = null, prev) { if (s === undefined) { if (prop in target) { - sources.set(prop, source(UNINITIALIZED, stack)); + sources.set(prop, source(UNINITIALIZED, options, stack)); } } else { // When working with arrays, we need to also ensure we update the length when removing @@ -142,7 +143,7 @@ export function proxy(value, parent = null, prev) { // 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, metadata), stack); + s = source(proxy(exists ? target[prop] : UNINITIALIZED, options, metadata), options, stack); sources.set(prop, s); } @@ -210,7 +211,7 @@ export function proxy(value, parent = null, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = source(has ? proxy(target[prop], metadata) : UNINITIALIZED, stack); + s = source(has ? proxy(target[prop], options, metadata) : UNINITIALIZED, options, stack); sources.set(prop, s); } @@ -237,7 +238,7 @@ export function proxy(value, 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, stack); + other_s = source(UNINITIALIZED, options, stack); sources.set(i + '', other_s); } } @@ -249,13 +250,13 @@ export function proxy(value, parent = null, prev) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - s = source(undefined, stack); - set(s, proxy(value, metadata)); + s = source(undefined, options, stack); + set(s, proxy(value, options, metadata)); sources.set(prop, s); } } else { has = s.v !== UNINITIALIZED; - set(s, proxy(value, metadata)); + set(s, proxy(value, options, metadata)); } if (DEV) { diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 4500a7c5a84a..4cd9da7e3caf 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -1,4 +1,4 @@ -/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */ +/** @import { Derived, Effect, Reaction, Source, Value, ValueOptions } from '#client' */ import { DEV } from 'esm-env'; import { component_context, @@ -47,10 +47,11 @@ export function set_inspect_effects(v) { /** * @template V * @param {V} v + * @param {ValueOptions} [o] * @param {Error | null} [stack] * @returns {Source} */ -export function source(v, stack) { +export function source(v, o, stack) { /** @type {Value} */ var signal = { f: 0, // TODO ideally we could skip this altogether, but it causes type errors @@ -58,7 +59,8 @@ export function source(v, stack) { reactions: null, equals, rv: 0, - wv: 0 + wv: 0, + o }; if (DEV && tracing_mode_flag) { @@ -72,9 +74,18 @@ export function source(v, stack) { /** * @template V * @param {V} v + * @param {ValueOptions} [o] */ -export function state(v) { - return push_derived_source(source(v)); +export function state(v, o) { + return push_derived_source(source(v, o)); +} + +/** + * @param {Source} source + * @returns {ValueOptions | undefined} + */ +export function get_options(source) { + return source.o; } /** @@ -171,6 +182,7 @@ export function internal_set(source, value) { var old_value = source.v; source.v = value; source.wv = increment_write_version(); + untrack(() => source.o?.onchange?.()); if (DEV && tracing_mode_flag) { source.updated = get_stack('UpdatedAt'); diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 3a76a3ff836c..0b1492e4dd9b 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -7,6 +7,10 @@ export interface Signal { wv: number; } +export interface ValueOptions { + onchange?: () => unknown; +} + export interface Value extends Signal { /** Equality function */ equals: Equals; @@ -16,6 +20,8 @@ export interface Value extends Signal { rv: number; /** The latest value for this signal */ v: V; + /** Options for the source */ + o?: ValueOptions; /** Dev only */ created?: Error | null; updated?: Error | null; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 7208ed77837e..e7ce4137fb3e 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -1,6 +1,6 @@ import type { Store } from '#shared'; import { STATE_SYMBOL } from './constants.js'; -import type { Effect, Source, Value, Reaction } from './reactivity/types.js'; +import type { Effect, Source, Value, Reaction, ValueOptions } from './reactivity/types.js'; type EventCallback = (event: Event) => boolean; export type EventCallbackMap = Record; diff --git a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js index 993ca18f4765..bd8349d9b866 100644 --- a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js +++ b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js @@ -3,6 +3,6 @@ import { test } from '../../test'; export default test({ error: { code: 'rune_invalid_arguments_length', - message: '`$state` must be called with zero or one arguments' + message: '`$state` must be called with zero, one or two arguments' } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js new file mode 100644 index 000000000000..4864e64cd61b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js @@ -0,0 +1,44 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3, btn4, btn5, btn6] = target.querySelectorAll('button'); + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['count']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['count', 'proxy']); + + flushSync(() => { + btn3.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'proxy']); + + flushSync(() => { + btn4.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'proxy', 'class count']); + + flushSync(() => { + btn5.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'proxy', 'class count', 'class proxy']); + + flushSync(() => { + btn6.click(); + }); + assert.deepEqual(logs, [ + 'count', + 'proxy', + 'proxy', + 'class count', + 'class proxy', + 'class proxy' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte new file mode 100644 index 000000000000..46253488d20a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte @@ -0,0 +1,36 @@ + + + + + + + + + \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js index fa990b33ee56..df765613aa41 100644 --- a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js @@ -23,7 +23,7 @@ export default function Bind_component_snippet($$anchor) { return $.get(value); }, set value($$value) { - $.set(value, $.proxy($$value)); + $.set(value, $.proxy($$value, $.get_options(value))); } }); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index 2898f31a6fb5..b6d0f002a76b 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -12,7 +12,7 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro } set a(value) { - $.set(this.#a, $.proxy(value)); + $.set(this.#a, $.proxy(value, $.get_options(this.#a))); } #b = $.state(); diff --git a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js index 9651713c52f5..d791efe6460b 100644 --- a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js @@ -8,8 +8,8 @@ let d = 4; export function update(array) { ( - $.set(a, $.proxy(array[0])), - $.set(b, $.proxy(array[1])) + $.set(a, $.proxy(array[0], $.get_options(a))), + $.set(b, $.proxy(array[1], $.get_options(b))) ); [c, d] = array; diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index c545608bcacf..aca73406bda2 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -13,7 +13,7 @@ export default function Function_prop_no_getter($$anchor) { Button($$anchor, { onmousedown: () => $.set(count, $.get(count) + 1), onmouseup, - onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)))), + onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)), $.get_options(count))), children: ($$anchor, $$slotProps) => { $.next(); diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index d00b2b01ed18..1f9f94646641 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -412,6 +412,12 @@ declare module 'svelte' { * Synchronously flushes any pending state changes and those that result from it. * */ export function flushSync(fn?: (() => void) | undefined): void; + type Getters = { + [K in keyof T]: () => T[K]; + }; + export interface StateOptions { + onchange?: () => unknown; + } /** * Create a snippet programmatically * */ @@ -512,9 +518,6 @@ declare module 'svelte' { * * */ export function getAllContexts = Map>(): T; - type Getters = { - [K in keyof T]: () => T[K]; - }; export {}; } @@ -2676,6 +2679,7 @@ declare module 'svelte/types/compiler/interfaces' { * * @param initial The initial value */ +declare function $state(initial?: T, options?: import('svelte').StateOptions): T; declare function $state(initial: T): T; declare function $state(): T | undefined; From 82d45a203e580c783c7ba3d0771a2a4d3f2e3f70 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 09:44:42 +0100 Subject: [PATCH 02/50] fix: create `assignable_proxy` utils to prevent declaring an external const --- .../3-transform/client/visitors/ClassBody.js | 19 +++---------------- .../client/visitors/VariableDeclaration.js | 13 +++++-------- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/client/proxy.js | 13 ++++++++++++- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 1ade3508985a..8738c397b75d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -125,24 +125,11 @@ export function ClassBody(node, context) { let proxied = should_proxy(init, context.state.scope); - if (field.kind === 'state' && proxied && options != null) { - let generated = 'state_options'; - let i = 0; - while (private_ids.includes(generated)) { - generated = `state_options_${i++}`; - } - private_ids.push(generated); - body.push(b.prop_def(b.private_id(generated), options)); - options = b.member(b.this, `#${generated}`); - } - value = field.kind === 'state' - ? b.call( - '$.state', - should_proxy(init, context.state.scope) ? b.call('$.proxy', init, options) : init, - options - ) + ? should_proxy(init, context.state.scope) + ? b.call('$.assignable_proxy', init, options) + : b.call('$.state', init, options) : field.kind === 'raw_state' ? b.call('$.state', init) : field.kind === 'derived_by' diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 6c266fde210b..6897f554e2c4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -127,15 +127,12 @@ export function VariableDeclaration(node, context) { context.state.scope.get(id.name) ); const proxied = rune === '$state' && should_proxy(value, context.state.scope); - if (proxied) { - if (options != null) { - const generated = context.state.scope.generate('state_options'); - declarations.push(b.declarator(generated, options)); - options = b.id(generated); - } + const is_state = is_state_source(binding, context.state.analysis); + if (proxied && is_state) { + value = b.call('$.assignable_proxy', value, options); + } else if (proxied) { value = b.call('$.proxy', value, options); - } - if (is_state_source(binding, context.state.analysis)) { + } else if (is_state) { value = b.call('$.state', value, options); } return value; diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 0251b8e47b20..8f97f5809564 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -152,7 +152,7 @@ export { } from './runtime.js'; export { validate_binding, validate_each_keys } from './validate.js'; export { raf } from './timing.js'; -export { proxy } from './proxy.js'; +export { proxy, assignable_proxy } from './proxy.js'; export { create_custom_element } from './dom/elements/custom-element.js'; export { child, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 9a2e88cc8533..4acc52c943ec 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -9,7 +9,7 @@ import { object_prototype } from '../shared/utils.js'; import { check_ownership, widen_ownership } from './dev/ownership.js'; -import { source, set } from './reactivity/sources.js'; +import { source, set, state } from './reactivity/sources.js'; import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; @@ -318,6 +318,17 @@ export function proxy(value, options, parent = null, prev) { }); } +/** + * @template T + * @param {T} value + * @param {ValueOptions} [options] + * @returns {Source} + */ + +export function assignable_proxy(value, options) { + return state(proxy(value, options), options); +} + /** * @param {Source} signal * @param {1 | -1} [d] From e42c7cd56782766a0c24a0f07efd5b6a70d994af Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 10:58:41 +0100 Subject: [PATCH 03/50] fix: move logic to proxy inside `set` --- .../phases/3-transform/client/types.d.ts | 2 +- .../client/visitors/AssignmentExpression.js | 37 +++++++++---------- .../3-transform/client/visitors/ClassBody.js | 4 +- .../client/visitors/shared/declarations.js | 7 ++-- .../src/internal/client/reactivity/sources.js | 13 ++++++- .../_expected/client/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 4 +- .../_expected/client/index.svelte.js | 2 +- 9 files changed, 41 insertions(+), 32 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index db4adf451c2e..f4ac6c1bb400 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -30,7 +30,7 @@ export interface ClientTransformState extends TransformState { /** turn `foo` into e.g. `$.get(foo)` */ read: (id: Identifier) => Expression; /** turn `foo = bar` into e.g. `$.set(foo, bar)` */ - assign?: (node: Identifier, value: Expression) => Expression; + assign?: (node: Identifier, value: Expression, proxy?: boolean) => Expression; /** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */ mutate?: (node: Identifier, mutation: AssignmentExpression | UpdateExpression) => Expression; /** turn `foo++` into e.g. `$.update(foo)` */ diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 0c70f7e00cda..06a010806a73 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -65,21 +65,22 @@ function build_assignment(operator, left, right, context) { context.visit(build_assignment_value(operator, left, right)) ); - if ( + const needs_proxy = private_state.kind === 'state' && is_non_coercive_operator(operator) && - should_proxy(value, context.state.scope) - ) { - value = build_proxy_reassignment(value, b.member(b.this, private_state.id)); - } + should_proxy(value, context.state.scope); if (context.state.in_constructor) { // inside the constructor, we can assign to `this.#foo.v` rather than using `$.set`, // since nothing is tracking the signal at this point - return b.assignment(operator, /** @type {Pattern} */ (context.visit(left)), value); + return b.assignment( + operator, + /** @type {Pattern} */ (context.visit(left)), + needs_proxy ? build_proxy_reassignment(value, b.member(b.this, private_state.id)) : value + ); } - return b.call('$.set', left, value); + return b.call('$.set', left, value, needs_proxy && b.true, dev && needs_proxy && b.true); } } @@ -113,19 +114,17 @@ function build_assignment(operator, left, right, context) { context.visit(build_assignment_value(operator, left, right)) ); - if ( + return transform.assign( + object, + value, !is_primitive && - binding.kind !== 'prop' && - binding.kind !== 'bindable_prop' && - binding.kind !== 'raw_state' && - context.state.analysis.runes && - should_proxy(right, context.state.scope) && - is_non_coercive_operator(operator) - ) { - value = build_proxy_reassignment(value, object); - } - - return transform.assign(object, value); + binding.kind !== 'prop' && + binding.kind !== 'bindable_prop' && + binding.kind !== 'raw_state' && + context.state.analysis.runes && + should_proxy(right, context.state.scope) && + is_non_coercive_operator(operator) + ); } // mutation diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 8738c397b75d..ed1c1ade0864 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -5,7 +5,7 @@ import { dev, is_ignored } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; import { regex_invalid_identifier_chars } from '../../../patterns.js'; import { get_rune } from '../../../scope.js'; -import { build_proxy_reassignment, should_proxy } from '../utils.js'; +import { should_proxy } from '../utils.js'; /** * @param {ClassBody} node @@ -160,7 +160,7 @@ export function ClassBody(node, context) { 'set', definition.key, [value], - [b.stmt(b.call('$.set', member, build_proxy_reassignment(value, prev)))] + [b.stmt(b.call('$.set', member, value, b.true, dev && b.true))] ) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js index 0bd8c352f6a9..57e40a9536e3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -1,7 +1,8 @@ /** @import { Identifier } from 'estree' */ /** @import { ComponentContext, Context } from '../../types' */ -import { is_state_source } from '../../utils.js'; +import { is_state_source, should_proxy } from '../../utils.js'; import * as b from '../../../../../utils/builders.js'; +import { dev } from '../../../../../state.js'; /** * Turns `foo` into `$.get(foo)` @@ -24,8 +25,8 @@ export function add_state_transformers(context) { ) { context.state.transform[name] = { read: binding.declaration_kind === 'var' ? (node) => b.call('$.safe_get', node) : get_value, - assign: (node, value) => { - let call = b.call('$.set', node, value); + assign: (node, value, proxy = false) => { + let call = b.call('$.set', node, value, proxy && b.true, dev && proxy && b.true); if (context.state.scope.get(`$${node.name}`)?.kind === 'store_sub') { call = b.call('$.store_unsub', call, b.literal(`$${node.name}`), b.id('$$stores')); diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 4cd9da7e3caf..971bc1b336c4 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -34,6 +34,7 @@ import { import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { get_stack } from '../dev/tracing.js'; +import { proxy } from '../proxy.js'; export let inspect_effects = new Set(); @@ -154,9 +155,11 @@ export function mutate(source, value) { * @template V * @param {Source} source * @param {V} value + * @param {boolean} [should_proxy] + * @param {boolean} [needs_previous] * @returns {V} */ -export function set(source, value) { +export function set(source, value, should_proxy = false, needs_previous = false) { if ( active_reaction !== null && is_runes() && @@ -168,7 +171,13 @@ export function set(source, value) { e.state_unsafe_mutation(); } - return internal_set(source, value); + let new_value = should_proxy + ? needs_previous + ? proxy(value, source.o, null, source) + : proxy(value, source.o) + : value; + + return internal_set(source, new_value); } /** diff --git a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js index df765613aa41..390e86a3510a 100644 --- a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js @@ -23,7 +23,7 @@ export default function Bind_component_snippet($$anchor) { return $.get(value); }, set value($$value) { - $.set(value, $.proxy($$value, $.get_options(value))); + $.set(value, $$value, true); } }); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index b6d0f002a76b..331e94597d3d 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -12,7 +12,7 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro } set a(value) { - $.set(this.#a, $.proxy(value, $.get_options(this.#a))); + $.set(this.#a, value, true); } #b = $.state(); diff --git a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js index d791efe6460b..47f297bce9c7 100644 --- a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js @@ -8,8 +8,8 @@ let d = 4; export function update(array) { ( - $.set(a, $.proxy(array[0], $.get_options(a))), - $.set(b, $.proxy(array[1], $.get_options(b))) + $.set(a, array[0], true), + $.set(b, array[1], true) ); [c, d] = array; diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index aca73406bda2..762a23754c9b 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -13,7 +13,7 @@ export default function Function_prop_no_getter($$anchor) { Button($$anchor, { onmousedown: () => $.set(count, $.get(count) + 1), onmouseup, - onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)), $.get_options(count))), + onmouseenter: () => $.set(count, plusOne($.get(count)), true), children: ($$anchor, $$slotProps) => { $.next(); From 807ffbb6337dbbd03944a6041116ae9c5ca569fc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 21 Jan 2025 07:29:30 -0500 Subject: [PATCH 04/50] fix: only call `onchange` once for array mutations (#15073) * only call onchange callbacks once per array mutation * fix * fix --- packages/svelte/src/internal/client/proxy.js | 12 +++++-- .../src/internal/client/reactivity/sources.js | 36 ++++++++++++++++++- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 4acc52c943ec..dcb6f46e8129 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -9,13 +9,15 @@ import { object_prototype } from '../shared/utils.js'; import { check_ownership, widen_ownership } from './dev/ownership.js'; -import { source, set, state } from './reactivity/sources.js'; +import { source, set, state, batch_onchange } from './reactivity/sources.js'; import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; import { get_stack } from './dev/tracing.js'; import { tracing_mode_flag } from '../flags/index.js'; +const array_methods = ['push', 'pop', 'shift', 'unshift', 'splice', 'reverse', 'sort']; + /** * @template T * @param {T} value @@ -168,7 +170,13 @@ export function proxy(value, options, parent = null, prev) { return v === UNINITIALIZED ? undefined : v; } - return Reflect.get(target, prop, receiver); + v = Reflect.get(target, prop, receiver); + + if (is_proxied_array && array_methods.includes(/** @type {string} */ (prop))) { + return batch_onchange(v); + } + + return v; }, getOwnPropertyDescriptor(target, prop) { diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 971bc1b336c4..405a19805015 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -45,6 +45,32 @@ export function set_inspect_effects(v) { inspect_effects = v; } +/** @type {null | Set<() => void>} */ +let onchange_batch = null; + +/** + * @param {Function} fn + */ +export function batch_onchange(fn) { + // @ts-expect-error + return function (...args) { + let previous_onchange_batch = onchange_batch; + + try { + onchange_batch = new Set(); + + // @ts-expect-error + return fn.apply(this, args); + } finally { + for (const onchange of /** @type {Set<() => void>} */ (onchange_batch)) { + onchange(); + } + + onchange_batch = previous_onchange_batch; + } + }; +} + /** * @template V * @param {V} v @@ -191,7 +217,15 @@ export function internal_set(source, value) { var old_value = source.v; source.v = value; source.wv = increment_write_version(); - untrack(() => source.o?.onchange?.()); + + var onchange = source.o?.onchange; + if (onchange) { + if (onchange_batch) { + onchange_batch.add(onchange); + } else { + onchange(); + } + } if (DEV && tracing_mode_flag) { source.updated = get_stack('UpdatedAt'); From 3353fafa26256134a7fbc9da094084361ae8d4de Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 14:08:49 +0100 Subject: [PATCH 05/50] chore: add tests for arrays --- .../samples/state-onchange/_config.js | 44 ++++++++++++++++++- .../samples/state-onchange/main.svelte | 12 ++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js index 4864e64cd61b..4b1985a1f9a9 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js @@ -3,7 +3,7 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { - const [btn, btn2, btn3, btn4, btn5, btn6] = target.querySelectorAll('button'); + const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9] = target.querySelectorAll('button'); flushSync(() => { btn.click(); }); @@ -40,5 +40,47 @@ export default test({ 'class proxy', 'class proxy' ]); + + flushSync(() => { + btn7.click(); + }); + assert.deepEqual(logs, [ + 'count', + 'proxy', + 'proxy', + 'class count', + 'class proxy', + 'class proxy', + 'arr' + ]); + + flushSync(() => { + btn8.click(); + }); + assert.deepEqual(logs, [ + 'count', + 'proxy', + 'proxy', + 'class count', + 'class proxy', + 'class proxy', + 'arr', + 'arr' + ]); + + flushSync(() => { + btn9.click(); + }); + assert.deepEqual(logs, [ + 'count', + 'proxy', + 'proxy', + 'class count', + 'class proxy', + 'class proxy', + 'arr', + 'arr', + 'arr' + ]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte index 46253488d20a..efa688fc15e8 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte @@ -25,6 +25,12 @@ } const class_test = new Test(); + + let arr = $state([0,1,2], { + onchange(){ + console.log("arr"); + } + }) @@ -33,4 +39,8 @@ - \ No newline at end of file + + + + + \ No newline at end of file From 23df27fb02d9ea7497007c1e189532cbab358bab Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 14:20:09 +0100 Subject: [PATCH 06/50] chore: update types for `$state.raw` --- packages/svelte/src/ambient.d.ts | 1 + packages/svelte/types/index.d.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/svelte/src/ambient.d.ts b/packages/svelte/src/ambient.d.ts index 211064c535da..b0dc8986594b 100644 --- a/packages/svelte/src/ambient.d.ts +++ b/packages/svelte/src/ambient.d.ts @@ -117,6 +117,7 @@ declare namespace $state { * * @param initial The initial value */ + export function raw(initial?: T, options?: import('svelte').StateOptions): T; export function raw(initial: T): T; export function raw(): T | undefined; /** diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 1f9f94646641..6b6dcfdc8140 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2776,6 +2776,7 @@ declare namespace $state { * * @param initial The initial value */ + export function raw(initial?: T, options?: import('svelte').StateOptions): T; export function raw(initial: T): T; export function raw(): T | undefined; /** From 07499dac6909d840edc69784afdae89752a597a8 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 14:27:13 +0100 Subject: [PATCH 07/50] fix: add options to `$state.raw` in classes --- .../3-transform/client/visitors/ClassBody.js | 2 +- .../samples/state-raw-onchange/_config.js | 58 +++++++++++++++++++ .../samples/state-raw-onchange/main.svelte | 47 +++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index ed1c1ade0864..e9a1fec0c657 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -131,7 +131,7 @@ export function ClassBody(node, context) { ? b.call('$.assignable_proxy', init, options) : b.call('$.state', init, options) : field.kind === 'raw_state' - ? b.call('$.state', init) + ? b.call('$.state', init, options) : field.kind === 'derived_by' ? b.call('$.derived', init) : b.call('$.derived', b.thunk(init)); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js new file mode 100644 index 000000000000..49b367f78765 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js @@ -0,0 +1,58 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9, btn10] = + target.querySelectorAll('button'); + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['count']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['count']); + + flushSync(() => { + btn3.click(); + }); + assert.deepEqual(logs, ['count', 'proxy']); + + flushSync(() => { + btn4.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count']); + + flushSync(() => { + btn5.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count']); + + flushSync(() => { + btn6.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + + flushSync(() => { + btn7.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + + flushSync(() => { + btn8.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + + flushSync(() => { + btn9.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + + flushSync(() => { + btn10.click(); + }); + assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy', 'arr']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte new file mode 100644 index 000000000000..5f05dd7d0a99 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + \ No newline at end of file From 1fb57ebe16d24adbb7345d2b9adf9526eca87c20 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 14:40:34 +0100 Subject: [PATCH 08/50] docs: add docs for state options --- documentation/docs/02-runes/02-$state.md | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index 49e17cd08ff3..9f94418aea19 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -147,6 +147,35 @@ person = { This can improve performance with large arrays and objects that you weren't planning to mutate anyway, since it avoids the cost of making them reactive. Note that raw state can _contain_ reactive state (for example, a raw array of reactive objects). +## State options + +Both `$state` and `$state.raw` can optionally accept a second argument. This argument allow you to specify an `onchange` function that will be called synchronously whenever the object change (for `$state` it will also be called for deep mutations). + +The `onchange` function is untracked so even if you assign within an `$effect` it will not cause unwanted dependencies. + +```js +let count = $state(0, { + onchange(){ + console.log("count is now", count); + } +}); + +// this will log "count is now 1" +count++; +``` + +this could be especially useful if you want to sync some stateful variable that could be mutated without using an effect. + +```js +let array = $state([], { + onchange(){ + localStorage.setItem('array', JSON.stringify(array)); + } +}); + +array.push(array.length); +``` + ## `$state.snapshot` To take a static snapshot of a deeply reactive `$state` proxy, use `$state.snapshot`: From 4ed4351c54022523018bcbcb213694d883d896d9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 15:24:37 +0100 Subject: [PATCH 09/50] fix: invoke `onchange` in component constructor --- .../phases/3-transform/client/utils.js | 10 --------- .../client/visitors/AssignmentExpression.js | 22 +++++++++---------- packages/svelte/src/internal/client/index.js | 9 +++++++- .../src/internal/client/reactivity/sources.js | 20 +++++++++++++++++ .../samples/state-onchange/_config.js | 5 +++++ .../samples/state-onchange/main.svelte | 18 +++++++++++++++ .../samples/state-raw-onchange/_config.js | 5 +++++ .../samples/state-raw-onchange/main.svelte | 18 +++++++++++++++ .../_expected/client/index.svelte.js | 2 +- 9 files changed, 85 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 4256b84962a7..664b909a09af 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -45,16 +45,6 @@ export function build_getter(node, state) { return node; } -/** - * @param {Expression} value - * @param {Expression} previous - */ -export function build_proxy_reassignment(value, previous) { - return dev - ? b.call('$.proxy', value, b.call('$.get_options', previous), b.null, previous) - : b.call('$.proxy', value, b.call('$.get_options', previous)); -} - /** * @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node * @param {ComponentContext} context diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 06a010806a73..4d34f2c4681a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -9,7 +9,7 @@ import { is_event_attribute } from '../../../../utils/ast.js'; import { dev, filename, is_ignored, locate_node, locator } from '../../../../state.js'; -import { build_proxy_reassignment, should_proxy } from '../utils.js'; +import { should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; /** @@ -70,17 +70,15 @@ function build_assignment(operator, left, right, context) { is_non_coercive_operator(operator) && should_proxy(value, context.state.scope); - if (context.state.in_constructor) { - // inside the constructor, we can assign to `this.#foo.v` rather than using `$.set`, - // since nothing is tracking the signal at this point - return b.assignment( - operator, - /** @type {Pattern} */ (context.visit(left)), - needs_proxy ? build_proxy_reassignment(value, b.member(b.this, private_state.id)) : value - ); - } - - return b.call('$.set', left, value, needs_proxy && b.true, dev && needs_proxy && b.true); + return b.call( + // inside the constructor, we use `$.simple_set` rather than using `$.set`, + // that only assign the value and eventually call onchange since nothing is tracking the signal at this point + context.state.in_constructor ? '$.simple_set' : '$.set', + left, + value, + needs_proxy && b.true, + dev && needs_proxy && b.true + ); } } diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 8f97f5809564..5b8969d5ee11 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -109,7 +109,14 @@ export { user_effect, user_pre_effect } from './reactivity/effects.js'; -export { mutable_state, mutate, set, state, get_options } from './reactivity/sources.js'; +export { + mutable_state, + mutate, + set, + simple_set, + state, + get_options +} from './reactivity/sources.js'; export { prop, rest_props, diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 405a19805015..32b93a2b0aba 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -206,6 +206,26 @@ export function set(source, value, should_proxy = false, needs_previous = false) return internal_set(source, new_value); } +/** + * @template V + * @param {Source} source + * @param {V} value + * @param {boolean} [should_proxy] + * @param {boolean} [needs_previous] + * @returns {V} + */ +export function simple_set(source, value, should_proxy = false, needs_previous = false) { + let new_value = should_proxy + ? needs_previous + ? proxy(value, source.o, null, source) + : proxy(value, source.o) + : value; + + source.v = new_value; + + return new_value; +} + /** * @template V * @param {Source} source diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js index 4b1985a1f9a9..82625ded0acc 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js @@ -4,6 +4,11 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9] = target.querySelectorAll('button'); + + assert.deepEqual(logs, ['constructor count', 'constructor proxy']); + + logs.length = 0; + flushSync(() => { btn.click(); }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte index efa688fc15e8..11d102682a71 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte @@ -22,6 +22,24 @@ console.log("class proxy"); } }) + + #in_constructor = $state(0, { + onchange(){ + console.log("constructor count"); + } + }); + + #in_constructor_proxy = $state({ count: 0 }, { + onchange(){ + console.log("constructor proxy"); + } + }); + + + constructor(){ + this.#in_constructor++; + this.#in_constructor_proxy.count++; + } } const class_test = new Test(); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js index 49b367f78765..ab2a125c1213 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js @@ -5,6 +5,11 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9, btn10] = target.querySelectorAll('button'); + + assert.deepEqual(logs, ['constructor count', 'constructor proxy']); + + logs.length = 0; + flushSync(() => { btn.click(); }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte index 5f05dd7d0a99..3879e32621a0 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte @@ -22,6 +22,24 @@ console.log("class proxy"); } }) + + #in_constructor = $state(0, { + onchange(){ + console.log("constructor count"); + } + }); + + #in_constructor_proxy = $state({ count: 0 }, { + onchange(){ + console.log("constructor proxy"); + } + }); + + + constructor(){ + this.#in_constructor++; + this.#in_constructor_proxy.count++; + } } const class_test = new Test(); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index 331e94597d3d..bc451686422a 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -19,7 +19,7 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro constructor() { this.a = 1; - this.#b.v = 2; + $.simple_set(this.#b, 2); } } From e2c2580e81b32a137e0bec37c91708bfbdb88112 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 15:32:34 +0100 Subject: [PATCH 10/50] fix: move `onchange` call right before inspect effects --- .../src/internal/client/reactivity/sources.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 32b93a2b0aba..746d426aca6c 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -238,15 +238,6 @@ export function internal_set(source, value) { source.v = value; source.wv = increment_write_version(); - var onchange = source.o?.onchange; - if (onchange) { - if (onchange_batch) { - onchange_batch.add(onchange); - } else { - onchange(); - } - } - if (DEV && tracing_mode_flag) { source.updated = get_stack('UpdatedAt'); if (active_effect != null) { @@ -274,6 +265,15 @@ export function internal_set(source, value) { } } + var onchange = source.o?.onchange; + if (onchange) { + if (onchange_batch) { + onchange_batch.add(onchange); + } else { + onchange(); + } + } + if (DEV && inspect_effects.size > 0) { const inspects = Array.from(inspect_effects); var previously_flushing_effect = is_flushing_effect; From f013e8722e056b5c8a5e86f755cfb7a837aeaf24 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 15:38:29 +0100 Subject: [PATCH 11/50] fix: only batch array methods if there's an `onchange` function --- packages/svelte/src/internal/client/proxy.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index dcb6f46e8129..2d5b5d297c2e 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -172,7 +172,11 @@ export function proxy(value, options, parent = null, prev) { v = Reflect.get(target, prop, receiver); - if (is_proxied_array && array_methods.includes(/** @type {string} */ (prop))) { + if ( + is_proxied_array && + array_methods.includes(/** @type {string} */ (prop)) && + options?.onchange != null + ) { return batch_onchange(v); } From 37888f4cffee0c93a2c98ced66d022317d3e5f7f Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 15:39:25 +0100 Subject: [PATCH 12/50] fix: move easier condition up --- packages/svelte/src/internal/client/proxy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 2d5b5d297c2e..c6e304fbf24f 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -174,8 +174,8 @@ export function proxy(value, options, parent = null, prev) { if ( is_proxied_array && - array_methods.includes(/** @type {string} */ (prop)) && - options?.onchange != null + options?.onchange != null && + array_methods.includes(/** @type {string} */ (prop)) ) { return batch_onchange(v); } From c83d01c64ff3cb9043a93aa31343431b64addea9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 16:01:59 +0100 Subject: [PATCH 13/50] fix: move `onchange` after `inspect` effects --- .../src/internal/client/reactivity/sources.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 746d426aca6c..11e5d6173582 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -265,15 +265,6 @@ export function internal_set(source, value) { } } - var onchange = source.o?.onchange; - if (onchange) { - if (onchange_batch) { - onchange_batch.add(onchange); - } else { - onchange(); - } - } - if (DEV && inspect_effects.size > 0) { const inspects = Array.from(inspect_effects); var previously_flushing_effect = is_flushing_effect; @@ -294,6 +285,15 @@ export function internal_set(source, value) { } inspect_effects.clear(); } + + var onchange = source.o?.onchange; + if (onchange) { + if (onchange_batch) { + onchange_batch.add(onchange); + } else { + onchange(); + } + } } return value; From 7fc930a32d1ee31daa2c0dfb5e419291662932e5 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Wed, 22 Jan 2025 15:24:52 +0100 Subject: [PATCH 14/50] chore: bette phrasing for docs and error Co-authored-by: Matei Trandafir --- documentation/docs/02-runes/02-$state.md | 2 +- .../src/compiler/phases/2-analyze/visitors/CallExpression.js | 2 +- .../compiler-errors/samples/runes-wrong-state-args/_config.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index 9f94418aea19..1b094d375bd6 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -149,7 +149,7 @@ This can improve performance with large arrays and objects that you weren't plan ## State options -Both `$state` and `$state.raw` can optionally accept a second argument. This argument allow you to specify an `onchange` function that will be called synchronously whenever the object change (for `$state` it will also be called for deep mutations). +Both `$state` and `$state.raw` can optionally accept a second argument. This allows you to specify an `onchange` function that will be called synchronously whenever the state value changes (for `$state` it will also be called for deep mutations). The `onchange` function is untracked so even if you assign within an `$effect` it will not cause unwanted dependencies. diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 2eff18cd51ae..419cfd770256 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -88,7 +88,7 @@ export function CallExpression(node, context) { if ((rune === '$derived' || rune === '$derived.by') && node.arguments.length !== 1) { e.rune_invalid_arguments_length(node, rune, 'exactly one argument'); } else if (rune === '$state' && node.arguments.length > 2) { - e.rune_invalid_arguments_length(node, rune, 'zero, one or two arguments'); + e.rune_invalid_arguments_length(node, rune, 'at most two arguments'); } break; diff --git a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js index bd8349d9b866..d2c92ca6814e 100644 --- a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js +++ b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-args/_config.js @@ -3,6 +3,6 @@ import { test } from '../../test'; export default test({ error: { code: 'rune_invalid_arguments_length', - message: '`$state` must be called with zero, one or two arguments' + message: '`$state` must be called with at most two arguments' } }); From 7c215bf249cea0660f61769db1974a6ee286fc23 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 22 Jan 2025 16:10:10 +0100 Subject: [PATCH 15/50] fix: notify both `onchange` if proxy is passed into proxy --- .../svelte/src/internal/client/constants.js | 1 + packages/svelte/src/internal/client/proxy.js | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a4840ce4ebd0..5f24792eaed6 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -22,6 +22,7 @@ export const HEAD_EFFECT = 1 << 19; export const EFFECT_HAS_DERIVED = 1 << 20; export const STATE_SYMBOL = Symbol('$state'); +export const PROXY_ONCHANGE_SYMBOL = Symbol('proxy onchange'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); export const LEGACY_PROPS = Symbol('legacy props'); export const LOADING_ATTR_SYMBOL = Symbol(''); diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index c6e304fbf24f..406b279df3fe 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -10,7 +10,7 @@ import { } from '../shared/utils.js'; import { check_ownership, widen_ownership } from './dev/ownership.js'; import { source, set, state, batch_onchange } from './reactivity/sources.js'; -import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.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 { get_stack } from './dev/tracing.js'; @@ -33,7 +33,13 @@ export function proxy(value, options, parent = null, prev) { stack = get_stack('CreatedAt'); } // if non-proxyable, or is already a proxy, return `value` - if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) { + if (typeof value !== 'object' || value === null) { + return value; + } + + if (STATE_SYMBOL in value) { + // @ts-ignore + value[PROXY_ONCHANGE_SYMBOL] = options?.onchange; return value; } @@ -237,6 +243,14 @@ 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; From ec77f8bdf17af2ecb3eac9faea656456fe7b1d3a Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 22 Jan 2025 18:21:18 +0100 Subject: [PATCH 16/50] chore: add error for non-inline options --- .../docs/98-reference/.generated/compile-errors.md | 6 ++++++ packages/svelte/messages/compile-errors/script.md | 4 ++++ packages/svelte/src/compiler/errors.js | 10 ++++++++++ .../phases/2-analyze/visitors/CallExpression.js | 9 +++++++-- .../samples/rune-invalid-options/errors.json | 14 ++++++++++++++ .../samples/rune-invalid-options/input.svelte | 4 ++++ 6 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 packages/svelte/tests/validator/samples/rune-invalid-options/errors.json create mode 100644 packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index 2fef3bd45d50..58aabe20cbb0 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -648,6 +648,12 @@ Cannot access a computed property of a rune `%name%` is not a valid rune ``` +### rune_invalid_options + +``` +Options for `%rune%` needs to be declared inline +``` + ### rune_invalid_usage ``` diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index 0aa6fbed90d8..7e56490a2efe 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -158,6 +158,10 @@ This turned out to be buggy and unpredictable, particularly when working with de > `%name%` is not a valid rune +## rune_invalid_options + +> Options for `%rune%` needs to be declared inline + ## rune_invalid_usage > Cannot use `%rune%` rune in non-runes mode diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 53a6ac6849ec..e0d1189e2950 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -373,6 +373,16 @@ export function rune_invalid_name(node, name) { e(node, 'rune_invalid_name', `\`${name}\` is not a valid rune\nhttps://svelte.dev/e/rune_invalid_name`); } +/** + * Options for `%rune%` needs to be declared inline + * @param {null | number | NodeLike} node + * @param {string} rune + * @returns {never} + */ +export function rune_invalid_options(node, rune) { + e(node, 'rune_invalid_options', `Options for \`${rune}\` needs to be declared inline\nhttps://svelte.dev/e/rune_invalid_options`); +} + /** * Cannot use `%rune%` rune in non-runes mode * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 419cfd770256..e9adebd81af4 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -87,8 +87,13 @@ export function CallExpression(node, context) { if ((rune === '$derived' || rune === '$derived.by') && node.arguments.length !== 1) { e.rune_invalid_arguments_length(node, rune, 'exactly one argument'); - } else if (rune === '$state' && node.arguments.length > 2) { - e.rune_invalid_arguments_length(node, rune, 'at most two arguments'); + } else if (rune === '$state' || rune === '$state.raw') { + if (node.arguments.length > 2) { + e.rune_invalid_arguments_length(node, rune, 'at most two arguments'); + } + if (node.arguments.length === 2 && node.arguments[1].type !== 'ObjectExpression') { + e.rune_invalid_options(node.arguments[1], rune); + } } break; diff --git a/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json b/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json new file mode 100644 index 000000000000..0403da6a2812 --- /dev/null +++ b/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "rune_invalid_options", + "end": { + "column": 29, + "line": 3 + }, + "start": { + "column": 22, + "line": 3 + }, + "message": "Options for `$state` needs to be declared inline" + } +] diff --git a/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte b/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte new file mode 100644 index 000000000000..252737e26cef --- /dev/null +++ b/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte @@ -0,0 +1,4 @@ + \ No newline at end of file From d0d9a3642c86687d9cccf2872fdab7c6460ebcb5 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 22 Jan 2025 18:28:43 +0100 Subject: [PATCH 17/50] chore: add test for agglomerated `onchange` --- .../state-onchange-accumulated/_config.js | 18 ++++++++++++++++++ .../state-onchange-accumulated/main.svelte | 15 +++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js new file mode 100644 index 000000000000..f99a1c856b75 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js @@ -0,0 +1,18 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2] = target.querySelectorAll('button'); + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['foo', 'baz']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['foo', 'baz', 'foo', 'baz']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/main.svelte new file mode 100644 index 000000000000..1a299533a46c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/main.svelte @@ -0,0 +1,15 @@ + + + + From e2371321341ca65b61185a42afce1eda29f90c26 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 22 Jan 2025 18:45:38 +0100 Subject: [PATCH 18/50] fix: correct types --- packages/svelte/src/ambient.d.ts | 10 +++++++++- packages/svelte/types/index.d.ts | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/ambient.d.ts b/packages/svelte/src/ambient.d.ts index b0dc8986594b..0d6d45dfc20b 100644 --- a/packages/svelte/src/ambient.d.ts +++ b/packages/svelte/src/ambient.d.ts @@ -20,7 +20,11 @@ declare module '*.svelte' { * * @param initial The initial value */ -declare function $state(initial?: T, options?: import('svelte').StateOptions): T; +declare function $state( + initial: undefined, + options?: import('svelte').StateOptions +): T | undefined; +declare function $state(initial: T, options?: import('svelte').StateOptions): T; declare function $state(initial: T): T; declare function $state(): T | undefined; @@ -117,6 +121,10 @@ declare namespace $state { * * @param initial The initial value */ + export function raw( + initial: undefined, + options?: import('svelte').StateOptions + ): T | undefined; export function raw(initial?: T, options?: import('svelte').StateOptions): T; export function raw(initial: T): T; export function raw(): T | undefined; diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 6b6dcfdc8140..176de125a6a1 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2679,7 +2679,11 @@ declare module 'svelte/types/compiler/interfaces' { * * @param initial The initial value */ -declare function $state(initial?: T, options?: import('svelte').StateOptions): T; +declare function $state( + initial: undefined, + options?: import('svelte').StateOptions +): T | undefined; +declare function $state(initial: T, options?: import('svelte').StateOptions): T; declare function $state(initial: T): T; declare function $state(): T | undefined; @@ -2776,6 +2780,10 @@ declare namespace $state { * * @param initial The initial value */ + export function raw( + initial: undefined, + options?: import('svelte').StateOptions + ): T | undefined; export function raw(initial?: T, options?: import('svelte').StateOptions): T; export function raw(initial: T): T; export function raw(): T | undefined; From f16e4457ace965424b92fb67eba2a2f1355aab94 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 23 Jan 2025 11:54:32 +0100 Subject: [PATCH 19/50] chore: push failing test for extrapolated reference --- .../_config.js | 23 ++++++++++++++ .../main.svelte | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte 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 new file mode 100644 index 000000000000..27f3a731871b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/_config.js @@ -0,0 +1,23 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3, btn4] = target.querySelectorAll('button'); + logs.length = 0; + + flushSync(() => { + btn.click(); + }); + flushSync(() => { + btn2.click(); + }); + flushSync(() => { + btn3.click(); + }); + flushSync(() => { + btn4.click(); + }); + assert.deepEqual(logs, []); + } +}); 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 new file mode 100644 index 000000000000..3a039bbd232c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-extrapolated-reference/main.svelte @@ -0,0 +1,31 @@ + + + + + + From 316a341c79057854d4ec5bd98e69174b1fd632f5 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 23 Jan 2025 23:05:19 +0100 Subject: [PATCH 20/50] 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 406b279df3fe..81dbbfda2374 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 27f3a731871b..cc0bfbb1e382 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 3a039bbd232c..4d586c7707cb 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; + + From 55fdccc8638cbcd674d93b1234f5a3623d02612a Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 23 Jan 2025 23:08:52 +0100 Subject: [PATCH 21/50] fix: make it work with reassigned `length` --- packages/svelte/src/internal/client/proxy.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 81dbbfda2374..c884597c7309 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -306,6 +306,9 @@ export function proxy(value, options, parent = null, prev) { for (var i = value; i < /** @type {Source} */ (s).v; i += 1) { var other_s = sources.get(i + ''); if (other_s !== undefined) { + if (typeof other_s.v === 'object' && other_s.v !== null && STATE_SYMBOL in other_s.v) { + other_s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + } set(other_s, UNINITIALIZED); } else if (i in target) { // If the item exists in the original, we need to create a uninitialized source, From 873cd5fc846c8aeaf1ab96218060127cc4490363 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 24 Jan 2025 10:21:23 +0100 Subject: [PATCH 22/50] fix: double log on push --- packages/svelte/src/internal/client/proxy.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index c884597c7309..ae3fb3598db1 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -33,12 +33,13 @@ function clone_options(options) { /** * @template T * @param {T} value - * @param {ValueOptions} [options] + * @param {ValueOptions} [_options] * @param {ProxyMetadata | null} [parent] * @param {Source} [prev] dev mode only * @returns {T} */ -export function proxy(value, options, parent = null, prev) { +export function proxy(value, _options, parent = null, prev) { + let options = clone_options(_options); /** @type {Error | null} */ var stack = null; if (DEV && tracing_mode_flag) { @@ -80,7 +81,10 @@ export function proxy(value, options, parent = null, prev) { 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, options, stack)); + sources.set( + 'length', + source(/** @type {any[]} */ (value).length, clone_options(options), stack) + ); } /** @type {ProxyMetadata} */ @@ -183,9 +187,9 @@ export function proxy(value, options, parent = null, prev) { // 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); + onchanges?.add?.(value); } else if (options?.onchange != null && value) { - onchanges.delete(value); + onchanges?.delete?.(value); } else { options = { onchange: value From 35e2afe2bb451ef149954c038b1b7e18372a850b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 24 Jan 2025 10:24:57 +0100 Subject: [PATCH 23/50] fix: test for `simple_set` and `simple_set` --- packages/svelte/src/internal/client/reactivity/sources.js | 2 ++ .../tests/runtime-runes/samples/state-onchange/main.svelte | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7db9dc49d312..7495b7ef6e49 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -225,6 +225,8 @@ export function simple_set(source, value, should_proxy = false, needs_previous = source.v = new_value; + source.o?.onchange?.(); + return new_value; } diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte index 11d102682a71..565e8477dc26 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte @@ -37,7 +37,7 @@ constructor(){ - this.#in_constructor++; + this.#in_constructor = 42; this.#in_constructor_proxy.count++; } } From 2a3fb7a308c17e4ddbfd6493d2f6917c5c2aa584 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Mar 2025 15:06:34 +0100 Subject: [PATCH 24/50] fix: lint and test --- .../samples/runes-wrong-state-raw-args/_config.js | 2 +- packages/svelte/types/index.d.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-raw-args/_config.js b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-raw-args/_config.js index af226559d11b..59efc6af3942 100644 --- a/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-raw-args/_config.js +++ b/packages/svelte/tests/compiler-errors/samples/runes-wrong-state-raw-args/_config.js @@ -3,6 +3,6 @@ import { test } from '../../test'; export default test({ error: { code: 'rune_invalid_arguments_length', - message: '`$state.raw` must be called with zero or one arguments' + message: '`$state.raw` must be called with at most two arguments' } }); diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 0de66ff136dc..d748094a6cc4 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -410,7 +410,9 @@ declare module 'svelte' { * @deprecated Use [`$effect`](https://svelte.dev/docs/svelte/$effect) instead * */ export function afterUpdate(fn: () => void): void; - + type Getters = { + [K in keyof T]: () => T[K]; + }; export interface StateOptions { onchange?: () => unknown; } From 128c3254db9e35b30b49bf39d5df5387900bb7cb Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Mar 2025 23:02:39 +0100 Subject: [PATCH 25/50] fix: remove source onchange from proxy on reassignment --- .../src/internal/client/reactivity/sources.js | 11 ++++++- .../state-onchange-reassign-proxy/_config.js | 30 +++++++++++++++++++ .../state-onchange-reassign-proxy/main.svelte | 30 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index c2512d0d40d6..6fe837ff1225 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -27,7 +27,8 @@ import { UNOWNED, MAYBE_DIRTY, BLOCK_EFFECT, - ROOT_EFFECT + ROOT_EFFECT, + PROXY_ONCHANGE_SYMBOL } from '../constants.js'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -239,6 +240,14 @@ export function internal_set(source, value) { if (!source.equals(value)) { var old_value = source.v; + if (old_value != null && source.o?.onchange) { + // @ts-ignore + const remove = old_value[PROXY_ONCHANGE_SYMBOL]; + if (remove && typeof remove === 'function') { + remove(source.o?.onchange, true); + } + } + if (is_destroying_effect) { old_values.set(source, value); } else { diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js new file mode 100644 index 000000000000..92ddd1369d50 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js @@ -0,0 +1,30 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3, btn4] = target.querySelectorAll('button'); + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['a']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['a', 'b', 'c']); + flushSync(() => { + btn3.click(); + }); + assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c']); + flushSync(() => { + btn4.click(); + }); + assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c', 'c']); + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c', 'c', 'b']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte new file mode 100644 index 000000000000..d1931cbaac15 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte @@ -0,0 +1,30 @@ + + + + + + + + \ No newline at end of file From 3e886c71f0cd627f4e2305c6f381e1def05364d1 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Mar 2025 23:06:56 +0100 Subject: [PATCH 26/50] fix: add extra check --- packages/svelte/src/internal/client/reactivity/sources.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 6fe837ff1225..5abafa0ebdb9 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -240,7 +240,7 @@ export function internal_set(source, value) { if (!source.equals(value)) { var old_value = source.v; - if (old_value != null && source.o?.onchange) { + if (typeof old_value === 'object' && old_value != null && source.o?.onchange) { // @ts-ignore const remove = old_value[PROXY_ONCHANGE_SYMBOL]; if (remove && typeof remove === 'function') { From 25e03b3fada0605c3e57fe032981cfff227def7c Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Mar 2025 23:25:23 +0100 Subject: [PATCH 27/50] fix: batch assignment to length of an array --- packages/svelte/src/internal/client/proxy.js | 87 +++++++++++-------- .../_config.js | 18 ++++ .../main.svelte | 14 +++ 3 files changed, 84 insertions(+), 35 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/main.svelte diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index caa6fadf1dc1..229e4acc1800 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -19,6 +19,15 @@ import { active_effect, get } from './runtime.js'; const array_methods = ['push', 'pop', 'shift', 'unshift', 'splice', 'reverse', 'sort']; +/** + * Used to prevent batching in case we are not setting the length of an array + * @param {any} fn + * @returns + */ +function identity(fn) { + return fn; +} + /** * @param {ValueOptions | undefined} options * @returns {ValueOptions | undefined} @@ -306,46 +315,54 @@ export function proxy(value, _options, 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} */ (s).v; i += 1) { - var other_s = sources.get(i + ''); - if (other_s !== undefined) { - if (typeof other_s.v === 'object' && other_s.v !== null && STATE_SYMBOL in other_s.v) { - other_s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + // if we are changing the length of the array we batch all the changes + // to the sources and the original value by calling batch_onchange and immediately + // invoking it...otherwise we just invoke an identity function + (is_proxied_array && prop === 'length' ? batch_onchange : identity)(() => { + // variable.length = value -> clear all signals with index >= value + if (is_proxied_array && prop === 'length') { + for (var i = value; i < /** @type {Source} */ (s).v; i += 1) { + var other_s = sources.get(i + ''); + if (other_s !== undefined) { + if ( + typeof other_s.v === 'object' && + other_s.v !== null && + STATE_SYMBOL in other_s.v + ) { + other_s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + } + 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, clone_options(options), stack); + sources.set(i + '', other_s); } - 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, clone_options(options), stack); - 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 - // object property before writing to that property. - if (s === undefined) { - if (!has || get_descriptor(target, prop)?.writable) { - 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; - // 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); + // 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 + // object property before writing to that property. + if (s === undefined) { + if (!has || get_descriptor(target, prop)?.writable) { + 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; + // 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)); } - set(s, proxy(value, clone_options(options), metadata)); - } - + })(); if (DEV) { /** @type {ProxyMetadata | undefined} */ var prop_metadata = value?.[STATE_SYMBOL_METADATA]; diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js new file mode 100644 index 000000000000..64f81ac0e2ea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js @@ -0,0 +1,18 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2] = target.querySelectorAll('button'); + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, [[{}, {}, {}, {}, {}, {}, {}, {}]]); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, [[{}, {}, {}, {}, {}, {}, {}, {}], []]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/main.svelte new file mode 100644 index 000000000000..dcea39d2c3ca --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/main.svelte @@ -0,0 +1,14 @@ + + + + + + + +
{JSON.stringify(array)}
\ No newline at end of file From 8d02009cccc238371ded08c6fe7d3c741f48c441 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 21:59:26 -0400 Subject: [PATCH 28/50] Apply suggestions from code review --- .../client/visitors/AssignmentExpression.js | 3 +-- .../phases/3-transform/client/visitors/ClassBody.js | 6 +----- .../3-transform/client/visitors/shared/declarations.js | 2 +- .../svelte/src/internal/client/reactivity/sources.js | 10 ++++------ 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 1fd794672fd0..5ff9945fa67d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -75,8 +75,7 @@ function build_assignment(operator, left, right, context) { context.state.in_constructor ? '$.simple_set' : '$.set', left, value, - needs_proxy && b.true, - dev && needs_proxy && b.true + needs_proxy && b.true ); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 803d95044f57..a1306834f03b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -159,11 +159,7 @@ export function ClassBody(node, context) { 'set', definition.key, [value], - [ - b.stmt( - b.call('$.set', member, value, field.kind === 'state' && b.true, dev && b.true) - ) - ] + [b.stmt(b.call('$.set', member, value, field.kind === 'state' && b.true))] ) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js index 57e40a9536e3..77e9831e1123 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -26,7 +26,7 @@ export function add_state_transformers(context) { context.state.transform[name] = { read: binding.declaration_kind === 'var' ? (node) => b.call('$.safe_get', node) : get_value, assign: (node, value, proxy = false) => { - let call = b.call('$.set', node, value, proxy && b.true, dev && proxy && b.true); + let call = b.call('$.set', node, value, proxy && b.true); if (context.state.scope.get(`$${node.name}`)?.kind === 'store_sub') { call = b.call('$.store_unsub', call, b.literal(`$${node.name}`), b.id('$$stores')); diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 5abafa0ebdb9..dcee1f12cdc3 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -183,10 +183,9 @@ export function mutate(source, value) { * @param {Source} source * @param {V} value * @param {boolean} [should_proxy] - * @param {boolean} [needs_previous] * @returns {V} */ -export function set(source, value, should_proxy = false, needs_previous = false) { +export function set(source, value, should_proxy = false) { if ( active_reaction !== null && !untracking && @@ -200,7 +199,7 @@ export function set(source, value, should_proxy = false, needs_previous = false) } let new_value = should_proxy - ? needs_previous + ? DEV ? proxy(value, source.o, null, source) : proxy(value, source.o) : value; @@ -213,12 +212,11 @@ export function set(source, value, should_proxy = false, needs_previous = false) * @param {Source} source * @param {V} value * @param {boolean} [should_proxy] - * @param {boolean} [needs_previous] * @returns {V} */ -export function simple_set(source, value, should_proxy = false, needs_previous = false) { +export function simple_set(source, value, should_proxy = false) { let new_value = should_proxy - ? needs_previous + ? DEV ? proxy(value, source.o, null, source) : proxy(value, source.o) : value; From 0b8d2fa81707ad2077c28d8dc02fa5f9e80ba927 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:24:43 -0400 Subject: [PATCH 29/50] remove static analysis restriction --- .../docs/98-reference/.generated/compile-errors.md | 6 ------ packages/svelte/messages/compile-errors/script.md | 4 ---- packages/svelte/src/compiler/errors.js | 10 ---------- .../phases/2-analyze/visitors/CallExpression.js | 3 --- .../samples/rune-invalid-options/errors.json | 14 -------------- .../samples/rune-invalid-options/input.svelte | 4 ---- 6 files changed, 41 deletions(-) delete mode 100644 packages/svelte/tests/validator/samples/rune-invalid-options/errors.json delete mode 100644 packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index c2d4b2815939..ea116014e7b1 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -660,12 +660,6 @@ Cannot access a computed property of a rune `%name%` is not a valid rune ``` -### rune_invalid_options - -``` -Options for `%rune%` needs to be declared inline -``` - ### rune_invalid_usage ``` diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index 00e3797448dc..795c0b007dca 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -162,10 +162,6 @@ This turned out to be buggy and unpredictable, particularly when working with de > `%name%` is not a valid rune -## rune_invalid_options - -> Options for `%rune%` needs to be declared inline - ## rune_invalid_usage > Cannot use `%rune%` rune in non-runes mode diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 161b57820396..677b99fcff81 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -383,16 +383,6 @@ export function rune_invalid_name(node, name) { e(node, 'rune_invalid_name', `\`${name}\` is not a valid rune\nhttps://svelte.dev/e/rune_invalid_name`); } -/** - * Options for `%rune%` needs to be declared inline - * @param {null | number | NodeLike} node - * @param {string} rune - * @returns {never} - */ -export function rune_invalid_options(node, rune) { - e(node, 'rune_invalid_options', `Options for \`${rune}\` needs to be declared inline\nhttps://svelte.dev/e/rune_invalid_options`); -} - /** * Cannot use `%rune%` rune in non-runes mode * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 022489555ca7..a9ee96257aba 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -121,9 +121,6 @@ export function CallExpression(node, context) { if (node.arguments.length > 2) { e.rune_invalid_arguments_length(node, rune, 'at most two arguments'); } - if (node.arguments.length === 2 && node.arguments[1].type !== 'ObjectExpression') { - e.rune_invalid_options(node.arguments[1], rune); - } } break; diff --git a/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json b/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json deleted file mode 100644 index 0403da6a2812..000000000000 --- a/packages/svelte/tests/validator/samples/rune-invalid-options/errors.json +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - "code": "rune_invalid_options", - "end": { - "column": 29, - "line": 3 - }, - "start": { - "column": 22, - "line": 3 - }, - "message": "Options for `$state` needs to be declared inline" - } -] diff --git a/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte b/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte deleted file mode 100644 index 252737e26cef..000000000000 --- a/packages/svelte/tests/validator/samples/rune-invalid-options/input.svelte +++ /dev/null @@ -1,4 +0,0 @@ - \ No newline at end of file From a97465da2176fb53b7b2219a2cc617e2a77d56a1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:29:44 -0400 Subject: [PATCH 30/50] simplify --- .../compiler/phases/3-transform/client/transform-client.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index b9c8dfd99c63..605c0167866f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -295,11 +295,8 @@ export function client_component(analysis, options) { } if (binding?.kind === 'state' || binding?.kind === 'raw_state') { - const value = - binding.kind === 'state' - ? b.call('$.proxy', b.id('$$value'), b.call('$.get_options', b.id(name))) - : b.id('$$value'); - return [getter, b.set(alias ?? name, [b.stmt(b.call('$.set', b.id(name), value))])]; + const call = b.call('$.set', b.id(name), b.id('$$value'), binding.kind === 'state' && b.true); + return [getter, b.set(alias ?? name, [b.stmt(call)])]; } return getter; From e7fa79a5f686cf4d074097b336b2a517af9add85 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:36:02 -0400 Subject: [PATCH 31/50] simplify --- .../3-transform/client/visitors/ClassBody.js | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index a1306834f03b..c9353c963486 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -116,25 +116,17 @@ export function ClassBody(node, context) { context.visit(definition.value.arguments[0], child_state) ); - let options = - definition.value.arguments.length === 2 - ? /** @type {Expression} **/ ( - context.visit(definition.value.arguments[1], child_state) - ) - : undefined; - - let proxied = should_proxy(init, context.state.scope); + if (field.kind === 'state' || field.kind === 'raw_state') { + let arg = definition.value.arguments[1]; + let options = arg && /** @type {Expression} **/ (context.visit(arg, child_state)); - value = - field.kind === 'state' - ? should_proxy(init, context.state.scope) + value = + field.kind === 'state' && should_proxy(init, context.state.scope) ? b.call('$.assignable_proxy', init, options) - : b.call('$.state', init, options) - : field.kind === 'raw_state' - ? b.call('$.state', init, options) - : field.kind === 'derived_by' - ? b.call('$.derived', init) - : b.call('$.derived', b.thunk(init)); + : b.call('$.state', init, options); + } else { + value = b.call('$derived', field.kind === 'derived_by' ? init : b.thunk(init)); + } } else { // if no arguments, we know it's state as `$derived()` is a compile error value = b.call('$.state'); From c793cf3c9c04d1c7d5725220405861bc02f0559b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:39:08 -0400 Subject: [PATCH 32/50] simplify --- .../client/visitors/VariableDeclaration.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 6656f55a82bf..630ee6cef319 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -127,18 +127,18 @@ export function VariableDeclaration(node, context) { * @param {Expression} [options] */ const create_state_declarator = (id, value, options) => { - const binding = /** @type {import('#compiler').Binding} */ ( - context.state.scope.get(id.name) - ); + const binding = /** @type {Binding} */ (context.state.scope.get(id.name)); const proxied = rune === '$state' && should_proxy(value, context.state.scope); const is_state = is_state_source(binding, context.state.analysis); - if (proxied && is_state) { - value = b.call('$.assignable_proxy', value, options); - } else if (proxied) { - value = b.call('$.proxy', value, options); - } else if (is_state) { - value = b.call('$.state', value, options); + + if (proxied) { + return b.call(is_state ? '$.assignable_proxy' : '$.proxy', value, options); } + + if (is_state) { + return b.call('$.state', value, options); + } + return value; }; From 51ecbef5dcd258ce3234715ee8f577ecde4eeb22 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:40:52 -0400 Subject: [PATCH 33/50] oops --- .../compiler/phases/3-transform/client/visitors/ClassBody.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index c9353c963486..e3f8222502ed 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -125,7 +125,7 @@ export function ClassBody(node, context) { ? b.call('$.assignable_proxy', init, options) : b.call('$.state', init, options); } else { - value = b.call('$derived', field.kind === 'derived_by' ? init : b.thunk(init)); + value = b.call('$.derived', field.kind === 'derived_by' ? init : b.thunk(init)); } } else { // if no arguments, we know it's state as `$derived()` is a compile error From b158c8c5aa325aedc80e24f265b73dc5e06e6db8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:42:44 -0400 Subject: [PATCH 34/50] unused --- .../phases/3-transform/client/visitors/shared/declarations.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js index 77e9831e1123..a13ecfed2ce5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -1,8 +1,7 @@ /** @import { Identifier } from 'estree' */ /** @import { ComponentContext, Context } from '../../types' */ -import { is_state_source, should_proxy } from '../../utils.js'; +import { is_state_source } from '../../utils.js'; import * as b from '../../../../../utils/builders.js'; -import { dev } from '../../../../../state.js'; /** * Turns `foo` into `$.get(foo)` From ea75c5e6fa78f90858b4d6b473e62cba42416ad5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 22:48:28 -0400 Subject: [PATCH 35/50] tidy up --- packages/svelte/src/internal/client/proxy.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 4221822e0fb1..4f847f21af66 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -467,7 +467,6 @@ export function proxy(value, _options, prev) { * @param {ValueOptions} [options] * @returns {Source} */ - export function assignable_proxy(value, options) { return state(proxy(value, options), options); } From 3cb7b7926b61c52e5ee4ceabe7d2d3baecb7b10c Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 21 Mar 2025 15:38:13 +0100 Subject: [PATCH 36/50] chore: split tests --- .../samples/state-onchange-arrays/_config.js | 23 ++++++ .../samples/state-onchange-arrays/main.svelte | 11 +++ .../samples/state-onchange-classes/_config.js | 27 ++++++ .../state-onchange-classes/main.svelte | 37 +++++++++ .../samples/state-onchange-proxies/_config.js | 18 ++++ .../state-onchange-proxies/main.svelte | 10 +++ .../samples/state-onchange/_config.js | 82 +------------------ .../samples/state-onchange/main.svelte | 57 +------------ 8 files changed, 129 insertions(+), 136 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-classes/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js new file mode 100644 index 000000000000..7b89518f7564 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js @@ -0,0 +1,23 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3] = target.querySelectorAll('button'); + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['arr']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['arr', 'arr']); + + flushSync(() => { + btn3.click(); + }); + assert.deepEqual(logs, ['arr', 'arr', 'arr']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/main.svelte new file mode 100644 index 000000000000..41f8c7a948d8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/main.svelte @@ -0,0 +1,11 @@ + + + + + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js new file mode 100644 index 000000000000..acb7054cc4d1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js @@ -0,0 +1,27 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2, btn3] = target.querySelectorAll('button'); + + assert.deepEqual(logs, ['constructor count', 'constructor proxy']); + + logs.length = 0; + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['class count']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['class count', 'class proxy']); + + flushSync(() => { + btn3.click(); + }); + assert.deepEqual(logs, ['class count', 'class proxy', 'class proxy']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/main.svelte new file mode 100644 index 000000000000..a42c1c124511 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/main.svelte @@ -0,0 +1,37 @@ + + + + + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js new file mode 100644 index 000000000000..17d5205d17b4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js @@ -0,0 +1,18 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [btn, btn2] = target.querySelectorAll('button'); + + flushSync(() => { + btn.click(); + }); + assert.deepEqual(logs, ['proxy']); + + flushSync(() => { + btn2.click(); + }); + assert.deepEqual(logs, ['proxy', 'proxy']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/main.svelte new file mode 100644 index 000000000000..5340b231592d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/main.svelte @@ -0,0 +1,10 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js index 82625ded0acc..91d469a1e93b 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js @@ -3,89 +3,11 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { - const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9] = target.querySelectorAll('button'); - - assert.deepEqual(logs, ['constructor count', 'constructor proxy']); - - logs.length = 0; + const btn = target.querySelector('button'); flushSync(() => { - btn.click(); + btn?.click(); }); assert.deepEqual(logs, ['count']); - - flushSync(() => { - btn2.click(); - }); - assert.deepEqual(logs, ['count', 'proxy']); - - flushSync(() => { - btn3.click(); - }); - assert.deepEqual(logs, ['count', 'proxy', 'proxy']); - - flushSync(() => { - btn4.click(); - }); - assert.deepEqual(logs, ['count', 'proxy', 'proxy', 'class count']); - - flushSync(() => { - btn5.click(); - }); - assert.deepEqual(logs, ['count', 'proxy', 'proxy', 'class count', 'class proxy']); - - flushSync(() => { - btn6.click(); - }); - assert.deepEqual(logs, [ - 'count', - 'proxy', - 'proxy', - 'class count', - 'class proxy', - 'class proxy' - ]); - - flushSync(() => { - btn7.click(); - }); - assert.deepEqual(logs, [ - 'count', - 'proxy', - 'proxy', - 'class count', - 'class proxy', - 'class proxy', - 'arr' - ]); - - flushSync(() => { - btn8.click(); - }); - assert.deepEqual(logs, [ - 'count', - 'proxy', - 'proxy', - 'class count', - 'class proxy', - 'class proxy', - 'arr', - 'arr' - ]); - - flushSync(() => { - btn9.click(); - }); - assert.deepEqual(logs, [ - 'count', - 'proxy', - 'proxy', - 'class count', - 'class proxy', - 'class proxy', - 'arr', - 'arr', - 'arr' - ]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte index 565e8477dc26..8dc265b1df72 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/main.svelte @@ -4,61 +4,6 @@ console.log("count"); } }) - - let proxy = $state({count: 0}, { - onchange(){ - console.log("proxy"); - } - }) - - class Test{ - count = $state(0, { - onchange(){ - console.log("class count"); - } - }) - proxy = $state({count: 0}, { - onchange(){ - console.log("class proxy"); - } - }) - - #in_constructor = $state(0, { - onchange(){ - console.log("constructor count"); - } - }); - - #in_constructor_proxy = $state({ count: 0 }, { - onchange(){ - console.log("constructor proxy"); - } - }); - - - constructor(){ - this.#in_constructor = 42; - this.#in_constructor_proxy.count++; - } - } - - const class_test = new Test(); - - let arr = $state([0,1,2], { - onchange(){ - console.log("arr"); - } - }) - - - - - - - - - - - \ No newline at end of file + \ No newline at end of file From 35e4023c350be6bdfab51603e863958c9e6e05d4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Mar 2025 12:45:04 -0400 Subject: [PATCH 37/50] Update packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte --- .../samples/state-onchange-reassign-proxy/main.svelte | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte index d1931cbaac15..8661a3de262c 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/main.svelte @@ -24,7 +24,4 @@ - \ No newline at end of file From 118e9aa91cd7f838d13e817c93bce4b4d59c3648 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Mar 2025 12:49:06 -0400 Subject: [PATCH 38/50] =?UTF-8?q?put=20flushSync=20calls=20on=20single=20l?= =?UTF-8?q?ine=20=E2=80=94=20makes=20it=20easier=20to=20connect=20the=20ev?= =?UTF-8?q?ent=20with=20the=20subsequent=20assertion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../state-onchange-accumulated/_config.js | 8 +--- .../_config.js | 8 +--- .../samples/state-onchange-arrays/_config.js | 12 ++---- .../samples/state-onchange-classes/_config.js | 12 ++---- .../_config.js | 29 ++++---------- .../samples/state-onchange-proxies/_config.js | 8 +--- .../state-onchange-reassign-proxy/_config.js | 20 +++------- .../samples/state-onchange/_config.js | 4 +- .../samples/state-raw-onchange/_config.js | 40 +++++-------------- 9 files changed, 36 insertions(+), 105 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js index f99a1c856b75..619fab8a2caa 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-accumulated/_config.js @@ -5,14 +5,10 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2] = target.querySelectorAll('button'); - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['foo', 'baz']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['foo', 'baz', 'foo', 'baz']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js index 64f81ac0e2ea..3ad5b749ee74 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-array-length-batch/_config.js @@ -5,14 +5,10 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2] = target.querySelectorAll('button'); - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, [[{}, {}, {}, {}, {}, {}, {}, {}]]); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, [[{}, {}, {}, {}, {}, {}, {}, {}], []]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js index 7b89518f7564..d77d3f9aa707 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-arrays/_config.js @@ -5,19 +5,13 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2, btn3] = target.querySelectorAll('button'); - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['arr']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['arr', 'arr']); - flushSync(() => { - btn3.click(); - }); + flushSync(() => btn3.click()); assert.deepEqual(logs, ['arr', 'arr', 'arr']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js index acb7054cc4d1..dc9fe8bfb148 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-classes/_config.js @@ -9,19 +9,13 @@ export default test({ logs.length = 0; - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['class count']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['class count', 'class proxy']); - flushSync(() => { - btn3.click(); - }); + flushSync(() => btn3.click()); assert.deepEqual(logs, ['class count', 'class proxy', 'class proxy']); } }); 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 cc0bfbb1e382..ad79aa009243 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 @@ -6,28 +6,15 @@ export default test({ const [btn, btn2, btn3, btn4, btn5, btn6] = target.querySelectorAll('button'); logs.length = 0; - flushSync(() => { - btn.click(); - }); - flushSync(() => { - btn2.click(); - }); - flushSync(() => { - btn3.click(); - }); - flushSync(() => { - btn4.click(); - }); - flushSync(() => { - btn5.click(); - }); + flushSync(() => btn.click()); + flushSync(() => btn2.click()); + flushSync(() => btn3.click()); + flushSync(() => btn4.click()); + flushSync(() => btn5.click()); assert.deepEqual(logs, []); - flushSync(() => { - btn6.click(); - }); - flushSync(() => { - btn.click(); - }); + + flushSync(() => btn6.click()); + flushSync(() => btn.click()); assert.deepEqual(logs, ['arr', 'arr']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js index 17d5205d17b4..42cbcef00535 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-proxies/_config.js @@ -5,14 +5,10 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2] = target.querySelectorAll('button'); - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['proxy']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['proxy', 'proxy']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js index 92ddd1369d50..d9e2a1fdadae 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-reassign-proxy/_config.js @@ -5,26 +5,16 @@ export default test({ async test({ assert, target, logs }) { const [btn, btn2, btn3, btn4] = target.querySelectorAll('button'); - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['a']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['a', 'b', 'c']); - flushSync(() => { - btn3.click(); - }); + flushSync(() => btn3.click()); assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c']); - flushSync(() => { - btn4.click(); - }); + flushSync(() => btn4.click()); assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c', 'c']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['a', 'b', 'c', 'b', 'c', 'c', 'b']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js index 91d469a1e93b..ecade967c2a9 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange/_config.js @@ -5,9 +5,7 @@ export default test({ async test({ assert, target, logs }) { const btn = target.querySelector('button'); - flushSync(() => { - btn?.click(); - }); + flushSync(() => btn?.click()); assert.deepEqual(logs, ['count']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js index ab2a125c1213..d1df444372b4 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js @@ -10,54 +10,34 @@ export default test({ logs.length = 0; - flushSync(() => { - btn.click(); - }); + flushSync(() => btn.click()); assert.deepEqual(logs, ['count']); - flushSync(() => { - btn2.click(); - }); + flushSync(() => btn2.click()); assert.deepEqual(logs, ['count']); - flushSync(() => { - btn3.click(); - }); + flushSync(() => btn3.click()); assert.deepEqual(logs, ['count', 'proxy']); - flushSync(() => { - btn4.click(); - }); + flushSync(() => btn4.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count']); - flushSync(() => { - btn5.click(); - }); + flushSync(() => btn5.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count']); - flushSync(() => { - btn6.click(); - }); + flushSync(() => btn6.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); - flushSync(() => { - btn7.click(); - }); + flushSync(() => btn7.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); - flushSync(() => { - btn8.click(); - }); + flushSync(() => btn8.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); - flushSync(() => { - btn9.click(); - }); + flushSync(() => btn9.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); - flushSync(() => { - btn10.click(); - }); + flushSync(() => btn10.click()); assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy', 'arr']); } }); From 36bfef986a7a0d777ab9a5f350775843c166c17d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Mar 2025 12:51:33 -0400 Subject: [PATCH 39/50] not a proxy! --- .../samples/state-raw-onchange/_config.js | 18 ++++++++--------- .../samples/state-raw-onchange/main.svelte | 20 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js index d1df444372b4..ce41a485c527 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/_config.js @@ -6,7 +6,7 @@ export default test({ const [btn, btn2, btn3, btn4, btn5, btn6, btn7, btn8, btn9, btn10] = target.querySelectorAll('button'); - assert.deepEqual(logs, ['constructor count', 'constructor proxy']); + assert.deepEqual(logs, ['constructor count', 'constructor object']); logs.length = 0; @@ -17,27 +17,27 @@ export default test({ assert.deepEqual(logs, ['count']); flushSync(() => btn3.click()); - assert.deepEqual(logs, ['count', 'proxy']); + assert.deepEqual(logs, ['count', 'object']); flushSync(() => btn4.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count']); + assert.deepEqual(logs, ['count', 'object', 'class count']); flushSync(() => btn5.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count']); + assert.deepEqual(logs, ['count', 'object', 'class count']); flushSync(() => btn6.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + assert.deepEqual(logs, ['count', 'object', 'class count', 'class object']); flushSync(() => btn7.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + assert.deepEqual(logs, ['count', 'object', 'class count', 'class object']); flushSync(() => btn8.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + assert.deepEqual(logs, ['count', 'object', 'class count', 'class object']); flushSync(() => btn9.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy']); + assert.deepEqual(logs, ['count', 'object', 'class count', 'class object']); flushSync(() => btn10.click()); - assert.deepEqual(logs, ['count', 'proxy', 'class count', 'class proxy', 'arr']); + assert.deepEqual(logs, ['count', 'object', 'class count', 'class object', 'arr']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte index 3879e32621a0..cc19a21d4f9f 100644 --- a/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/state-raw-onchange/main.svelte @@ -5,9 +5,9 @@ } }) - let proxy = $state.raw({count: 0}, { + let object = $state.raw({count: 0}, { onchange(){ - console.log("proxy"); + console.log("object"); } }) @@ -17,9 +17,9 @@ console.log("class count"); } }) - proxy = $state.raw({count: 0}, { + object = $state.raw({count: 0}, { onchange(){ - console.log("class proxy"); + console.log("class object"); } }) @@ -31,7 +31,7 @@ #in_constructor_proxy = $state({ count: 0 }, { onchange(){ - console.log("constructor proxy"); + console.log("constructor object"); } }); @@ -52,14 +52,14 @@ - - + + - - + + - \ No newline at end of file + From 714c042e03f00c43550c14ff91d14a079e55c959 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Mar 2025 09:14:29 -0400 Subject: [PATCH 40/50] extract `onchange` callbacks from options (#15579) * WIP * extract onchange callbacks * const * tweak * docs * fix: unwrap args in case of spread * fix: revert unwrap args in case of spread --------- Co-authored-by: paoloricciuti --- .../3-transform/client/visitors/ClassBody.js | 12 ++- .../client/visitors/VariableDeclaration.js | 17 +++-- .../client/visitors/shared/state.js | 31 ++++++++ packages/svelte/src/internal/client/index.js | 10 +-- packages/svelte/src/internal/client/proxy.js | 73 +++++++------------ .../src/internal/client/reactivity/sources.js | 18 ++--- .../src/internal/client/reactivity/types.d.ts | 4 +- 7 files changed, 82 insertions(+), 83 deletions(-) create mode 100644 packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/state.js diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index e3f8222502ed..b67bb8893c30 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -6,6 +6,7 @@ import * as b from '../../../../utils/builders.js'; import { regex_invalid_identifier_chars } from '../../../patterns.js'; import { get_rune } from '../../../scope.js'; import { should_proxy } from '../utils.js'; +import { get_onchange } from './shared/state.js'; /** * @param {ClassBody} node @@ -117,13 +118,16 @@ export function ClassBody(node, context) { ); if (field.kind === 'state' || field.kind === 'raw_state') { - let arg = definition.value.arguments[1]; - let options = arg && /** @type {Expression} **/ (context.visit(arg, child_state)); + const onchange = get_onchange( + /** @type {Expression} */ (definition.value.arguments[1]), + // @ts-ignore mismatch between Context and ComponentContext. TODO look into + context + ); value = field.kind === 'state' && should_proxy(init, context.state.scope) - ? b.call('$.assignable_proxy', init, options) - : b.call('$.state', init, options); + ? b.call('$.assignable_proxy', init, onchange) + : b.call('$.state', init, onchange); } else { value = b.call('$.derived', field.kind === 'derived_by' ? init : b.thunk(init)); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 630ee6cef319..d12735d15102 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -8,6 +8,7 @@ import * as assert from '../../../../utils/assert.js'; import { get_rune } from '../../../scope.js'; import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js'; import { is_hoisted_function } from '../../utils.js'; +import { get_onchange } from './shared/state.js'; /** * @param {VariableDeclaration} node @@ -117,26 +118,26 @@ export function VariableDeclaration(node, context) { const args = /** @type {CallExpression} */ (init).arguments; const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0; - let options = - args.length === 2 ? /** @type {Expression} */ (context.visit(args[1])) : undefined; + + const onchange = get_onchange(/** @type {Expression} */ (args[1]), context); if (rune === '$state' || rune === '$state.raw') { /** * @param {Identifier} id * @param {Expression} value - * @param {Expression} [options] + * @param {Expression} [onchange] */ - const create_state_declarator = (id, value, options) => { + const create_state_declarator = (id, value, onchange) => { const binding = /** @type {Binding} */ (context.state.scope.get(id.name)); const proxied = rune === '$state' && should_proxy(value, context.state.scope); const is_state = is_state_source(binding, context.state.analysis); if (proxied) { - return b.call(is_state ? '$.assignable_proxy' : '$.proxy', value, options); + return b.call(is_state ? '$.assignable_proxy' : '$.proxy', value, onchange); } if (is_state) { - return b.call('$.state', value, options); + return b.call('$.state', value, onchange); } return value; @@ -144,7 +145,7 @@ export function VariableDeclaration(node, context) { if (declarator.id.type === 'Identifier') { declarations.push( - b.declarator(declarator.id, create_state_declarator(declarator.id, value, options)) + b.declarator(declarator.id, create_state_declarator(declarator.id, value, onchange)) ); } else { const tmp = context.state.scope.generate('tmp'); @@ -157,7 +158,7 @@ export function VariableDeclaration(node, context) { return b.declarator( path.node, binding?.kind === 'state' || binding?.kind === 'raw_state' - ? create_state_declarator(binding.node, value, options) + ? create_state_declarator(binding.node, value, onchange) : value ); }) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/state.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/state.js new file mode 100644 index 000000000000..0f8a7b1b5bfd --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/state.js @@ -0,0 +1,31 @@ +/** @import { Expression, Property } from 'estree' */ +/** @import { ComponentContext } from '../../types' */ +import * as b from '../../../../../utils/builders.js'; + +/** + * Extract the `onchange` callback from the options passed to `$state` + * @param {Expression} options + * @param {ComponentContext} context + * @returns {Expression | undefined} + */ +export function get_onchange(options, context) { + if (!options) return; + + if (options.type === 'ObjectExpression') { + const onchange = /** @type {Property | undefined} */ ( + options.properties.find( + (property) => + property.type === 'Property' && + !property.computed && + property.key.type === 'Identifier' && + property.key.name === 'onchange' + ) + ); + + if (!onchange) return; + + return /** @type {Expression} */ (context.visit(onchange.value)); + } + + return b.member(/** @type {Expression} */ (context.visit(options)), 'onchange'); +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index d1293a58705b..a1e7c44370c1 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -113,15 +113,7 @@ export { user_effect, user_pre_effect } from './reactivity/effects.js'; -export { - mutable_source, - mutate, - set, - state, - update, - update_pre, - get_options -} from './reactivity/sources.js'; +export { mutable_source, mutate, set, state, update, update_pre } from './reactivity/sources.js'; export { prop, rest_props, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index c69afda4d951..6efbfe2cbd20 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -28,47 +28,33 @@ function identity(fn) { return fn; } -/** - * @param {ValueOptions | undefined} options - * @returns {ValueOptions | undefined} - */ -function clone_options(options) { - return options != null - ? { - onchange: options.onchange - } - : undefined; -} - /** @type {ProxyMetadata | null} */ var parent_metadata = null; /** * @template T * @param {T} value - * @param {ValueOptions} [_options] + * @param {() => void} [onchange] * @param {Source} [prev] dev mode only * @returns {T} */ -export function proxy(value, _options, prev) { +export function proxy(value, onchange, prev) { // if non-proxyable, or is already a proxy, return `value` if (typeof value !== 'object' || value === null) { return value; } - var options = clone_options(_options); - if (STATE_SYMBOL in value) { // @ts-ignore - value[PROXY_ONCHANGE_SYMBOL](options?.onchange); + value[PROXY_ONCHANGE_SYMBOL](onchange); return value; } - if (options?.onchange) { + if (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 = () => { + var onchanges = new Set([onchange]); + onchange = () => { for (let onchange of onchanges) { onchange(); } @@ -116,10 +102,7 @@ export function proxy(value, _options, prev) { 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, clone_options(options), stack) - ); + sources.set('length', source(/** @type {any[]} */ (value).length, onchange, stack)); } /** @type {ProxyMetadata} */ @@ -165,12 +148,12 @@ export function proxy(value, _options, prev) { var s = sources.get(prop); if (s === undefined) { - s = with_parent(() => source(descriptor.value, clone_options(options), stack)); + s = with_parent(() => source(descriptor.value, onchange, stack)); sources.set(prop, s); } else { set( s, - with_parent(() => proxy(descriptor.value, options)) + with_parent(() => proxy(descriptor.value, onchange)) ); } @@ -184,7 +167,7 @@ export function proxy(value, _options, prev) { if (prop in target) { sources.set( prop, - with_parent(() => source(UNINITIALIZED, clone_options(options), stack)) + with_parent(() => source(UNINITIALIZED, onchange, stack)) ); } } else { @@ -201,7 +184,7 @@ export function proxy(value, _options, prev) { // 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); + s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } set(s, UNINITIALIZED); update_version(version); @@ -227,18 +210,14 @@ export function proxy(value, _options, prev) { // 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) { + if (onchange != null && value && !remove) { onchanges?.add?.(value); - } else if (options?.onchange != null && value) { + } else if (onchange != null && value) { onchanges?.delete?.(value); } else { - options = { - onchange: value - }; + onchange = value; for (let [, s] of sources) { - if (s.o) { - s.o.onchange = value; - } + s.o = value; } } }; @@ -249,7 +228,7 @@ export function proxy(value, _options, prev) { // 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)) { - let opt = clone_options(options); + let opt = onchange; s = with_parent(() => source(proxy(exists ? target[prop] : UNINITIALIZED, opt), opt, stack) ); @@ -281,7 +260,7 @@ export function proxy(value, _options, prev) { if ( is_proxied_array && - options?.onchange != null && + onchange != null && array_methods.includes(/** @type {string} */ (prop)) ) { return batch_onchange(v); @@ -330,7 +309,7 @@ export function proxy(value, _options, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - let opt = clone_options(options); + let opt = onchange; s = with_parent(() => source(has ? proxy(target[prop], opt) : UNINITIALIZED, opt, stack)); sources.set(prop, s); } @@ -362,14 +341,14 @@ export function proxy(value, _options, prev) { other_s.v !== null && STATE_SYMBOL in other_s.v ) { - other_s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true); + other_s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } 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 = with_parent(() => source(UNINITIALIZED, clone_options(options), stack)); + other_s = with_parent(() => source(UNINITIALIZED, onchange, stack)); sources.set(i + '', other_s); } } @@ -381,7 +360,7 @@ export function proxy(value, _options, prev) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - const opt = clone_options(options); + const opt = onchange; s = with_parent(() => source(undefined, opt, stack)); set( s, @@ -394,11 +373,11 @@ export function proxy(value, _options, prev) { // 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); + s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } set( s, - with_parent(() => proxy(value, clone_options(options))) + with_parent(() => proxy(value, onchange)) ); } })(); @@ -464,11 +443,11 @@ export function proxy(value, _options, prev) { /** * @template T * @param {T} value - * @param {ValueOptions} [options] + * @param {() => void} [onchange] * @returns {Source} */ -export function assignable_proxy(value, options) { - return state(proxy(value, options), options); +export function assignable_proxy(value, onchange) { + return state(proxy(value, onchange), onchange); } /** diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index a20871b3da91..62b8e5dc770d 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -77,7 +77,7 @@ export function batch_onchange(fn) { /** * @template V * @param {V} v - * @param {ValueOptions} [o] + * @param {() => void} [o] * @param {Error | null} [stack] * @returns {Source} */ @@ -102,18 +102,10 @@ export function source(v, o, stack) { return signal; } -/** - * @param {Source} source - * @returns {ValueOptions | undefined} - */ -export function get_options(source) { - return source.o; -} - /** * @template V * @param {V} v - * @param {ValueOptions} [o] + * @param {() => void} [o] * @param {Error | null} [stack] */ export function state(v, o, stack) { @@ -196,11 +188,11 @@ export function internal_set(source, value) { if (!source.equals(value)) { var old_value = source.v; - if (typeof old_value === 'object' && old_value != null && source.o?.onchange) { + if (typeof old_value === 'object' && old_value != null && source.o) { // @ts-ignore const remove = old_value[PROXY_ONCHANGE_SYMBOL]; if (remove && typeof remove === 'function') { - remove(source.o?.onchange, true); + remove(source.o, true); } } @@ -257,7 +249,7 @@ export function internal_set(source, value) { inspect_effects.clear(); } - var onchange = source.o?.onchange; + var onchange = source.o; if (onchange) { if (onchange_batch) { onchange_batch.add(onchange); diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index c0d490d5c7ab..c43363a9b146 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -20,8 +20,8 @@ export interface Value extends Signal { rv: number; /** The latest value for this signal */ v: V; - /** Options for the source */ - o?: ValueOptions; + /** onchange callback */ + o?: () => void; /** Dev only */ created?: Error | null; updated?: Error | null; From af70cef5a09d5e663680517e0ec8dffc15af7ba4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 11 Apr 2025 22:11:59 -0400 Subject: [PATCH 41/50] ValueOptions -> StateOptions --- packages/svelte/src/index.d.ts | 2 +- packages/svelte/src/internal/client/proxy.js | 2 +- packages/svelte/src/internal/client/reactivity/sources.js | 2 +- packages/svelte/src/internal/client/reactivity/types.d.ts | 2 +- packages/svelte/src/internal/client/types.d.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index 594c063cfed0..ca4accc26ff3 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -352,6 +352,6 @@ export type MountOptions = Record props: Props; }); -export { ValueOptions as StateOptions } from './internal/client/types.js'; +export { StateOptions } from './internal/client/types.js'; export * from './index-client.js'; diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index fe1f358501a8..ef5db6a9d829 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -1,4 +1,4 @@ -/** @import { Source, ValueOptions } from '#client' */ +/** @import { Source } from '#client' */ import { DEV } from 'esm-env'; import { UNINITIALIZED } from '../../constants.js'; import { tracing_mode_flag } from '../flags/index.js'; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index cb50cad39c03..02147eec6e8a 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -1,4 +1,4 @@ -/** @import { Derived, Effect, Reaction, Source, Value, ValueOptions } from '#client' */ +/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */ import { DEV } from 'esm-env'; import { active_reaction, diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index c43363a9b146..662a00a926c6 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -7,7 +7,7 @@ export interface Signal { wv: number; } -export interface ValueOptions { +export interface StateOptions { onchange?: () => unknown; } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index b57e3b7356dc..b46bdf201341 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -1,6 +1,6 @@ import type { Store } from '#shared'; import { STATE_SYMBOL } from './constants.js'; -import type { Effect, Source, Value, Reaction, ValueOptions } from './reactivity/types.js'; +import type { Effect, Source, Value, Reaction } from './reactivity/types.js'; type EventCallback = (event: Event) => boolean; export type EventCallbackMap = Record; From bde02520363e6563301cfd787d38a9f79d531b68 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 11:16:04 -0400 Subject: [PATCH 42/50] tweak docs --- documentation/docs/02-runes/02-$state.md | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index 26541072f381..fc96e6517e7e 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -149,32 +149,19 @@ This can improve performance with large arrays and objects that you weren't plan ## State options -Both `$state` and `$state.raw` can optionally accept a second argument. This allows you to specify an `onchange` function that will be called synchronously whenever the state value changes (for `$state` it will also be called for deep mutations). +Both `$state` and `$state.raw` accept an optional second argument that includes an `onchange` function. -The `onchange` function is untracked so even if you assign within an `$effect` it will not cause unwanted dependencies. +This function is called synchronously whenever the value is reassigned or (for `$state`) mutated, allowing you to respond to changes before [effects]($effect) run. It's useful for — for example — persisting data, or validating it: ```js let count = $state(0, { - onchange(){ - console.log("count is now", count); + onchange() { + count = Math.min(count, 10); } }); - -// this will log "count is now 1" -count++; ``` -this could be especially useful if you want to sync some stateful variable that could be mutated without using an effect. - -```js -let array = $state([], { - onchange(){ - localStorage.setItem('array', JSON.stringify(array)); - } -}); - -array.push(array.length); -``` +> The `onchange` function is [untracked](svelte#untrack). ## `$state.snapshot` From c4182f51ec61d186d11d7f6b0d9e92d1c5fed92b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 11:33:11 -0400 Subject: [PATCH 43/50] simplify --- packages/svelte/src/internal/client/reactivity/sources.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 5b3ef9215094..ef6d0405ec76 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -186,10 +186,7 @@ export function internal_set(source, value) { if (typeof old_value === 'object' && old_value != null && source.o) { // @ts-ignore - const remove = old_value[PROXY_ONCHANGE_SYMBOL]; - if (remove && typeof remove === 'function') { - remove(source.o, true); - } + old_value[PROXY_ONCHANGE_SYMBOL]?.(source.o, true); } if (is_destroying_effect) { From 0e57669f0477460f3c6270dd6057ec3f328d84ad Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 11:38:33 -0400 Subject: [PATCH 44/50] cosmetic tweak --- packages/svelte/src/internal/client/proxy.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index ef5db6a9d829..fe41663a3e62 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -141,11 +141,13 @@ export function proxy(value, onchange) { 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](onchange, true); } + set(s, UNINITIALIZED); update_version(version); } From 42f73a0225c6526022ded50b6e18bb59edaaba7c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 11:44:09 -0400 Subject: [PATCH 45/50] tweak conditions --- packages/svelte/src/internal/client/proxy.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index fe41663a3e62..58a541f219ae 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -168,10 +168,12 @@ export function proxy(value, onchange) { // 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 (onchange != null && value && !remove) { - onchanges?.add?.(value); - } else if (onchange != null && value) { - onchanges?.delete?.(value); + if (onchange != null && value) { + if (remove) { + onchanges?.delete(value); + } else { + onchanges?.add(value); + } } else { onchange = value; for (let [, s] of sources) { From dbf2b4c099d229b4e309bb63f8f3fd77ed3318f8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 16:52:14 -0400 Subject: [PATCH 46/50] simplify --- packages/svelte/src/internal/client/proxy.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 58a541f219ae..5787e7cdc68f 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -300,11 +300,10 @@ export function proxy(value, onchange) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - const opt = onchange; - s = with_parent(() => source(undefined, opt, stack)); + s = with_parent(() => source(undefined, onchange, stack)); set( s, - with_parent(() => proxy(value, opt)) + with_parent(() => proxy(value, onchange)) ); sources.set(prop, s); } From d5f785bd7eecc581cf53495a7d1903c9df106693 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 17:13:17 -0400 Subject: [PATCH 47/50] add a failing test --- .../samples/state-onchange-child/_config.js | 11 +++++++++++ .../samples/state-onchange-child/main.svelte | 13 +++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-child/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-child/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-child/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-child/_config.js new file mode 100644 index 000000000000..76380eddc96c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-child/_config.js @@ -0,0 +1,11 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + assert.deepEqual(logs, ['b changed']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-child/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-child/main.svelte new file mode 100644 index 000000000000..e1f60934220b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-child/main.svelte @@ -0,0 +1,13 @@ + + + From d8e60f014089c6d1201a463c1d5db01d3d4ee0e9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 17:42:05 -0400 Subject: [PATCH 48/50] fix --- packages/svelte/src/internal/client/proxy.js | 33 +++++++++----------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 5787e7cdc68f..633c6a0eeb1e 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -39,8 +39,11 @@ export function proxy(value, onchange) { } if (STATE_SYMBOL in value) { - // @ts-ignore - value[PROXY_ONCHANGE_SYMBOL](onchange); + if (onchange) { + // @ts-ignore + value[PROXY_ONCHANGE_SYMBOL](onchange); + } + return value; } @@ -144,7 +147,7 @@ export function proxy(value, onchange) { // 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) { + if (onchange && typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) { s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } @@ -161,24 +164,15 @@ export function proxy(value, onchange) { } if (prop === PROXY_ONCHANGE_SYMBOL) { - return ( - /** @type {(() => unknown) | undefined} */ value, - /** @type {boolean} */ remove - ) => { + return (/** @type {(() => unknown)} */ 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 (onchange != null && value) { - if (remove) { - onchanges?.delete(value); - } else { - onchanges?.add(value); - } + // if (onchange != null && value) { + if (remove) { + onchanges?.delete(value); } else { - onchange = value; - for (let [, s] of sources) { - s.o = value; - } + onchanges?.add(value); } }; } @@ -277,6 +271,7 @@ export function proxy(value, onchange) { var other_s = sources.get(i + ''); if (other_s !== undefined) { if ( + onchange && typeof other_s.v === 'object' && other_s.v !== null && STATE_SYMBOL in other_s.v @@ -309,11 +304,13 @@ export function proxy(value, onchange) { } } else { has = s.v !== UNINITIALIZED; + // 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) { + if (onchange && typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) { s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } + set( s, with_parent(() => proxy(value, onchange)) From 6c9380c0ecc58e304f4fa19d61d22e0a12f15344 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 17:46:42 -0400 Subject: [PATCH 49/50] failing test --- .../samples/state-onchange-after-mutate/_config.js | 11 +++++++++++ .../state-onchange-after-mutate/main.svelte | 14 ++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/_config.js b/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/_config.js new file mode 100644 index 000000000000..3985156f0379 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/_config.js @@ -0,0 +1,11 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + assert.deepEqual(logs, [{ message: 'hello' }, { message: 'goodbye' }]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/main.svelte new file mode 100644 index 000000000000..1a1fc4089130 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-onchange-after-mutate/main.svelte @@ -0,0 +1,14 @@ + + + From ac05b73380095f7e399716b97b1436e75f8b09d9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 17:50:35 -0400 Subject: [PATCH 50/50] fix --- packages/svelte/src/internal/client/proxy.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 633c6a0eeb1e..806f9a324ad5 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -296,10 +296,6 @@ export function proxy(value, onchange) { if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { s = with_parent(() => source(undefined, onchange, stack)); - set( - s, - with_parent(() => proxy(value, onchange)) - ); sources.set(prop, s); } } else { @@ -310,7 +306,9 @@ export function proxy(value, onchange) { if (onchange && typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) { s.v[PROXY_ONCHANGE_SYMBOL](onchange, true); } + } + if (s !== undefined) { set( s, with_parent(() => proxy(value, onchange))