Skip to content

Commit

Permalink
fix: ensure unowned deriveds correctly get re-linked to the graph (#1…
Browse files Browse the repository at this point in the history
…4855)

* fix: ensure unowned deriveds correctly get re-linked to the graph

* fix: ensure unowned deriveds correctly get re-linked to the graph

* fix: ensure unowned deriveds correctly get re-linked to the graph

* add test

* add test

* cleaner apporach

* cleaner apporach

* cleaner apporach

* cleaner apporach
  • Loading branch information
trueadm authored Jan 2, 2025
1 parent f7f87dc commit 8148734
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-dolphins-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: ensure unowned deriveds correctly get re-linked to the graph
38 changes: 19 additions & 19 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,34 +196,34 @@ export function check_dirtiness(reaction) {

if (dependencies !== null) {
var i;

if ((flags & DISCONNECTED) !== 0) {
for (i = 0; i < dependencies.length; i++) {
(dependencies[i].reactions ??= []).push(reaction);
var dependency;
var is_disconnected = (flags & DISCONNECTED) !== 0;
var is_unowned_connected = is_unowned && active_effect !== null && !skip_reaction;
var length = dependencies.length;

// If we are working with a disconnected or an unowned signal that is now connected (due to an active effect)
// then we need to re-connect the reaction to the dependency
if (is_disconnected || is_unowned_connected) {
for (i = 0; i < length; i++) {
dependency = dependencies[i];

if (!dependency?.reactions?.includes(reaction)) {
(dependency.reactions ??= []).push(reaction);
}
}

reaction.f ^= DISCONNECTED;
if (is_disconnected) {
reaction.f ^= DISCONNECTED;
}
}

for (i = 0; i < dependencies.length; i++) {
var dependency = dependencies[i];
for (i = 0; i < length; i++) {
dependency = dependencies[i];

if (check_dirtiness(/** @type {Derived} */ (dependency))) {
update_derived(/** @type {Derived} */ (dependency));
}

// If we are working with an unowned signal as part of an effect (due to !skip_reaction)
// and the version hasn't changed, we still need to check that this reaction
// is linked to the dependency source – otherwise future updates will not be caught.
if (
is_unowned &&
active_effect !== null &&
!skip_reaction &&
!dependency?.reactions?.includes(reaction)
) {
(dependency.reactions ??= []).push(reaction);
}

if (dependency.version > reaction.version) {
return true;
}
Expand Down
33 changes: 33 additions & 0 deletions packages/svelte/tests/signals/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,4 +781,37 @@ describe('signals', () => {
assert.equal($.get(count), 0n);
};
});

test('unowned deriveds correctly re-attach to their source', () => {
const log: any[] = [];

return () => {
const a = state(0);
const b = state(0);
const c = derived(() => {
$.get(a);
return $.get(b);
});

$.get(c);

set(a, 1);

const destroy = effect_root(() => {
render_effect(() => {
log.push($.get(c));
});
});

assert.deepEqual(log, [0]);

set(b, 1);

flushSync();

assert.deepEqual(log, [0, 1]);

destroy();
};
});
});

0 comments on commit 8148734

Please sign in to comment.