Skip to content

Commit

Permalink
Fix computed updated when no deps changed
Browse files Browse the repository at this point in the history
  • Loading branch information
marvinhagemeister committed Sep 14, 2022
1 parent 1422baf commit 160ea77
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/honest-tomatoes-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals-core": patch
---

Fix computed signal being re-calculated despite dependencies not having changed
33 changes: 18 additions & 15 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,22 @@ function sweep(subs: Set<Signal<any>>) {
subs.forEach(signal => {
// If a computed errored during sweep, we'll discard that subtree
// for this sweep cycle by setting PENDING to 0;
if (signal._pending > 0) {
signal._requiresUpdate = true;

if (--signal._pending === 0) {
if (signal._isComputing) {
throw Error("Cycle detected");
}

signal._requiresUpdate = false;
signal._isComputing = true;
signal._updater();
signal._isComputing = false;
sweep(signal._subs);
if (signal._pending > 1) return --signal._pending;
let ready = true;
signal._deps.forEach(dep => {
if (dep._pending > 0) ready = false;
});

if (ready && signal._pending > 0 && --signal._pending === 0) {
if (signal._isComputing) {
throw Error("Cycle detected");
}

signal._requiresUpdate = false;
signal._isComputing = true;
signal._updater();
signal._isComputing = false;
sweep(signal._subs);
}
});
}
Expand Down Expand Up @@ -238,8 +240,9 @@ export function computed<T>(compute: () => T): ReadonlySignal<T> {

try {
let ret = compute();

finish(signal._value === ret, true);
const stale = signal._value === ret;
if (!stale) signal._subs.forEach(sub => (sub._requiresUpdate = true));
finish(stale, true);
signal._value = ret;
} catch (err: any) {
// Ensure that we log the first error not the last
Expand Down
43 changes: 37 additions & 6 deletions packages/core/test/signal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -230,18 +230,23 @@ describe("computed()", () => {
});

it("should drop A->B->A updates", async () => {
// A
// / |
// B | <- Looks like a flag doesn't it? :D
// \ |
// C
// |
// D
const a = signal(2);

const b = computed(() => a.value - 1);
const c = computed(() => a.value + 1);

const d = computed(() => a.value + b.value);
const c = computed(() => a.value + b.value);

const compute = sinon.spy(() => "d: " + d.value);
const e = computed(compute);
const compute = sinon.spy(() => "d: " + c.value);
const d = computed(compute);

// Trigger read
e.value;
expect(d.value).to.equal("d: 3");
expect(compute).to.have.been.calledOnce;
compute.resetHistory();

Expand Down Expand Up @@ -607,6 +612,32 @@ describe("computed()", () => {
a.value = 0;
expect(c.value).to.equal(0);
});

it("should not update a sub if all deps unmark it", () => {
// In this scenario "B" and "C" always return the same value. When "A"
// changes, "D" should not update.
// A
// / \
// *B *C
// \ /
// D
const a = signal("a");
const b = computed(() => {
a.value;
return "b";
});
const c = computed(() => {
a.value;
return "c";
});
const spy = sinon.spy(() => b.value + " " + c.value);
const d = computed(spy);
expect(d.value).to.equal("b c");
spy.resetHistory();

a.value = "aa";
expect(spy).not.to.be.called;
});
});
});

Expand Down

0 comments on commit 160ea77

Please sign in to comment.