diff --git a/packages/core/primitives/signals/src/linked_signal.ts b/packages/core/primitives/signals/src/linked_signal.ts index 14434c58581f..5f3328681c3f 100644 --- a/packages/core/primitives/signals/src/linked_signal.ts +++ b/packages/core/primitives/signals/src/linked_signal.ts @@ -17,6 +17,7 @@ import { REACTIVE_NODE, ReactiveNode, runPostProducerCreatedFn, + setActiveConsumer, SIGNAL, } from './graph'; import {signalSetFn, signalUpdateFn} from './signal'; @@ -151,17 +152,22 @@ export const LINKED_SIGNAL_NODE: object = /* @__PURE__ */ (() => { const prevConsumer = consumerBeforeComputation(node); let newValue: unknown; + let wasEqual = false; try { const newSourceValue = node.source(); - const prev = - oldValue === UNSET || oldValue === ERRORED - ? undefined - : { - source: node.sourceValue, - value: oldValue, - }; + const oldValueValid = oldValue !== UNSET && oldValue !== ERRORED; + const prev = oldValueValid + ? { + source: node.sourceValue, + value: oldValue, + } + : undefined; newValue = node.computation(newSourceValue, prev); node.sourceValue = newSourceValue; + // We want to mark this node as errored if calling `equal` throws; however, we don't want + // to track any reactive reads inside `equal`. + setActiveConsumer(null); + wasEqual = oldValueValid && newValue !== ERRORED && node.equal(oldValue, newValue); } catch (err) { newValue = ERRORED; node.error = err; @@ -169,7 +175,7 @@ export const LINKED_SIGNAL_NODE: object = /* @__PURE__ */ (() => { consumerAfterComputation(node, prevConsumer); } - if (oldValue !== UNSET && newValue !== ERRORED && node.equal(oldValue, newValue)) { + if (wasEqual) { // No change to `valueVersion` - old and new values are // semantically equivalent. node.value = oldValue; diff --git a/packages/core/test/signals/linked_signal_spec.ts b/packages/core/test/signals/linked_signal_spec.ts index 848d57e5e28b..4ccdfc24cc77 100644 --- a/packages/core/test/signals/linked_signal_spec.ts +++ b/packages/core/test/signals/linked_signal_spec.ts @@ -7,7 +7,7 @@ */ import {isSignal, linkedSignal, signal, computed} from '../../src/core'; -import {setPostProducerCreatedFn} from '../../primitives/signals'; +import {defaultEquals, setPostProducerCreatedFn} from '../../primitives/signals'; import {testingEffect} from './effect_util'; describe('linkedSignal', () => { @@ -319,4 +319,125 @@ describe('linkedSignal', () => { expect(producers).toBe(2); setPostProducerCreatedFn(prev); }); + + describe('with custom equal', () => { + it('should cache exceptions thrown by equal', () => { + const s = signal(0); + + let computedRunCount = 0; + let equalRunCount = 0; + const c = linkedSignal( + () => { + computedRunCount++; + return s(); + }, + { + equal: () => { + equalRunCount++; + throw new Error('equal'); + }, + }, + ); + + // equal() isn't run for the initial computation. + expect(c()).toBe(0); + expect(computedRunCount).toBe(1); + expect(equalRunCount).toBe(0); + + s.set(1); + + // Error is thrown by equal(). + expect(() => c()).toThrowError('equal'); + expect(computedRunCount).toBe(2); + expect(equalRunCount).toBe(1); + + // Error is cached; c throws again without needing to rerun computation or equal(). + expect(() => c()).toThrowError('equal'); + expect(computedRunCount).toBe(2); + expect(equalRunCount).toBe(1); + }); + + it('should not track signal reads inside equal', () => { + const value = signal(1); + const epsilon = signal(0.5); + + let innerRunCount = 0; + let equalRunCount = 0; + const inner = linkedSignal( + () => { + innerRunCount++; + return value(); + }, + { + equal: (a, b) => { + equalRunCount++; + return Math.abs(a - b) < epsilon(); + }, + }, + ); + + let outerRunCount = 0; + const outer = linkedSignal(() => { + outerRunCount++; + return inner(); + }); + + // Everything runs the first time. + expect(outer()).toBe(1); + expect(innerRunCount).toBe(1); + expect(outerRunCount).toBe(1); + + // Difference is less than epsilon(). + value.set(1.2); + + // `inner` reruns because `value` was changed, and `equal` is called for the first time. + expect(outer()).toBe(1); + expect(innerRunCount).toBe(2); + expect(equalRunCount).toBe(1); + // `outer does not rerun because `equal` determined that `inner` had not changed. + expect(outerRunCount).toBe(1); + + // Previous difference is now greater than epsilon(). + epsilon.set(0.1); + + // While changing `epsilon` would change the outcome of the `inner`, we don't rerun it + // because we intentionally don't track reactive reads in `equal`. + expect(outer()).toBe(1); + expect(innerRunCount).toBe(2); + expect(equalRunCount).toBe(1); + // Equally important is that the signal read in `equal` doesn't leak into the outer reactive + // context either. + expect(outerRunCount).toBe(1); + }); + + it('should recover from exception', () => { + let shouldThrow = true; + const source = signal(0); + const derived = linkedSignal({ + source, + computation: (value, previous) => { + return `${value}, hasPrevious: ${previous !== undefined}`; + }, + equal: (a, b) => { + if (shouldThrow) { + throw new Error('equal'); + } + return defaultEquals(a, b); + }, + }); + + // Initial read doesn't throw because it doesn't call `equal`. + expect(derived()).toBe('0, hasPrevious: false'); + + // Update `source` to begin throwing. + source.set(1); + expect(() => derived()).toThrowError('equal'); + + // Stop throwing and update `source` to cause `derived` to recompute. No previous value + // should be made available as the linked signal transitions from an error state. + shouldThrow = false; + source.set(2); + expect(derived()).toBe('2, hasPrevious: false'); + }); + }); });