diff --git a/packages/reactivity-core/README.md b/packages/reactivity-core/README.md index de57b84..10d687d 100644 --- a/packages/reactivity-core/README.md +++ b/packages/reactivity-core/README.md @@ -379,27 +379,23 @@ npm install @conterra/reactivity-core ## Gotchas and tips -### Avoid side effects in computed signals +### Avoid cycles in computed signals -Computed signals should use a side effect free function. -Oftentimes, you cannot control how often the function is re-executed, because that depends on how often -the dependencies of your functions change and when your signal is actually read (computation is lazy). +Don't use the value of a computed signal during its own computation. +The error will be obvious in a simple example, but it may also occur by accident when many objects or functions are involved. Example: ```ts -let nonReactiveValue = 1; -const reactiveValue = reactive(1); +import { computed } from "@conterra/reactivity-core"; + const computedValue = computed(() => { - // This works, but it usually bad style for the reasons outlined above: - nonReactiveValue += 1; - - // XXX - // This is outright forbidden and will result in a runtime error. - // You cannot modify a signal from inside a computed signal. - reactiveValue.value += 1; - return "whatever"; + // Trivial example. This may happen through many layers of indirection in real world code. + let v = computedValue.value; + return v * 2; }); + +console.log(computedValue.value); // throws "Cycle detected" ``` ### Don't trigger an effect from within itself @@ -410,6 +406,8 @@ However, you should take care not to produce a cycle. Example: this is okay (but could be replaced by a computed signal). ```ts +import { reactive, effect } from "@conterra/reactivity-core"; + const v1 = reactive(0); const v2 = reactive(1); effect(() => { @@ -421,6 +419,8 @@ effect(() => { Example: this is _not_ okay. ```ts +import { reactive, effect } from "@conterra/reactivity-core"; + const v1 = reactive(0); effect(() => { // same as `v1.value = v1.value + 1` @@ -456,6 +456,48 @@ The example above will not throw an error anymore because the _read_ to `v1` has ### Batching multiple updates +Every update to a signal will usually trigger all watchers. +This is not really a problem when using the default `watch()` or `effect()`, since multiple changes that follow _immediately_ after each other are grouped into a single notification, with a minimal delay. + +However, when using `syncEffect` or `syncWatch`, you will be triggered many times: + +```ts +import { reactive, syncEffect } from "@conterra/reactivity-core"; + +const count = reactive(0); +syncEffect(() => { + console.log(count.value); +}); + +count.value += 1; +count.value += 1; +count.value += 1; +count.value += 1; +// Effect has executed 5 times +``` + +You can avoid this by grouping many updates into a single _batch_. +Effects or watchers will not get notified until the batch is complete: + +```ts +import { reactive, syncEffect, batch } from "@conterra/reactivity-core"; + +const count = reactive(0); +syncEffect(() => { + console.log(count.value); +}); + +batch(() => { + count.value += 1; + count.value += 1; + count.value += 1; + count.value += 1; +}); +// Effect has executed only twice: one initial call and once after batch() as completed. +``` + +It is usually a good idea to surround a complex update operation with `batch()`. + ### Sync vs async effect / watch ### Writing nonreactive code diff --git a/packages/reactivity-core/Reactive.ts b/packages/reactivity-core/Reactive.ts index a3cad07..403fc01 100644 --- a/packages/reactivity-core/Reactive.ts +++ b/packages/reactivity-core/Reactive.ts @@ -15,7 +15,7 @@ export type AddWritableBrand = AddBrand & { [IS_WRITABLE_REACTIVE]: true } * * When the value changes, all users of that value (computed signals, effects, watchers) * are notified automatically. - * + * * @group Primitives */ export interface ReadonlyReactive { @@ -40,16 +40,16 @@ export interface ReadonlyReactive { */ peek(): T; - /** + /** * Same as `.value`. - * + * * For compatibility with builtin JS constructs. **/ toJSON(): T; /** * Formats `.value` as a string. - * + * * For compatibility with builtin JS constructs. **/ toString(): string; @@ -60,7 +60,7 @@ export interface ReadonlyReactive { * * The value stored in this object can be changed through assignment, * and all its users will be notified automatically. - * + * * @group Primitives */ export interface Reactive extends ReadonlyReactive { @@ -82,7 +82,7 @@ export interface Reactive extends ReadonlyReactive { * A signal that holds a value from an external source. * * Instances of this type are used to integrate "foreign" state into the reactivity system. - * + * * @group Primitives */ export interface ExternalReactive extends ReadonlyReactive { diff --git a/packages/reactivity-core/ReactiveImpl.ts b/packages/reactivity-core/ReactiveImpl.ts index 0bdd487..f659f43 100644 --- a/packages/reactivity-core/ReactiveImpl.ts +++ b/packages/reactivity-core/ReactiveImpl.ts @@ -16,14 +16,14 @@ import { /** * A function that should return `true` if `a` and `b` are considered equal, `false` otherwise. - * + * * @group Primitives */ export type EqualsFunc = (a: T, b: T) => boolean; /** * Options that can be passed when creating a new signal. - * + * * @group Primitives */ export interface ReactiveOptions { @@ -47,7 +47,7 @@ export interface ReactiveOptions { * console.log(foo.value); // undefined * foo.value = 123; // updates the current value * ``` - * + * * @group Primitives */ export function reactive(): Reactive; @@ -62,7 +62,7 @@ export function reactive(): Reactive; * console.log(foo.value); // 123 * foo.value = 456; // updates the current value * ``` - * + * * @group Primitives */ export function reactive(initialValue: T, options?: ReactiveOptions): Reactive; @@ -94,7 +94,7 @@ export function reactive( * foo.value = 2; * console.log(doubleFoo.value); // 4 * ``` - * + * * @group Primitives */ export function computed(compute: () => T, options?: ReactiveOptions): ReadonlyReactive { @@ -131,7 +131,7 @@ export function computed(compute: () => T, options?: ReactiveOptions): Rea * * // later: unsubscribe from signal * ``` - * + * * @group Primitives */ export function external(compute: () => T, options?: ReactiveOptions): ExternalReactive { @@ -158,7 +158,8 @@ export function external(compute: () => T, options?: ReactiveOptions): Ext invalidateSignal.value; return rawUntracked(() => compute()); }, options); - (externalReactive as RemoveBrand as ReactiveImpl).trigger = invalidate; + (externalReactive as RemoveBrand as ReactiveImpl).trigger = + invalidate; return externalReactive as ExternalReactive; } @@ -190,7 +191,7 @@ export function external(compute: () => T, options?: ReactiveOptions): Ext * }); * // now the effect runs once * ``` - * + * * @group Primitives */ export function batch(callback: () => T): T { @@ -204,7 +205,7 @@ export function batch(callback: () => T): T { * even if they occur inside a computed object or in an effect. * * `untracked` returns the value of `callback()`. - * + * * @group Primitives */ export function untracked(callback: () => T): T { @@ -216,7 +217,7 @@ export function untracked(callback: () => T): T { * if it is not reactive. * * The access to `.value` is tracked. - * + * * @group Primitives */ export function getValue(maybeReactive: ReadonlyReactive | T) { @@ -231,7 +232,7 @@ export function getValue(maybeReactive: ReadonlyReactive | T) { * if it is not reactive. * * The access to `.value` is _not_ tracked. - * + * * @group Primitives */ export function peekValue(maybeReactive: ReadonlyReactive | T) { diff --git a/packages/reactivity-core/TaskQueue.ts b/packages/reactivity-core/TaskQueue.ts index 1377309..19fa7c8 100644 --- a/packages/reactivity-core/TaskQueue.ts +++ b/packages/reactivity-core/TaskQueue.ts @@ -1,3 +1,4 @@ +import { reportTaskError } from "./reportTaskError"; import { CleanupHandle } from "./sync"; type TaskFn = () => void; @@ -55,13 +56,12 @@ export class TaskQueue { // register and unregister for every iteration otherwise node will not terminate // https://stackoverflow.com/a/61574326 this.channel.port2.addEventListener("message", this.messageHandler); - this.channel.port1.postMessage(""); // queue macro task } private runIteration() { this.channel.port2.removeEventListener("message", this.messageHandler); - + // Swap arrays so that NEW tasks are not queued into the same array; // they will be handled in the next iteration. const tasks = this.queue; @@ -74,9 +74,7 @@ export class TaskQueue { try { task.fn(); } catch (e) { - // This makes the error an unhandled rejection for lack of a better - // reporting mechanisms. Stupid idea? - Promise.reject(new Error(`Error in effect or watch callback`, { cause: e })); + reportTaskError(e); } } } diff --git a/packages/reactivity-core/async.test.ts b/packages/reactivity-core/async.test.ts index 0e6ba5a..421116e 100644 --- a/packages/reactivity-core/async.test.ts +++ b/packages/reactivity-core/async.test.ts @@ -1,6 +1,11 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { reactive } from "./ReactiveImpl"; import { effect, watch } from "./async"; +import * as report from "./reportTaskError"; + +afterEach(() => { + vi.restoreAllMocks(); +}); describe("effect", () => { it("re-executes the callback asynchronously", async () => { @@ -70,6 +75,35 @@ describe("effect", () => { await waitForMacroTask(); expect(spy).toHaveBeenCalledTimes(1); // not called again }); + + it("throws when a cycle is detected", async () => { + const v1 = reactive(0); + expect(() => { + effect(() => { + v1.value = v1.value + 1; + }); + }).toThrowError(/Cycle detected/); + }); + + it("throws when a cycle is detected in a later execution", async () => { + const errorSpy = vi.spyOn(report, "reportTaskError").mockImplementation(() => {}); + + const v1 = reactive(0); + const trigger = reactive(false); + effect(() => { + if (!trigger.value) { + return; + } + + v1.value = v1.value + 1; + }); + expect(errorSpy).toHaveBeenCalledTimes(0); + + trigger.value = true; + await waitForMacroTask(); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy.mock.lastCall![0]).toMatchInlineSnapshot(`[Error: Cycle detected]`); + }); }); describe("watch", () => { diff --git a/packages/reactivity-core/async.ts b/packages/reactivity-core/async.ts index 233e0a0..601f4eb 100644 --- a/packages/reactivity-core/async.ts +++ b/packages/reactivity-core/async.ts @@ -37,12 +37,13 @@ import { EffectFunc, syncEffect, syncEffectOnce, syncWatch, WatchOptions } from * > This is done to avoid redundant executions as a result of many fine-grained changes. * > * > If you need more control, take a look at {@link syncEffect}. - * + * * @group Watching */ export function effect(callback: EffectFunc): CleanupHandle { let currentSyncEffect: CleanupHandle | undefined; let currentDispatch: CleanupHandle | undefined; + let syncExecution: boolean; // Runs the actual effect body (once). // When the reactive dependencies change, an async callback is dispatched, @@ -60,20 +61,29 @@ export function effect(callback: EffectFunc): CleanupHandle { currentSyncEffect?.destroy(); currentSyncEffect = undefined; - currentSyncEffect = syncEffectOnce(callback, () => { - currentSyncEffect = undefined; - if (currentDispatch) { - return; - } + syncExecution = true; + try { + currentSyncEffect = syncEffectOnce(callback, () => { + currentSyncEffect = undefined; + if (syncExecution) { + throw new Error("Cycle detected"); + } - currentDispatch = dispatchCallback(() => { - try { - rerunEffect(); - } finally { - currentDispatch = undefined; + if (currentDispatch) { + return; } + + currentDispatch = dispatchCallback(() => { + try { + rerunEffect(); + } finally { + currentDispatch = undefined; + } + }); }); - }); + } finally { + syncExecution = false; + } } function destroy() { @@ -135,7 +145,7 @@ export function effect(callback: EffectFunc): CleanupHandle { * > This is done to avoid redundant executions as a result of many fine-grained changes. * > * > If you need more control, take a look at {@link syncWatch}. - * + * * @group Watching */ export function watch( @@ -150,7 +160,7 @@ export function watch( selector, (values) => { currentValues = values; - + // If the user passed 'immediate: true', the initial execution is not deferred sync. if (initialSyncExecution) { callback(currentValues); diff --git a/packages/reactivity-core/collections/array.ts b/packages/reactivity-core/collections/array.ts index f698bb7..f4895a6 100644 --- a/packages/reactivity-core/collections/array.ts +++ b/packages/reactivity-core/collections/array.ts @@ -5,7 +5,7 @@ import { reactive } from "../ReactiveImpl"; * Reactive array interface without modifying methods. * * See also {@link ReactiveArray}. - * + * * @group Collections */ export interface ReadonlyReactiveArray extends Iterable { @@ -269,7 +269,7 @@ export interface ReadonlyReactiveArray extends Iterable { * Not all builtin array methods are implemented right now, but most of them are. * * Reads and writes to this array are reactive. - * + * * @group Collections */ export interface ReactiveArray extends ReadonlyReactiveArray { @@ -335,7 +335,7 @@ export interface ReactiveArray extends ReadonlyReactiveArray { * // With initial content * const array2 = reactiveArray([1, 2, 3]); * ``` - * + * * @group Collections */ export function reactiveArray(items?: Iterable): ReactiveArray { diff --git a/packages/reactivity-core/collections/map.ts b/packages/reactivity-core/collections/map.ts index e0adbc3..0eac09e 100644 --- a/packages/reactivity-core/collections/map.ts +++ b/packages/reactivity-core/collections/map.ts @@ -7,7 +7,7 @@ import { reactive } from "../ReactiveImpl"; * This map interface is designed ot be very similar to (but not exactly the same as) the standard JavaScript `Map`. * * Reads from and writes to this map are reactive. - * + * * @group Collections */ export interface ReactiveMap extends Iterable<[key: K, value: V]> { @@ -67,7 +67,7 @@ export interface ReactiveMap extends Iterable<[key: K, value: V]> { * Reactive map interface without modifying methods. * * See also {@link ReactiveMap}. - * + * * @group Collections */ export type ReadonlyReactiveMap = Omit, "set" | "delete" | "clear">; @@ -84,7 +84,7 @@ export type ReadonlyReactiveMap = Omit, "set" | "delete" * // With initial content * const map2 = reactiveMap([["foo", 1], ["bar", 2]]); * ``` - * + * * @group Collections */ export function reactiveMap(initial?: Iterable<[K, V]> | undefined): ReactiveMap { diff --git a/packages/reactivity-core/collections/set.ts b/packages/reactivity-core/collections/set.ts index 00b9ac2..adbe0bc 100644 --- a/packages/reactivity-core/collections/set.ts +++ b/packages/reactivity-core/collections/set.ts @@ -6,7 +6,7 @@ import { ReactiveMap, reactiveMap } from "./map"; * This set interface is designed ot be very similar to (but not exactly the same as) the standard JavaScript `Set`. * * Reads from and writes to this set are reactive. - * + * * @group Collections */ export interface ReactiveSet extends Iterable { @@ -58,7 +58,7 @@ export interface ReactiveSet extends Iterable { * Reactive set interface without modifying methods. * * See also {@link ReactiveSet}. - * + * * @group Collections */ export type ReadonlyReactiveSet = Omit, "add" | "delete" | "clear">; @@ -75,7 +75,7 @@ export type ReadonlyReactiveSet = Omit, "add" | "delete" | "cl * // With initial content * const set2 = reactiveSet(["foo", "bar"]); * ``` - * + * * @group Collections */ export function reactiveSet(initial?: Iterable | undefined): ReactiveSet { diff --git a/packages/reactivity-core/index.ts b/packages/reactivity-core/index.ts index 6c64172..e7b3db6 100644 --- a/packages/reactivity-core/index.ts +++ b/packages/reactivity-core/index.ts @@ -1,26 +1,22 @@ /** * A framework agnostic library for building reactive applications. - * + * * @module - * + * * @groupDescription Primitives - * + * * Primitive building blocks for reactive code. - * + * * @groupDescription Watching - * + * * Utilities to run code when reactive values change. - * + * * @groupDescription Collections - * + * * Reactive collections to simplify working with complex data structures. */ -export { - type ReadonlyReactive, - type Reactive, - type ExternalReactive, -} from "./Reactive"; +export { type ReadonlyReactive, type Reactive, type ExternalReactive } from "./Reactive"; export { type EqualsFunc, type ReactiveOptions, diff --git a/packages/reactivity-core/reportTaskError.ts b/packages/reactivity-core/reportTaskError.ts new file mode 100644 index 0000000..79339ab --- /dev/null +++ b/packages/reactivity-core/reportTaskError.ts @@ -0,0 +1,6 @@ +// This function can be mocked in tests +export function reportTaskError(e: unknown) { + // This makes the error an unhandled rejection for lack of a better + // reporting mechanisms. Stupid idea? + Promise.reject(new Error(`Error in effect or watch callback`, { cause: e })); +} diff --git a/packages/reactivity-core/sync.test.ts b/packages/reactivity-core/sync.test.ts index f21b6cf..aaf99eb 100644 --- a/packages/reactivity-core/sync.test.ts +++ b/packages/reactivity-core/sync.test.ts @@ -160,7 +160,7 @@ describe("syncEffectOnce", () => { expect(invalidateSpy).toHaveBeenCalledTimes(1); }); - it("can be cleand up", () => { + it("can be cleaned up", () => { const r = reactive(1); const spy = vi.fn(); diff --git a/packages/reactivity-core/sync.ts b/packages/reactivity-core/sync.ts index 5f3fe7f..2c454fb 100644 --- a/packages/reactivity-core/sync.ts +++ b/packages/reactivity-core/sync.ts @@ -8,7 +8,7 @@ import { effect, watch } from "./async"; /** * A handle returned by various functions to dispose of a resource, * such as a watcher or an effect. - * + * * @group Watching */ export interface CleanupHandle { @@ -23,7 +23,7 @@ export interface CleanupHandle { * * This function will be invoked before the effect is triggered again, * or when the effect is disposed. - * + * * @group Watching */ export type EffectCleanupFn = () => void; @@ -33,7 +33,7 @@ export type EffectCleanupFn = () => void; * * Instructions in this function are tracked: when any of its reactive * dependencies change, the effect will be triggered again. - * + * * @group Watching */ export type EffectFunc = (() => void) | (() => EffectCleanupFn); @@ -70,7 +70,7 @@ export type EffectFunc = (() => void) | (() => EffectCleanupFn); * // later: * handle.destroy(); * ``` - * + * * @group Watching */ export function syncEffect(callback: EffectFunc): CleanupHandle { @@ -87,12 +87,14 @@ export function syncEffect(callback: EffectFunc): CleanupHandle { * Typically, `onInvalidate` will be very cheap (e.g. schedule a new render). * * Note that `onInvalidate` will never be invoked more than once. - * + * * @group Watching */ export function syncEffectOnce(callback: EffectFunc, onInvalidate: () => void): CleanupHandle { let execution = 0; - const handle = syncEffect(() => { + let syncExecution = true; + let handle: CleanupHandle | undefined = undefined; + handle = syncEffect(() => { const thisExecution = execution++; if (thisExecution === 0) { callback(); @@ -101,17 +103,22 @@ export function syncEffectOnce(callback: EffectFunc, onInvalidate: () => void): try { onInvalidate(); } finally { - handle.destroy(); + if (syncExecution) { + Promise.resolve().then(() => handle?.destroy()); + } else { + handle?.destroy(); + } } }); } }); + syncExecution = false; return handle; } /** * Options that can be passed to {@link syncWatch}. - * + * * @group Watching */ export interface WatchOptions { @@ -159,7 +166,7 @@ export interface WatchOptions { * ``` * * > NOTE: You must *not* modify the array that gets passed into `callback`. - * + * * @group Watching */ export function syncWatch(