From 5ace53d0178acc494074174549756db903a152f8 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sat, 22 Mar 2025 19:10:48 +0100 Subject: [PATCH 1/7] fix: set deriveds as `CLEAN` if they are assigned to --- .changeset/nervous-kids-shake.md | 5 +++++ .../src/internal/client/reactivity/sources.js | 4 ++++ packages/svelte/tests/signals/test.ts | 15 +++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 .changeset/nervous-kids-shake.md diff --git a/.changeset/nervous-kids-shake.md b/.changeset/nervous-kids-shake.md new file mode 100644 index 000000000000..3fc642979738 --- /dev/null +++ b/.changeset/nervous-kids-shake.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: set deriveds as `CLEAN` if they are assigned to diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index e4834902fe3f..e111530a54ea 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -171,6 +171,10 @@ export function internal_set(source, value) { } } + if ((source.f & DERIVED) !== 0) { + set_signal_status(/** @type {Derived} */ (source), CLEAN); + } + mark_reactions(source, DIRTY); // It's possible that the current reaction might not have up-to-date dependencies diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 3977caae36ad..3d883c2c78d8 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1080,6 +1080,21 @@ describe('signals', () => { }; }); + test.only("deriveds set after they are DIRTY doesn't get updated on get", () => { + return () => { + const a = state(0); + const b = derived(() => $.get(a)); + + set(b, 1); + assert.equal($.get(b), 1); + + set(a, 2); + set(b, 3); + + assert.equal($.get(b), 3); + }; + }); + test('deriveds containing effects work correctly when used with untrack', () => { return () => { let a = render_effect(() => {}); From 908861efdbb2e3a0f089559c9a1c4c8dda4d9457 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Sat, 22 Mar 2025 19:19:30 +0100 Subject: [PATCH 2/7] chore: remove only --- packages/svelte/tests/signals/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 3d883c2c78d8..5577171b07f8 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1080,7 +1080,7 @@ describe('signals', () => { }; }); - test.only("deriveds set after they are DIRTY doesn't get updated on get", () => { + test("deriveds set after they are DIRTY doesn't get updated on get", () => { return () => { const a = state(0); const b = derived(() => $.get(a)); From 3bb648160323c21960bace11f54688798a5c7df4 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Sat, 22 Mar 2025 19:23:18 +0100 Subject: [PATCH 3/7] chore: remove unnecessary typecast --- 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 e111530a54ea..b80d68ac7685 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -172,7 +172,7 @@ export function internal_set(source, value) { } if ((source.f & DERIVED) !== 0) { - set_signal_status(/** @type {Derived} */ (source), CLEAN); + set_signal_status(source, CLEAN); } mark_reactions(source, DIRTY); From 81d972f6456ae5abbb74a6a4eb59043ef3924501 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 23 Mar 2025 10:25:46 +0100 Subject: [PATCH 4/7] fix: set unowned as `MAYBE_DIRTY` instead of `CLEAN` --- 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 b80d68ac7685..866fe85d3b92 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -172,7 +172,7 @@ export function internal_set(source, value) { } if ((source.f & DERIVED) !== 0) { - set_signal_status(source, CLEAN); + set_signal_status(source, (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY); } mark_reactions(source, DIRTY); From 4e281a5a42d769fbc021469121b0d9eac4f9c2dc Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 23 Mar 2025 17:50:48 +0100 Subject: [PATCH 5/7] fix: visit the derived function when to update the dependencies even when it's reassigned --- .../svelte/src/internal/client/constants.js | 19 ++++++++++--------- .../src/internal/client/reactivity/sources.js | 8 ++++++-- .../svelte/src/internal/client/runtime.js | 10 ++++++++-- packages/svelte/tests/signals/test.ts | 17 +++++++++++++++++ 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 21377c1cc85f..a673da268818 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -10,17 +10,18 @@ export const DISCONNECTED = 1 << 9; export const CLEAN = 1 << 10; export const DIRTY = 1 << 11; export const MAYBE_DIRTY = 1 << 12; -export const INERT = 1 << 13; -export const DESTROYED = 1 << 14; -export const EFFECT_RAN = 1 << 15; +export const ASSIGNED_DERIVED = 1 << 13; +export const INERT = 1 << 14; +export const DESTROYED = 1 << 15; +export const EFFECT_RAN = 1 << 16; /** 'Transparent' effects do not create a transition boundary */ -export const EFFECT_TRANSPARENT = 1 << 16; +export const EFFECT_TRANSPARENT = 1 << 17; /** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */ -export const LEGACY_DERIVED_PROP = 1 << 17; -export const INSPECT_EFFECT = 1 << 18; -export const HEAD_EFFECT = 1 << 19; -export const EFFECT_HAS_DERIVED = 1 << 20; -export const EFFECT_IS_UPDATING = 1 << 21; +export const LEGACY_DERIVED_PROP = 1 << 18; +export const INSPECT_EFFECT = 1 << 19; +export const HEAD_EFFECT = 1 << 20; +export const EFFECT_HAS_DERIVED = 1 << 21; +export const EFFECT_IS_UPDATING = 1 << 22; export const STATE_SYMBOL = Symbol('$state'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 866fe85d3b92..199f0f9c67c1 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -29,7 +29,7 @@ import { MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, - EFFECT_IS_UPDATING + ASSIGNED_DERIVED } from '../constants.js'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -172,7 +172,11 @@ export function internal_set(source, value) { } if ((source.f & DERIVED) !== 0) { - set_signal_status(source, (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY); + // if we are assigning a derived we set it to clean but we also + // keep the assigned derived flag so that we can read the dependencies + // in the get function + var flags = (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY; + set_signal_status(source, flags | ASSIGNED_DERIVED); } mark_reactions(source, DIRTY); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a5d26412a4e6..546c7348a1d0 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -23,7 +23,8 @@ import { LEGACY_DERIVED_PROP, DISCONNECTED, BOUNDARY_EFFECT, - EFFECT_IS_UPDATING + EFFECT_IS_UPDATING, + ASSIGNED_DERIVED } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; @@ -926,6 +927,11 @@ export function get(signal) { if (check_dirtiness(derived)) { update_derived(derived); + } else if ((derived.f & ASSIGNED_DERIVED) !== 0) { + // if the derived is clean but has been assigned a new value, then we need to + // still invoke the derived function and update the dependencies or else + // it will not depend on the original source + update_reaction(derived); } } @@ -1043,7 +1049,7 @@ export function untrack(fn) { } } -const STATUS_MASK = ~(DIRTY | MAYBE_DIRTY | CLEAN); +const STATUS_MASK = ~(DIRTY | MAYBE_DIRTY | CLEAN | ASSIGNED_DERIVED); /** * @param {Signal} signal diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 5577171b07f8..3a427e939274 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1089,12 +1089,29 @@ describe('signals', () => { assert.equal($.get(b), 1); set(a, 2); + assert.equal($.get(b), 2); set(b, 3); assert.equal($.get(b), 3); }; }); + test("unowned deriveds set after they are DIRTY doesn't get updated on get", () => { + return () => { + const a = state(0); + const b = derived(() => $.get(a)); + const c = derived(() => $.get(b)); + + set(b, 1); + assert.equal($.get(c), 1); + + set(a, 2); + + assert.equal($.get(b), 2); + assert.equal($.get(c), 2); + }; + }); + test('deriveds containing effects work correctly when used with untrack', () => { return () => { let a = render_effect(() => {}); From 097fbb48a01d7efa8a082580a8c794c06828b01f Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 23 Mar 2025 17:55:21 +0100 Subject: [PATCH 6/7] fix: use `execute_derived` instead of `update_reaction` --- packages/svelte/src/internal/client/reactivity/deriveds.js | 2 +- packages/svelte/src/internal/client/runtime.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index cd7bbba02f91..fe27d5f4b9fa 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -130,7 +130,7 @@ function get_derived_parent_effect(derived) { * @param {Derived} derived * @returns {T} */ -function execute_derived(derived) { +export function execute_derived(derived) { var value; var prev_active_effect = active_effect; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 546c7348a1d0..6daf5a5cb89e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -28,7 +28,7 @@ import { } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; -import { destroy_derived_effects, update_derived } from './reactivity/deriveds.js'; +import { destroy_derived_effects, execute_derived, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; import { tracing_mode_flag } from '../flags/index.js'; @@ -931,7 +931,7 @@ export function get(signal) { // if the derived is clean but has been assigned a new value, then we need to // still invoke the derived function and update the dependencies or else // it will not depend on the original source - update_reaction(derived); + execute_derived(derived); } } From b56c71afd701c1aa38cf0bc7a500c1fed6ab740d Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 2 Apr 2025 19:19:40 +0200 Subject: [PATCH 7/7] fix: execute deriveds eagerly when they are set if DIRTY --- .../svelte/src/internal/client/constants.js | 19 +++++++++---------- .../src/internal/client/reactivity/sources.js | 14 +++++++------- .../svelte/src/internal/client/runtime.js | 10 ++-------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a673da268818..21377c1cc85f 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -10,18 +10,17 @@ export const DISCONNECTED = 1 << 9; export const CLEAN = 1 << 10; export const DIRTY = 1 << 11; export const MAYBE_DIRTY = 1 << 12; -export const ASSIGNED_DERIVED = 1 << 13; -export const INERT = 1 << 14; -export const DESTROYED = 1 << 15; -export const EFFECT_RAN = 1 << 16; +export const INERT = 1 << 13; +export const DESTROYED = 1 << 14; +export const EFFECT_RAN = 1 << 15; /** 'Transparent' effects do not create a transition boundary */ -export const EFFECT_TRANSPARENT = 1 << 17; +export const EFFECT_TRANSPARENT = 1 << 16; /** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */ -export const LEGACY_DERIVED_PROP = 1 << 18; -export const INSPECT_EFFECT = 1 << 19; -export const HEAD_EFFECT = 1 << 20; -export const EFFECT_HAS_DERIVED = 1 << 21; -export const EFFECT_IS_UPDATING = 1 << 22; +export const LEGACY_DERIVED_PROP = 1 << 17; +export const INSPECT_EFFECT = 1 << 18; +export const HEAD_EFFECT = 1 << 19; +export const EFFECT_HAS_DERIVED = 1 << 20; +export const EFFECT_IS_UPDATING = 1 << 21; export const STATE_SYMBOL = Symbol('$state'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 199f0f9c67c1..613f2cb98ebd 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -28,14 +28,14 @@ import { UNOWNED, MAYBE_DIRTY, BLOCK_EFFECT, - ROOT_EFFECT, - ASSIGNED_DERIVED + ROOT_EFFECT } from '../constants.js'; 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 { component_context, is_runes } from '../context.js'; import { proxy } from '../proxy.js'; +import { execute_derived } from './deriveds.js'; export let inspect_effects = new Set(); export const old_values = new Map(); @@ -172,11 +172,11 @@ export function internal_set(source, value) { } if ((source.f & DERIVED) !== 0) { - // if we are assigning a derived we set it to clean but we also - // keep the assigned derived flag so that we can read the dependencies - // in the get function - var flags = (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY; - set_signal_status(source, flags | ASSIGNED_DERIVED); + // if we are assigning to a dirty derived we set it to clean/maybe dirty but we also eagerly execute it to track the dependencies + if ((source.f & DIRTY) !== 0) { + execute_derived(/** @type {Derived} */ (source)); + } + set_signal_status(source, (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY); } mark_reactions(source, DIRTY); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 6daf5a5cb89e..4b913dc9b918 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -23,8 +23,7 @@ import { LEGACY_DERIVED_PROP, DISCONNECTED, BOUNDARY_EFFECT, - EFFECT_IS_UPDATING, - ASSIGNED_DERIVED + EFFECT_IS_UPDATING } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; @@ -927,11 +926,6 @@ export function get(signal) { if (check_dirtiness(derived)) { update_derived(derived); - } else if ((derived.f & ASSIGNED_DERIVED) !== 0) { - // if the derived is clean but has been assigned a new value, then we need to - // still invoke the derived function and update the dependencies or else - // it will not depend on the original source - execute_derived(derived); } } @@ -1049,7 +1043,7 @@ export function untrack(fn) { } } -const STATUS_MASK = ~(DIRTY | MAYBE_DIRTY | CLEAN | ASSIGNED_DERIVED); +const STATUS_MASK = ~(DIRTY | MAYBE_DIRTY | CLEAN); /** * @param {Signal} signal