From 4ed4351c54022523018bcbcb213694d883d896d9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 21 Jan 2025 15:24:37 +0100 Subject: [PATCH] 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 4256b84962..664b909a09 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 06a010806a..4d34f2c468 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 8f97f58095..5b8969d5ee 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 405a198050..32b93a2b0a 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 4b1985a1f9..82625ded0a 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 efa688fc15..11d102682a 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 49b367f787..ab2a125c12 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 5f05dd7d0a..3879e32621 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 331e94597d..bc45168642 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); } }