From 0ff1437984841d10d5d047d0928f52d4fe7e1880 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Wed, 8 Jan 2025 21:16:15 -0500 Subject: [PATCH 1/8] feat!: make `signal.get` reactive and add `signal.getOnce` @affects atoms, react, stores --- packages/atoms/src/classes/Ecosystem.ts | 19 +- packages/atoms/src/classes/GraphNode.ts | 245 +++--------------- packages/atoms/src/classes/MappedSignal.ts | 2 +- .../atoms/src/classes/SelectorInstance.ts | 23 +- packages/atoms/src/classes/Signal.ts | 15 +- .../src/classes/instances/AtomInstance.ts | 73 ++++-- packages/atoms/src/index.ts | 10 +- .../atoms/src/injectors/injectAtomInstance.ts | 4 +- .../atoms/src/injectors/injectAtomState.ts | 2 +- .../atoms/src/injectors/injectAtomValue.ts | 2 +- packages/atoms/src/injectors/injectSelf.ts | 2 +- packages/atoms/src/injectors/injectSignal.ts | 3 +- packages/atoms/src/types/index.ts | 9 +- packages/atoms/src/utils/evaluationContext.ts | 12 +- packages/atoms/src/utils/general.ts | 2 +- packages/atoms/src/utils/graph.ts | 206 +++++++++++++++ packages/react/src/hooks/useAtomInstance.ts | 4 +- packages/react/src/hooks/useAtomState.ts | 2 +- packages/react/src/hooks/useAtomValue.ts | 2 +- .../test/integrations/ecosystem.test.tsx | 4 +- .../test/integrations/injectors.test.tsx | 5 +- .../react/test/integrations/plugins.test.tsx | 2 +- .../react/test/integrations/signals.test.tsx | 22 ++ packages/react/test/types.test.tsx | 2 +- packages/stores/src/AtomInstance.ts | 8 +- 25 files changed, 371 insertions(+), 309 deletions(-) create mode 100644 packages/atoms/src/utils/graph.ts diff --git a/packages/atoms/src/classes/Ecosystem.ts b/packages/atoms/src/classes/Ecosystem.ts index fc175d00..aeeb971d 100644 --- a/packages/atoms/src/classes/Ecosystem.ts +++ b/packages/atoms/src/classes/Ecosystem.ts @@ -137,19 +137,7 @@ export class Ecosystem | undefined = any> const get: AtomGetters['get'] = ( atom: AtomTemplateBase, params?: G['Params'] - ) => { - const instance = this.getNode(atom, params as G['Params']) - const node = getEvaluationContext().n - - // If get is called in a reactive context, track the required atom - // instances so we can add graph edges for them. When called outside a - // reactive context, get() is just an alias for ecosystem.get() - if (node) { - bufferEdge(instance, 'get', 0) - } - - return instance.get() - } + ) => this.getNode(atom, params as G['Params']).get() const getInstance: AtomGetters['getInstance'] = ( atom: AtomTemplateBase, @@ -444,7 +432,7 @@ export class Ecosystem | undefined = any> params?: ParamsOf ) { if ((atom as GraphNode).izn) { - return (atom as GraphNode).get() + return (atom as GraphNode).v } const instance = this.getInstance( @@ -452,7 +440,7 @@ export class Ecosystem | undefined = any> params as ParamsOf ) as AnyAtomInstance - return instance.get() + return instance.v } public getInstance( @@ -633,7 +621,6 @@ export class Ecosystem | undefined = any> instance = new SelectorInstance(this, id, selectorOrConfig, params || []) this.n.set(id, instance) - runSelector(instance, true) return instance } diff --git a/packages/atoms/src/classes/GraphNode.ts b/packages/atoms/src/classes/GraphNode.ts index 26b42e60..88537042 100644 --- a/packages/atoms/src/classes/GraphNode.ts +++ b/packages/atoms/src/classes/GraphNode.ts @@ -12,217 +12,16 @@ import { } from '@zedux/atoms/types/index' import { is, Job } from '@zedux/core' import { Ecosystem } from './Ecosystem' -import { pluginActions } from '../utils/plugin-actions' -import { Destroy, EventSent, ExplicitExternal, Static } from '../utils/general' +import { EventSent, ExplicitExternal } from '../utils/general' import { AtomTemplateBase } from './templates/AtomTemplateBase' import { - ExplicitEvents, CatchAllListener, EventEmitter, SingleEventListener, ListenableEvents, } from '../types/events' - -/** - * Actually add an edge to the graph. When we buffer graph updates, we're - * really just deferring the calling of this method. - */ -export const addEdge = ( - dependent: GraphNode, - dependency: GraphNode, - newEdge: GraphEdge -) => { - const { _mods, modBus } = dependency.e - - // draw the edge in both nodes. Dependent may not exist if it's an external - // pseudo-node - dependent && dependent.s.set(dependency, newEdge) - dependency.o.set(dependent, newEdge) - dependency.c?.() - - // static dependencies don't change a node's weight - if (!(newEdge.flags & Static)) { - recalculateNodeWeight(dependency.W, dependent) - } - - if (_mods.edgeCreated) { - modBus.dispatch( - pluginActions.edgeCreated({ - dependency, - dependent: dependent, // unfortunate but not changing for now - edge: newEdge, - }) - ) - } - - return newEdge -} - -export const destroyNodeStart = (node: GraphNode, force?: boolean) => { - // If we're not force-destroying, don't destroy if there are dependents - if (node.l === 'Destroyed' || (!force && node.o.size)) return - - node.c?.() - node.c = undefined - - setNodeStatus(node, 'Destroyed') - - if (node.w.length) node.e._scheduler.unschedule(node) - - return true -} - -// TODO: merge this into destroyNodeStart. We should be able to -export const destroyNodeFinish = (node: GraphNode) => { - // first remove all edges between this node and its dependencies - for (const dependency of node.s.keys()) { - removeEdge(node, dependency) - } - - // if an atom instance is force-destroyed, it could still have dependents. - // Inform them of the destruction - scheduleDependents( - { - r: node.w, - s: node, - t: Destroy, - }, - true, - true - ) - - // now remove all edges between this node and its dependents - for (const [observer, edge] of node.o) { - if (!(edge.flags & Static)) { - recalculateNodeWeight(-node.W, observer) - } - - observer.s.delete(node) - - // we _probably_ don't need to send edgeRemoved mod events to plugins for - // these - it's better that they receive the duplicate edgeCreated event - // when the dependency is recreated by its dependent(s) so they can infer - // that the edge was "moved" - } - - node.e.n.delete(node.id) -} - -export const handleStateChange = < - G extends Pick ->( - node: GraphNode, - oldState: G['State'], - events?: Partial -) => { - scheduleDependents({ e: events, p: oldState, r: node.w, s: node }, false) - - if (node.e._mods.stateChanged) { - node.e.modBus.dispatch( - pluginActions.stateChanged({ - node, - newState: node.get(), - oldState, - reasons: node.w, - }) - ) - } - - // run the scheduler synchronously after any node state update - events?.batch || node.e._scheduler.flush() -} - -export const normalizeNodeFilter = (options?: NodeFilter) => - typeof options === 'object' && !is(options, AtomTemplateBase) - ? (options as NodeFilterOptions) - : { include: options ? [options as string | AnyAtomTemplate] : [] } - -const recalculateNodeWeight = (weightDiff: number, node?: GraphNode) => { - if (!node) return // happens when node is external - - node.W += weightDiff - - for (const observer of node.o.keys()) { - recalculateNodeWeight(weightDiff, observer) - } -} - -/** - * Remove the graph edge between two nodes. The dependent may not exist as a - * node in the graph if it's external, e.g. a React component - * - * For some reason in React 18+, React destroys parents before children. This - * means a parent EcosystemProvider may have already unmounted and wiped the - * whole graph; this edge may already be destroyed. - */ -export const removeEdge = (dependent: GraphNode, dependency: GraphNode) => { - // erase graph edge between dependent and dependency - dependent && dependent.s.delete(dependency) - - // hmm could maybe happen when a dependency was force-destroyed if a child - // tries to destroy its edge before recreating it (I don't think we ever do - // that though) - if (!dependency) return - - const edge = dependency.o.get(dependent) - - // happens in React 18+ (see this method's jsdoc above) - if (!edge) return - - dependency.o.delete(dependent) - - // static dependencies don't change a node's weight - if (!(edge.flags & Static)) { - recalculateNodeWeight(-dependency.W, dependent) - } - - if (dependency.e._mods.edgeRemoved) { - dependency.e.modBus.dispatch( - pluginActions.edgeRemoved({ - dependency, - dependent: dependent, - edge: edge, - }) - ) - } - - scheduleNodeDestruction(dependency) -} - -export const scheduleDependents = ( - reason: Omit & { - s: NonNullable - }, - defer?: boolean, - scheduleStaticDeps?: boolean -) => { - for (const [observer, edge] of reason.s.o) { - // Static deps don't update on state change, only on promise change or node - // force-destruction - if (scheduleStaticDeps || !(edge.flags & Static)) observer.r(reason, defer) - } -} - -/** - * When a node's refCount hits 0, schedule destruction of that node. - */ -export const scheduleNodeDestruction = (node: GraphNode) => - node.o.size || node.l !== 'Active' || node.m() - -export const setNodeStatus = (node: GraphNode, newStatus: LifecycleStatus) => { - const oldStatus = node.l - node.l = newStatus - - if (node.e._mods.statusChanged) { - node.e.modBus.dispatch( - pluginActions.statusChanged({ - newStatus, - node, - oldStatus, - }) - ) - } -} +import { bufferEdge, getEvaluationContext } from '../utils/evaluationContext' +import { addEdge, removeEdge, setNodeStatus } from '../utils/graph' export abstract class GraphNode< G extends Pick = { @@ -257,6 +56,13 @@ export abstract class GraphNode< */ public W = 1 + /** + * `v`alue - the current state of this signal. + */ + // @ts-expect-error only some node types have state. They will need to make + // sure they set this. This should be undefined for nodes that don't. + public v: G['State'] + /** * Detach this node from the ecosystem and clean up all graph edges and other * subscriptions/effects created by this node. @@ -271,8 +77,31 @@ export abstract class GraphNode< /** * Get the current value of this node. + * + * This is reactive! When called inside a reactive context (e.g. an atom state + * factory or atom selector function), calling this method creates a graph + * edge between the evaluating node and the node whose value this returns. + * + * Outside reactive contexts, this behaves exactly the same as `.getOnce()` + * + * To retrieve the node's value non-reactively, use `.getOnce()` instead. */ - public abstract get(): G['State'] + public get() { + // If get is called in a reactive context, track the required atom + // instances so we can add graph edges for them. When called outside a + // reactive context, get() is just an alias for ecosystem.get() + getEvaluationContext().n && bufferEdge(this, 'get', 0) + + return this.v + } + + /** + * Get the current value of this node without registering any graph + * dependencies in reactive contexts. + */ + public getOnce() { + return this.v + } /** * The unique id of this node in the graph. Zedux always tries to make this @@ -322,7 +151,7 @@ export abstract class GraphNode< ? reason.e ?? {} : { ...reason.e, - change: { newState: this.get(), oldState: reason.p }, + change: { newState: this.v, oldState: reason.p }, } ) as ListenableEvents @@ -408,7 +237,9 @@ export abstract class GraphNode< excludeFlags = [], include = [], includeFlags = [], - } = normalizeNodeFilter(options) + } = typeof options === 'object' && !is(options, AtomTemplateBase) + ? (options as NodeFilterOptions) + : { include: options ? [options as string | AnyAtomTemplate] : [] } const isExcluded = exclude.some(templateOrKey => diff --git a/packages/atoms/src/classes/MappedSignal.ts b/packages/atoms/src/classes/MappedSignal.ts index a7e6d1cc..681ceb41 100644 --- a/packages/atoms/src/classes/MappedSignal.ts +++ b/packages/atoms/src/classes/MappedSignal.ts @@ -208,7 +208,7 @@ export class MappedSignal< if (reason.s) this.N = { ...this.v } } - if (reason.s) this.N![this.I[reason.s.id]] = reason.s.get() + if (reason.s) this.N![this.I[reason.s.id]] = reason.s.v // forward events from wrapped signals to observers of this wrapper signal. // Use `super.send` for this 'cause `this.send` intercepts events and passes diff --git a/packages/atoms/src/classes/SelectorInstance.ts b/packages/atoms/src/classes/SelectorInstance.ts index fe9bd0f8..e829b263 100644 --- a/packages/atoms/src/classes/SelectorInstance.ts +++ b/packages/atoms/src/classes/SelectorInstance.ts @@ -13,15 +13,15 @@ import { startBuffer, } from '../utils/evaluationContext' import { prefix } from '../utils/general' -import { pluginActions } from '../utils/plugin-actions' -import { Ecosystem } from './Ecosystem' import { destroyNodeFinish, destroyNodeStart, - GraphNode, scheduleDependents, setNodeStatus, -} from './GraphNode' +} from '../utils/graph' +import { pluginActions } from '../utils/plugin-actions' +import { Ecosystem } from './Ecosystem' +import { GraphNode } from './GraphNode' const defaultResultsComparator = (a: any, b: any) => a === b @@ -129,11 +129,6 @@ export class SelectorInstance< > extends GraphNode { public static $$typeof = Symbol.for(`${prefix}/SelectorInstance`) - /** - * `v`alue - the current cached selector result - */ - public v?: G['State'] - constructor( /** * @see GraphNode.e @@ -156,6 +151,7 @@ export class SelectorInstance< public p: G['Params'] ) { super() + runSelector(this, true) } /** @@ -169,18 +165,11 @@ export class SelectorInstance< // let the WeakMap clean itself up. } - /** - * @see GraphNode.get - */ - public get() { - return this.v as G['State'] - } - /** * @see GraphNode.d */ public d(options?: DehydrationFilter) { - if (this.f(options)) return this.get() + if (this.f(options)) return this.v } /** diff --git a/packages/atoms/src/classes/Signal.ts b/packages/atoms/src/classes/Signal.ts index 15870759..c2f18a39 100644 --- a/packages/atoms/src/classes/Signal.ts +++ b/packages/atoms/src/classes/Signal.ts @@ -9,14 +9,14 @@ import { UndefinedEvents, } from '../types/index' import { EventSent } from '../utils/general' -import { Ecosystem } from './Ecosystem' import { destroyNodeFinish, destroyNodeStart, - GraphNode, handleStateChange, scheduleDependents, -} from './GraphNode' +} from '../utils/graph' +import { Ecosystem } from './Ecosystem' +import { GraphNode } from './GraphNode' import { recursivelyMutate, recursivelyProxy } from './proxies' export class Signal< @@ -59,7 +59,7 @@ export class Signal< public readonly id: string, /** - * `v`alue - the current state of this signal. + * @see GraphNode.v */ public v: G['State'], @@ -80,13 +80,6 @@ export class Signal< destroyNodeStart(this, force) && destroyNodeFinish(this) } - /** - * @see GraphNode.get - */ - public get() { - return this.v - } - /** * Sets up a proxy that listens to all mutations on this signal's state in the * passed callback. diff --git a/packages/atoms/src/classes/instances/AtomInstance.ts b/packages/atoms/src/classes/instances/AtomInstance.ts index e12c2c6f..8dc98326 100644 --- a/packages/atoms/src/classes/instances/AtomInstance.ts +++ b/packages/atoms/src/classes/instances/AtomInstance.ts @@ -19,6 +19,7 @@ import { Invalidate, prefix, PromiseChange, + Static, } from '@zedux/atoms/utils/general' import { pluginActions } from '@zedux/atoms/utils/plugin-actions' import { @@ -35,8 +36,9 @@ import { destroyNodeStart, handleStateChange, scheduleDependents, -} from '../GraphNode' +} from '../../utils/graph' import { + bufferEdge, destroyBuffer, flushBuffer, getEvaluationContext, @@ -104,7 +106,7 @@ const setPromise = >( promise: Promise, isStateUpdater?: boolean ) => { - const currentState = instance.get() + const currentState = instance.v if (promise === instance.promise) return currentState instance.promise = promise as G['Promise'] @@ -162,6 +164,12 @@ export class AtomInstance< // @ts-expect-error same as exports public promise: G['Promise'] + /** + * `a`lteredEdge - tracks whether we've altered the edge between this atom and + * its wrapped signal (if any). + */ + public a: boolean | undefined = undefined + /** * `b`ufferedEvents - when the wrapped signal emits events, we */ @@ -237,9 +245,9 @@ export class AtomInstance< * value. */ public get() { - const { S, v } = this - - return S ? S.get() : v + // defer to `Signal#get` to auto-track usages of this signal in the current + // reactive context (if any) + return this.S ? this.S.get() : super.get() } /** @@ -301,15 +309,14 @@ export class AtomInstance< public d(options?: DehydrationFilter) { if (!this.f(options)) return - const { t } = this - const state = this.get() + const { t, v } = this const transform = (typeof options === 'object' && !is(options, AtomTemplateBase) && (options as DehydrationOptions).transform) ?? true - return transform && t.dehydrate ? t.dehydrate(state) : state + return transform && t.dehydrate ? t.dehydrate(v) : v } /** @@ -321,7 +328,7 @@ export class AtomInstance< /** * `i`nit - Perform the initial run of this atom's state factory. Create the - * store, promise, exports, and hydrate (all optional except the store). + * `S`ignal (if any), promise, exports, and hydrate (all optional). */ public i() { const { n, s } = getEvaluationContext() @@ -345,6 +352,19 @@ export class AtomInstance< */ public j() { const { n, s } = getEvaluationContext() + + if (this.a && this.w.length === 1 && this.w[0].s === this.S) { + // if we altered the edge between this atom and its wrapped signal, the + // wrapped signal should not trigger an evaluation of this atom. Skip + // evaluation - just capture the state update and forward to this atom's + // observers. + const oldState = this.v + this.v = this.S!.v // `this.S`ignal must exist if `this.a`lteredEdge is true + handleStateChange(this, oldState, this.b) + + return + } + this._nextInjectors = [] this._isEvaluating = true startBuffer(this) @@ -376,6 +396,21 @@ export class AtomInstance< this.v === oldState || handleStateChange(this, oldState, this.b) } + + if (this.S) { + const edge = this.s.get(newFactoryResult) + + if (edge) { + // the edge between this atom and its wrapped signal needs to be + // reactive. Track whether we altered the `p`endingFlags added by + // `injectSignal`/similar. If we did, the edge was static and should + // not trigger a reevaluation of this atom. + this.a = !!(edge.p! & Static) // `.p!` - doesn't matter if undefined + edge.p = 0 + } else { + bufferEdge(newFactoryResult, 'implicit', 0) + } + } } catch (err) { this._nextInjectors.forEach(injector => { injector.cleanup?.() @@ -387,7 +422,7 @@ export class AtomInstance< } finally { this._isEvaluating = false - // even if evaluation errored, we need to update dependents if the store's + // even if evaluation errored, we need to update dependents if the `S`ignal's // state changed if (this.S && this.S.v !== this.v) { const oldState = this.v @@ -482,16 +517,18 @@ export class AtomInstance< // status is Stale (and should we just always evaluate once when // waking up a stale atom)? if (this.l !== 'Destroyed' && this.w.push(reason) === 1) { + if (reason.s && reason.s === this.S) { + // when `this.S`ignal gives us events along with a state update, we need + // to buffer those and emit them together after this atom evaluates + if (reason.e) { + this.b = this.b + ? { ...this.b, ...(reason.e as typeof this.b) } + : (reason.e as unknown as typeof this.b) + } + } + // refCount just hit 1; we haven't scheduled a job for this node yet this.e._scheduler.schedule(this, defer) - - // when `this.S`ignal gives us events along with a state update, we need - // to buffer those and emit them together after this atom evaluates - if (reason.s === this.S && reason.e) { - this.b = this.b - ? { ...this.b, ...(reason.e as typeof this.b) } - : (reason.e as unknown as typeof this.b) - } } } diff --git a/packages/atoms/src/index.ts b/packages/atoms/src/index.ts index 7d5be04c..dea848eb 100644 --- a/packages/atoms/src/index.ts +++ b/packages/atoms/src/index.ts @@ -1,8 +1,3 @@ -import { - destroyNodeFinish, - destroyNodeStart, - scheduleDependents, -} from './classes/GraphNode' import { createInjector } from './factories/createInjector' import { destroyBuffer, @@ -10,6 +5,11 @@ import { getEvaluationContext, startBuffer, } from './utils/evaluationContext' +import { + destroyNodeFinish, + destroyNodeStart, + scheduleDependents, +} from './utils/graph' export * from '@zedux/core' export * from './classes/index' diff --git a/packages/atoms/src/injectors/injectAtomInstance.ts b/packages/atoms/src/injectors/injectAtomInstance.ts index 7445d246..cd18e6e3 100644 --- a/packages/atoms/src/injectors/injectAtomInstance.ts +++ b/packages/atoms/src/injectors/injectAtomInstance.ts @@ -59,7 +59,7 @@ export const injectAtomInstance: { params?: ParamsOf, config?: InjectAtomInstanceConfig ) => { - const injectedInstance = instance.e.live.getInstance( + const injectedInstance = instance.e.live.getNode( atom as A, params as ParamsOf, { @@ -81,7 +81,7 @@ export const injectAtomInstance: { config?: InjectAtomInstanceConfig ) => { // make sure the dependency gets registered for this evaluation - const injectedInstance = instance.e.live.getInstance( + const injectedInstance = instance.e.live.getNode( atom as A, params as ParamsOf, { diff --git a/packages/atoms/src/injectors/injectAtomState.ts b/packages/atoms/src/injectors/injectAtomState.ts index 54c9951a..0d8a56f1 100644 --- a/packages/atoms/src/injectors/injectAtomState.ts +++ b/packages/atoms/src/injectors/injectAtomState.ts @@ -41,5 +41,5 @@ export const injectAtomState: { subscribe: true, }) - return [instance.get(), instance._infusedSetter] + return [instance.v, instance._infusedSetter] } diff --git a/packages/atoms/src/injectors/injectAtomValue.ts b/packages/atoms/src/injectors/injectAtomValue.ts index 4db64a86..b4b62521 100644 --- a/packages/atoms/src/injectors/injectAtomValue.ts +++ b/packages/atoms/src/injectors/injectAtomValue.ts @@ -24,4 +24,4 @@ export const injectAtomValue: { injectAtomInstance(atom, params, { operation: 'injectAtomValue', subscribe: true, - }).get() + }).v diff --git a/packages/atoms/src/injectors/injectSelf.ts b/packages/atoms/src/injectors/injectSelf.ts index d2cd2a29..20bbeb52 100644 --- a/packages/atoms/src/injectors/injectSelf.ts +++ b/packages/atoms/src/injectors/injectSelf.ts @@ -5,7 +5,7 @@ import { readInstance } from '../utils/evaluationContext' * An unrestricted injector (can actually be used in loops and if statements). * * Returns the currently-evaluating AtomInstance. Note that this instance will - * not have its `exports`, `promise`, or `store` set yet on initial evaluation. + * not have its `exports`, `promise`, or `signal` set yet on initial evaluation. */ export const injectSelf = readInstance as () => | AnyAtomInstance diff --git a/packages/atoms/src/injectors/injectSignal.ts b/packages/atoms/src/injectors/injectSignal.ts index 9895d558..7bfeaa0a 100644 --- a/packages/atoms/src/injectors/injectSignal.ts +++ b/packages/atoms/src/injectors/injectSignal.ts @@ -2,7 +2,6 @@ import { Signal } from '../classes/Signal' import { EventMap, InjectSignalConfig, MapEvents, None } from '../types/index' import { readInstance } from '../utils/evaluationContext' import { Static } from '../utils/general' -import { injectAtomGetters } from './injectAtomGetters' import { injectMemo } from './injectMemo' /** @@ -51,7 +50,7 @@ export const injectSignal = ( }, []) // create a graph edge between the current atom and the new signal - injectAtomGetters().getNode(signal, undefined, { + instance.e.live.getNode(signal, undefined, { f: config?.reactive === false ? Static : 0, op: 'injectSignal', }) diff --git a/packages/atoms/src/types/index.ts b/packages/atoms/src/types/index.ts index 4edc8e0d..ae4a42ae 100644 --- a/packages/atoms/src/types/index.ts +++ b/packages/atoms/src/types/index.ts @@ -277,13 +277,6 @@ export interface EvaluationReason { type: EvaluationType } -export type EvaluationSourceType = - | 'Atom' - | 'AtomSelector' - | 'External' - | 'Injector' - | 'Store' - export type EvaluationType = | 'cache invalidated' | 'node destroyed' @@ -451,7 +444,7 @@ export type ParamlessTemplate< */ export type PartialAtomInstance = Omit< AnyAtomInstance, - 'api' | 'exports' | 'promise' | 'store' + 'api' | 'exports' | 'promise' | 'S' > // from Matt Pocock https://twitter.com/mattpocockuk/status/1622730173446557697 diff --git a/packages/atoms/src/utils/evaluationContext.ts b/packages/atoms/src/utils/evaluationContext.ts index 49db9dd4..93cf75f2 100644 --- a/packages/atoms/src/utils/evaluationContext.ts +++ b/packages/atoms/src/utils/evaluationContext.ts @@ -1,14 +1,10 @@ import { ActionFactoryPayloadType, is } from '@zedux/core' import { AnyAtomInstance } from '../types/index' -import { ExplicitExternal, OutOfRange } from '../utils/general' -import { pluginActions } from '../utils/plugin-actions' -import { - addEdge, - GraphNode, - removeEdge, - scheduleNodeDestruction, -} from '../classes/GraphNode' +import { type GraphNode } from '../classes/GraphNode' import { AtomInstance } from '../classes/instances/AtomInstance' +import { ExplicitExternal, OutOfRange } from './general' +import { addEdge, removeEdge, scheduleNodeDestruction } from './graph' +import { pluginActions } from './plugin-actions' export interface EvaluationContext { /** diff --git a/packages/atoms/src/utils/general.ts b/packages/atoms/src/utils/general.ts index 370921ad..160b767b 100644 --- a/packages/atoms/src/utils/general.ts +++ b/packages/atoms/src/utils/general.ts @@ -63,7 +63,7 @@ export const makeReasonsReadable = ( internalReasons: InternalEvaluationReason[] | undefined = node?.w ): EvaluationReason[] | undefined => internalReasons?.map(reason => ({ - newState: reason.s?.get(), + newState: reason.s?.v, oldState: reason.p, operation: node?.s.get(reason.s!)?.operation, reasons: reason.r && makeReasonsReadable(reason.s, reason.r), diff --git a/packages/atoms/src/utils/graph.ts b/packages/atoms/src/utils/graph.ts new file mode 100644 index 00000000..dc313f68 --- /dev/null +++ b/packages/atoms/src/utils/graph.ts @@ -0,0 +1,206 @@ +import { + AtomGenerics, + GraphEdge, + InternalEvaluationReason, + LifecycleStatus, +} from '@zedux/atoms/types/index' +import { type GraphNode } from '../classes/GraphNode' +import { ExplicitEvents } from '../types/events' +import { pluginActions } from './plugin-actions' +import { Destroy, Static } from './general' + +/** + * Actually add an edge to the graph. When we buffer graph updates, we're + * really just deferring the calling of this method. + */ +export const addEdge = ( + dependent: GraphNode, + dependency: GraphNode, + newEdge: GraphEdge +) => { + const { _mods, modBus } = dependency.e + + // draw the edge in both nodes. Dependent may not exist if it's an external + // pseudo-node + dependent && dependent.s.set(dependency, newEdge) + dependency.o.set(dependent, newEdge) + dependency.c?.() + + // static dependencies don't change a node's weight + if (!(newEdge.flags & Static)) { + recalculateNodeWeight(dependency.W, dependent) + } + + if (_mods.edgeCreated) { + modBus.dispatch( + pluginActions.edgeCreated({ + dependency, + dependent: dependent, // unfortunate but not changing for now + edge: newEdge, + }) + ) + } + + return newEdge +} + +export const destroyNodeStart = (node: GraphNode, force?: boolean) => { + // If we're not force-destroying, don't destroy if there are dependents + if (node.l === 'Destroyed' || (!force && node.o.size)) return + + node.c?.() + node.c = undefined + + setNodeStatus(node, 'Destroyed') + + if (node.w.length) node.e._scheduler.unschedule(node) + + return true +} + +// TODO: merge this into destroyNodeStart. We should be able to +export const destroyNodeFinish = (node: GraphNode) => { + // first remove all edges between this node and its dependencies + for (const dependency of node.s.keys()) { + removeEdge(node, dependency) + } + + // if an atom instance is force-destroyed, it could still have dependents. + // Inform them of the destruction + scheduleDependents( + { + r: node.w, + s: node, + t: Destroy, + }, + true, + true + ) + + // now remove all edges between this node and its dependents + for (const [observer, edge] of node.o) { + if (!(edge.flags & Static)) { + recalculateNodeWeight(-node.W, observer) + } + + observer.s.delete(node) + + // we _probably_ don't need to send edgeRemoved mod events to plugins for + // these - it's better that they receive the duplicate edgeCreated event + // when the dependency is recreated by its dependent(s) so they can infer + // that the edge was "moved" + } + + node.e.n.delete(node.id) +} + +export const handleStateChange = < + G extends Pick +>( + node: GraphNode, + oldState: G['State'], + events?: Partial +) => { + scheduleDependents({ e: events, p: oldState, r: node.w, s: node }, false) + + if (node.e._mods.stateChanged) { + node.e.modBus.dispatch( + pluginActions.stateChanged({ + node, + newState: node.v, + oldState, + reasons: node.w, + }) + ) + } + + // run the scheduler synchronously after any node state update + events?.batch || node.e._scheduler.flush() +} + +const recalculateNodeWeight = (weightDiff: number, node?: GraphNode) => { + if (!node) return // happens when node is external + + node.W += weightDiff + + for (const observer of node.o.keys()) { + recalculateNodeWeight(weightDiff, observer) + } +} + +/** + * Remove the graph edge between two nodes. The dependent may not exist as a + * node in the graph if it's external, e.g. a React component + * + * For some reason in React 18+, React destroys parents before children. This + * means a parent EcosystemProvider may have already unmounted and wiped the + * whole graph; this edge may already be destroyed. + */ +export const removeEdge = (dependent: GraphNode, dependency: GraphNode) => { + // erase graph edge between dependent and dependency + dependent && dependent.s.delete(dependency) + + // hmm could maybe happen when a dependency was force-destroyed if a child + // tries to destroy its edge before recreating it (I don't think we ever do + // that though) + if (!dependency) return + + const edge = dependency.o.get(dependent) + + // happens in React 18+ (see this method's jsdoc above) + if (!edge) return + + dependency.o.delete(dependent) + + // static dependencies don't change a node's weight + if (!(edge.flags & Static)) { + recalculateNodeWeight(-dependency.W, dependent) + } + + if (dependency.e._mods.edgeRemoved) { + dependency.e.modBus.dispatch( + pluginActions.edgeRemoved({ + dependency, + dependent: dependent, + edge: edge, + }) + ) + } + + scheduleNodeDestruction(dependency) +} + +export const scheduleDependents = ( + reason: Omit & { + s: NonNullable + }, + defer?: boolean, + scheduleStaticDeps?: boolean +) => { + for (const [observer, edge] of reason.s.o) { + // Static deps don't update on state change, only on promise change or node + // force-destruction + if (scheduleStaticDeps || !(edge.flags & Static)) observer.r(reason, defer) + } +} + +/** + * When a node's refCount hits 0, schedule destruction of that node. + */ +export const scheduleNodeDestruction = (node: GraphNode) => + node.o.size || node.l !== 'Active' || node.m() + +export const setNodeStatus = (node: GraphNode, newStatus: LifecycleStatus) => { + const oldStatus = node.l + node.l = newStatus + + if (node.e._mods.statusChanged) { + node.e.modBus.dispatch( + pluginActions.statusChanged({ + newStatus, + node, + oldStatus, + }) + ) + } +} diff --git a/packages/react/src/hooks/useAtomInstance.ts b/packages/react/src/hooks/useAtomInstance.ts index 2f1a19f0..15bc56bd 100644 --- a/packages/react/src/hooks/useAtomInstance.ts +++ b/packages/react/src/hooks/useAtomInstance.ts @@ -82,7 +82,7 @@ export const useAtomInstance: { // It should be fine for this to run every render. It's possible to change // approaches if it is too heavy sometimes. But don't memoize this call: const instance: AtomInstance = ecosystem.getNode(atom, params) - const renderedValue = instance.get() + const renderedValue = instance.v let node = (ecosystem.n.get(observerId) as ExternalNode) ?? @@ -110,7 +110,7 @@ export const useAtomInstance: { // an unmounting component's effect cleanup can update or force-destroy the // atom instance before this component is mounted. If that happened, trigger // a rerender to recreate the atom instance and/or get its new state - if (instance.get() !== renderedValue || instance.l === 'Destroyed') { + if (instance.v !== renderedValue || instance.l === 'Destroyed') { render({}) } diff --git a/packages/react/src/hooks/useAtomState.ts b/packages/react/src/hooks/useAtomState.ts index 99f294fc..eace4eb4 100644 --- a/packages/react/src/hooks/useAtomState.ts +++ b/packages/react/src/hooks/useAtomState.ts @@ -74,5 +74,5 @@ export const useAtomState: { subscribe: true, }) - return [instance.get(), instance._infusedSetter] + return [instance.v, instance._infusedSetter] } diff --git a/packages/react/src/hooks/useAtomValue.ts b/packages/react/src/hooks/useAtomValue.ts index 7f94a7cf..137dc9e4 100644 --- a/packages/react/src/hooks/useAtomValue.ts +++ b/packages/react/src/hooks/useAtomValue.ts @@ -61,4 +61,4 @@ export const useAtomValue: { useAtomInstance(atom, params, { ...config, subscribe: true, - }).get() + }).v diff --git a/packages/react/test/integrations/ecosystem.test.tsx b/packages/react/test/integrations/ecosystem.test.tsx index dbe6ff24..26a6133e 100644 --- a/packages/react/test/integrations/ecosystem.test.tsx +++ b/packages/react/test/integrations/ecosystem.test.tsx @@ -30,9 +30,9 @@ describe('ecosystem', () => { const atom1 = atom('atom1', () => { evaluate1() - const store = injectSignal('1') + const signal = injectSignal('1') - return store + return signal }) const atom2 = atom('atom2', () => { diff --git a/packages/react/test/integrations/injectors.test.tsx b/packages/react/test/integrations/injectors.test.tsx index 9d50126f..3a25a03d 100644 --- a/packages/react/test/integrations/injectors.test.tsx +++ b/packages/react/test/integrations/injectors.test.tsx @@ -84,6 +84,9 @@ describe('injectors', () => { instance.set('b') + // all those `get` calls shouldn't add any edges besides the one already + // added by `injectSignal`: + expect(instance.s.size).toBe(1) expect(vals).toEqual(['a', 'a', 'a', 'b', 'a', 'b']) expect(cbs).toEqual(['aa', 'aa', 'aa', 'bb', 'aa', 'bb']) expect(effects).toEqual(['b']) @@ -165,7 +168,7 @@ describe('injectors', () => { const signal = injectSignal('a', { reactive: isReactive }) const instance2 = injectAtomInstance(atom2) - vals.push([signal.get(), isReactive, instance2.get()]) + vals.push([signal.get(), isReactive, instance2.getOnce()]) return api(signal).setExports({ invalidate, diff --git a/packages/react/test/integrations/plugins.test.tsx b/packages/react/test/integrations/plugins.test.tsx index b8bb0661..3a242bf5 100644 --- a/packages/react/test/integrations/plugins.test.tsx +++ b/packages/react/test/integrations/plugins.test.tsx @@ -214,7 +214,7 @@ describe('plugins', () => { > ).payload - const state = node.get() + const state = node.getOnce() updates.push(state) } }, diff --git a/packages/react/test/integrations/signals.test.tsx b/packages/react/test/integrations/signals.test.tsx index 7dd600fe..1712621b 100644 --- a/packages/react/test/integrations/signals.test.tsx +++ b/packages/react/test/integrations/signals.test.tsx @@ -134,6 +134,28 @@ describe('signals', () => { expect(calls).toEqual([['change', expectedEvents2.change, expectedEvents2]]) }) + + test("a non-reactively-injected signal still updates the atom's value", () => { + const testAtom = atom('test', () => { + const signal = injectSignal(1, { reactive: false }) + + return api(signal).setExports({ + increment: () => signal.set(state => state + 1), + }) + }) + + const testInstance = ecosystem.getNode(testAtom) + + expect(testInstance.v).toBe(1) + expect(testInstance.get()).toBe(1) + expect(testInstance.getOnce()).toBe(1) + + testInstance.exports.increment() + + expect(testInstance.v).toBe(2) + expect(testInstance.get()).toBe(2) + expect(testInstance.getOnce()).toBe(2) + }) }) describe('mapped signals', () => { diff --git a/packages/react/test/types.test.tsx b/packages/react/test/types.test.tsx index 58374861..ddc63c28 100644 --- a/packages/react/test/types.test.tsx +++ b/packages/react/test/types.test.tsx @@ -615,7 +615,7 @@ describe('react types', () => { const val6 = injectCallback(() => true, []) const val7 = injectPromise(() => Promise.resolve(1), []) - return api(injectSignal(instance.get())).setExports({ + return api(injectSignal(instance.getOnce())).setExports({ val1, val2, val3, diff --git a/packages/stores/src/AtomInstance.ts b/packages/stores/src/AtomInstance.ts index 7178e22c..64c27221 100644 --- a/packages/stores/src/AtomInstance.ts +++ b/packages/stores/src/AtomInstance.ts @@ -157,7 +157,8 @@ export class AtomInstance< * An alias for `instance.store.getState()`. Returns the current state of this * atom instance's store. * - * @deprecated - use `.get()` instead @see AtomInstance.get + * @deprecated - use `.getOnce()` instead @see NewAtomInstance.getOnce. Also + * see `.get()` for automatic reactivity. */ public getState(): G['State'] { return this.store.getState() @@ -169,6 +170,8 @@ export class AtomInstance< * An alias for `instance.store.getState()`. */ public get() { + super.get() // register graph edge + return this.store.getState() } @@ -248,6 +251,8 @@ export class AtomInstance< this._handleStateChange(newState, oldState, action) } ) + + this.v = this.store.getState() } else { const newStateType = getStateType(newFactoryResult) @@ -392,6 +397,7 @@ export class AtomInstance< oldState: G['State'] | undefined, action: ActionChain ) { + this.v = newState zi.u({ p: oldState, r: this.w, s: this }, false) if (this.e._mods.stateChanged) { From eb1ffe24389eb64d00d19a4e42f41da1c9ef626f Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Wed, 8 Jan 2025 21:17:57 -0500 Subject: [PATCH 2/8] make change events give listeners the full readable reason --- packages/atoms/src/classes/GraphNode.ts | 8 +++++-- packages/atoms/src/utils/general.ts | 21 ++++++++++++------- .../react/test/integrations/signals.test.tsx | 19 +++++++++++++++-- packages/react/test/types.test.tsx | 15 ++++++++++--- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/atoms/src/classes/GraphNode.ts b/packages/atoms/src/classes/GraphNode.ts index 88537042..44fb10d7 100644 --- a/packages/atoms/src/classes/GraphNode.ts +++ b/packages/atoms/src/classes/GraphNode.ts @@ -12,7 +12,11 @@ import { } from '@zedux/atoms/types/index' import { is, Job } from '@zedux/core' import { Ecosystem } from './Ecosystem' -import { EventSent, ExplicitExternal } from '../utils/general' +import { + EventSent, + ExplicitExternal, + makeReasonReadable, +} from '../utils/general' import { AtomTemplateBase } from './templates/AtomTemplateBase' import { CatchAllListener, @@ -151,7 +155,7 @@ export abstract class GraphNode< ? reason.e ?? {} : { ...reason.e, - change: { newState: this.v, oldState: reason.p }, + change: makeReasonReadable(reason, observer), } ) as ListenableEvents diff --git a/packages/atoms/src/utils/general.ts b/packages/atoms/src/utils/general.ts index 160b767b..06ccaaea 100644 --- a/packages/atoms/src/utils/general.ts +++ b/packages/atoms/src/utils/general.ts @@ -58,15 +58,20 @@ const reasonTypeMap = { 4: 'state changed', } as const +export const makeReasonReadable = ( + reason: InternalEvaluationReason, + node?: GraphNode +) => ({ + newState: reason.s?.v, + oldState: reason.p, + operation: node?.s.get(reason.s!)?.operation, + reasons: reason.r && makeReasonsReadable(reason.s, reason.r), + source: reason.s, + type: reasonTypeMap[reason.t ?? 4], +}) + export const makeReasonsReadable = ( node?: GraphNode, internalReasons: InternalEvaluationReason[] | undefined = node?.w ): EvaluationReason[] | undefined => - internalReasons?.map(reason => ({ - newState: reason.s?.v, - oldState: reason.p, - operation: node?.s.get(reason.s!)?.operation, - reasons: reason.r && makeReasonsReadable(reason.s, reason.r), - source: reason.s, - type: reasonTypeMap[reason.t ?? 4], - })) + internalReasons?.map(reason => makeReasonReadable(reason, node)) diff --git a/packages/react/test/integrations/signals.test.tsx b/packages/react/test/integrations/signals.test.tsx index 1712621b..5e2b33df 100644 --- a/packages/react/test/integrations/signals.test.tsx +++ b/packages/react/test/integrations/signals.test.tsx @@ -112,8 +112,19 @@ describe('signals', () => { state.b = [] }) + const commonChangeProps = { + operation: 'on', + reasons: [], + source: instance1.exports.signal, + type: 'state changed', + } + const expectedEvents = { - change: { newState: { a: 1, b: [] }, oldState: { a: 1, b: [{ c: 2 }] } }, + change: { + newState: { a: 1, b: [] }, + oldState: { a: 1, b: [{ c: 2 }] }, + ...commonChangeProps, + }, mutate: [ { k: ['b', '0', 'c'], v: 3 }, { k: 'b', v: [] }, @@ -129,7 +140,11 @@ describe('signals', () => { instance1.exports.signal.set(state => ({ ...state, a: state.a + 1 })) const expectedEvents2 = { - change: { newState: { a: 2, b: [] }, oldState: { a: 1, b: [] } }, + change: { + newState: { a: 2, b: [] }, + oldState: { a: 1, b: [] }, + ...commonChangeProps, + }, } expect(calls).toEqual([['change', expectedEvents2.change, expectedEvents2]]) diff --git a/packages/react/test/types.test.tsx b/packages/react/test/types.test.tsx index ddc63c28..a9c294c1 100644 --- a/packages/react/test/types.test.tsx +++ b/packages/react/test/types.test.tsx @@ -793,14 +793,23 @@ describe('react types', () => { signal.set(state => state + 1, { a: 11 }) + const expectedChangeEvent = { + newState: 2, + oldState: 1, + operation: 'on', + reasons: [], + source: signal, + type: 'state changed', + } + expect(calls).toEqual([ - ['a', 11, { a: 11, change: { newState: 2, oldState: 1 } }], + ['a', 11, { a: 11, change: expectedChangeEvent }], [ 'change', { newState: 2, oldState: 1 }, - { a: 11, change: { newState: 2, oldState: 1 } }, + { a: 11, change: expectedChangeEvent }, ], - ['*', { a: 11, change: { newState: 2, oldState: 1 } }], + ['*', { a: 11, change: expectedChangeEvent }], ]) calls.splice(0, 3) From 61a1e9943398aca9a918f8d833692dc88a03c940 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Thu, 9 Jan 2025 11:27:27 -0500 Subject: [PATCH 3/8] add `GraphNode#K`eep to prevent destruction --- packages/atoms/src/classes/GraphNode.ts | 8 ++++++++ packages/atoms/src/classes/SelectorInstance.ts | 3 +++ packages/atoms/src/utils/graph.ts | 5 +++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/atoms/src/classes/GraphNode.ts b/packages/atoms/src/classes/GraphNode.ts index 44fb10d7..7ad9e6c3 100644 --- a/packages/atoms/src/classes/GraphNode.ts +++ b/packages/atoms/src/classes/GraphNode.ts @@ -50,6 +50,14 @@ export abstract class GraphNode< */ public izn = true + /** + * `K`eep - whether to prevent this node from ever being destroyed. When + * creating signals/selectors/atom instances outside of reactive contexts, you + * almost never want the node to be destroyed. Set this to true to keep it + * around regardless of ref count. + */ + public K = false + /** * @see Job.T */ diff --git a/packages/atoms/src/classes/SelectorInstance.ts b/packages/atoms/src/classes/SelectorInstance.ts index e829b263..f3b7244f 100644 --- a/packages/atoms/src/classes/SelectorInstance.ts +++ b/packages/atoms/src/classes/SelectorInstance.ts @@ -134,10 +134,12 @@ export class SelectorInstance< * @see GraphNode.e */ public e: Ecosystem, + /** * @see GraphNode.id */ public id: string, + /** * `t`emplate - the function or object reference of this selector or * selector config object @@ -145,6 +147,7 @@ export class SelectorInstance< * @see GraphNode.t */ public t: G['Template'], + /** * @see GraphNode.p */ diff --git a/packages/atoms/src/utils/graph.ts b/packages/atoms/src/utils/graph.ts index dc313f68..bd46a743 100644 --- a/packages/atoms/src/utils/graph.ts +++ b/packages/atoms/src/utils/graph.ts @@ -45,8 +45,9 @@ export const addEdge = ( } export const destroyNodeStart = (node: GraphNode, force?: boolean) => { - // If we're not force-destroying, don't destroy if there are dependents - if (node.l === 'Destroyed' || (!force && node.o.size)) return + // If we're not force-destroying, don't destroy if there are dependents. Also + // don't destroy of `node.K`eep is set + if (node.l === 'Destroyed' || node.K || (!force && node.o.size)) return node.c?.() node.c = undefined From 0cb583eb2b5629005ebc9ea5e127b108f2144012 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Mon, 13 Jan 2025 09:31:45 -0500 Subject: [PATCH 4/8] add ChangeEvent type; rework evaluation type strings --- packages/atoms/src/classes/GraphNode.ts | 13 +--- .../src/classes/instances/AtomInstance.ts | 2 +- packages/atoms/src/types/atoms.ts | 11 +++ packages/atoms/src/types/events.ts | 30 +++++--- packages/atoms/src/types/index.ts | 33 +++++--- packages/atoms/src/utils/general.ts | 12 ++- packages/atoms/src/utils/graph.ts | 4 +- .../test/integrations/injectors.test.tsx | 4 +- .../react/test/integrations/signals.test.tsx | 77 ++++--------------- packages/react/test/stores/graph.test.tsx | 6 +- packages/react/test/stores/injectors.test.tsx | 4 +- packages/react/test/types.test.tsx | 9 +-- packages/stores/src/AtomInstance.ts | 2 +- packages/stores/src/injectPromise.ts | 2 +- 14 files changed, 92 insertions(+), 117 deletions(-) diff --git a/packages/atoms/src/classes/GraphNode.ts b/packages/atoms/src/classes/GraphNode.ts index 7ad9e6c3..3a95b7b1 100644 --- a/packages/atoms/src/classes/GraphNode.ts +++ b/packages/atoms/src/classes/GraphNode.ts @@ -1,6 +1,6 @@ import { AnyAtomTemplate, - AtomGenerics, + AnyNodeGenerics, Cleanup, DehydrationFilter, GraphEdge, @@ -9,6 +9,7 @@ import { LifecycleStatus, NodeFilter, NodeFilterOptions, + NodeGenerics, } from '@zedux/atoms/types/index' import { is, Job } from '@zedux/core' import { Ecosystem } from './Ecosystem' @@ -27,14 +28,8 @@ import { import { bufferEdge, getEvaluationContext } from '../utils/evaluationContext' import { addEdge, removeEdge, setNodeStatus } from '../utils/graph' -export abstract class GraphNode< - G extends Pick = { - Events: any - Params: any - State: any - Template: any - } -> implements Job, EventEmitter +export abstract class GraphNode + implements Job, EventEmitter { /** * TS drops the entire `G`enerics type unless it's used somewhere in this diff --git a/packages/atoms/src/classes/instances/AtomInstance.ts b/packages/atoms/src/classes/instances/AtomInstance.ts index 8dc98326..1dfb88fa 100644 --- a/packages/atoms/src/classes/instances/AtomInstance.ts +++ b/packages/atoms/src/classes/instances/AtomInstance.ts @@ -254,7 +254,7 @@ export class AtomInstance< * Force this atom instance to reevaluate. */ public invalidate() { - this.r({ t: Invalidate }, false) + this.r({ s: this, t: Invalidate }, false) // run the scheduler synchronously after invalidation this.e._scheduler.flush() diff --git a/packages/atoms/src/types/atoms.ts b/packages/atoms/src/types/atoms.ts index 8c0a428e..85dde03b 100644 --- a/packages/atoms/src/types/atoms.ts +++ b/packages/atoms/src/types/atoms.ts @@ -36,6 +36,12 @@ export type AnyAtomTemplate | 'any' = 'any'> = : any > +export type AnyNodeGenerics< + G extends Partial = { [K in keyof NodeGenerics]: any } +> = { + [K in keyof NodeGenerics]: keyof G extends K ? G[K] : any +} + export type AtomApiGenerics = Pick< AtomGenerics, 'Exports' | 'Promise' | 'State' @@ -89,6 +95,11 @@ export type ExportsOf = ? G['Exports'] : never +export type NodeGenerics = Pick< + AtomGenerics, + 'Events' | 'Params' | 'State' | 'Template' +> + export type NodeOf = A extends AtomTemplateBase ? // this allows the Node generic to be extracted from functions that don't diff --git a/packages/atoms/src/types/events.ts b/packages/atoms/src/types/events.ts index 5f713230..a72c2f7f 100644 --- a/packages/atoms/src/types/events.ts +++ b/packages/atoms/src/types/events.ts @@ -1,5 +1,13 @@ import { RecursivePartial } from '@zedux/core' -import { AtomGenerics, Cleanup, GraphEdgeConfig, Prettify } from './index' +import { + AnyNodeGenerics, + AtomGenerics, + ChangeEvent, + Cleanup, + GraphEdgeConfig, + NodeGenerics, + Prettify, +} from './index' /** * Events that can be dispatched manually. This is not the full list of events @@ -26,12 +34,11 @@ export interface ExplicitEvents { mutate: Transaction[] } -export type CatchAllListener> = - (eventMap: Partial>) => void +export type CatchAllListener = ( + eventMap: Partial> +) => void -export interface EventEmitter< - G extends Pick = { Events: any; State: any } -> { +export interface EventEmitter { // TODO: add a `passive` option for listeners that don't prevent destruction on( eventName: E, @@ -42,12 +49,13 @@ export interface EventEmitter< on(callback: CatchAllListener, edgeDetails?: GraphEdgeConfig): Cleanup } -export interface ImplicitEvents { - change: { newState: State; oldState: State } +export interface ImplicitEvents { + change: ChangeEvent } -export type ListenableEvents> = - Prettify> +export type ListenableEvents = Prettify< + G['Events'] & ExplicitEvents & ImplicitEvents +> export type Mutatable = | RecursivePartial @@ -60,7 +68,7 @@ export type SendableEvents> = Prettify< > export type SingleEventListener< - G extends Pick, + G extends NodeGenerics, E extends keyof G['Events'] > = ( payload: ListenableEvents[E], diff --git a/packages/atoms/src/types/index.ts b/packages/atoms/src/types/index.ts index ae4a42ae..5257394f 100644 --- a/packages/atoms/src/types/index.ts +++ b/packages/atoms/src/types/index.ts @@ -1,4 +1,4 @@ -import { ActionChain, Observable, Settable } from '@zedux/core' +import { Observable, Settable } from '@zedux/core' import { AtomApi } from '../classes/AtomApi' import { Ecosystem } from '../classes/Ecosystem' import { GraphNode } from '../classes/GraphNode' @@ -10,8 +10,10 @@ import { AnyAtomGenerics, AnyAtomInstance, AnyAtomTemplate, + AnyNodeGenerics, AtomGenerics, AtomGenericsToAtomApiGenerics, + NodeGenerics, NodeOf, ParamsOf, StateOf, @@ -238,6 +240,14 @@ export type AtomValueOrFactory< } > = AtomStateFactory | G['State'] +export type ChangeEvent = Prettify< + Omit, 'type'> & { + newState: G['State'] + oldState: G['State'] + type: 'change' + } +> + export type Cleanup = () => void export interface DehydrationOptions extends NodeFilterOptions { @@ -267,21 +277,24 @@ export interface EcosystemConfig< export type EffectCallback = () => MaybeCleanup | Promise -export interface EvaluationReason { - action?: ActionChain - newState?: State - oldState?: State +export interface EvaluationReasonBase< + G extends NodeGenerics = AnyNodeGenerics +> { operation?: string // e.g. a method like "injectValue" - source?: GraphNode reasons?: EvaluationReason[] + source?: GraphNode type: EvaluationType } +export type EvaluationReason = + | ChangeEvent + | EvaluationReasonBase + export type EvaluationType = - | 'cache invalidated' - | 'node destroyed' - | 'promise changed' - | 'state changed' + | 'invalidate' + | 'destroy' + | 'promiseChange' + | 'change' /** * A user-defined object mapping custom event names to unused placeholder diff --git a/packages/atoms/src/utils/general.ts b/packages/atoms/src/utils/general.ts index 06ccaaea..ba3f067d 100644 --- a/packages/atoms/src/utils/general.ts +++ b/packages/atoms/src/utils/general.ts @@ -37,8 +37,6 @@ export type InternalEvaluationType = | typeof PromiseChange | typeof EventSent -export const isZeduxNode = 'isZeduxNode' - /** * Compare two arrays for shallow equality. Returns true if they're "equal". * Returns false if either array is undefined @@ -52,16 +50,16 @@ export const compare = (nextDeps?: any[], prevDeps?: any[]) => export const prefix = '@@zedux' const reasonTypeMap = { - [Destroy]: 'node destroyed', - [Invalidate]: 'cache invalidated', - [PromiseChange]: 'promise changed', - 4: 'state changed', + [Destroy]: 'destroy', + [Invalidate]: 'invalidate', + [PromiseChange]: 'promiseChange', + 4: 'change', } as const export const makeReasonReadable = ( reason: InternalEvaluationReason, node?: GraphNode -) => ({ +): EvaluationReason => ({ newState: reason.s?.v, oldState: reason.p, operation: node?.s.get(reason.s!)?.operation, diff --git a/packages/atoms/src/utils/graph.ts b/packages/atoms/src/utils/graph.ts index bd46a743..bf1ee9e2 100644 --- a/packages/atoms/src/utils/graph.ts +++ b/packages/atoms/src/utils/graph.ts @@ -47,7 +47,7 @@ export const addEdge = ( export const destroyNodeStart = (node: GraphNode, force?: boolean) => { // If we're not force-destroying, don't destroy if there are dependents. Also // don't destroy of `node.K`eep is set - if (node.l === 'Destroyed' || node.K || (!force && node.o.size)) return + if (node.l === 'Destroyed' || (!force && node.o.size)) return node.c?.() node.c = undefined @@ -189,7 +189,7 @@ export const scheduleDependents = ( * When a node's refCount hits 0, schedule destruction of that node. */ export const scheduleNodeDestruction = (node: GraphNode) => - node.o.size || node.l !== 'Active' || node.m() + node.o.size || node.l !== 'Active' || node.K || node.m() export const setNodeStatus = (node: GraphNode, newStatus: LifecycleStatus) => { const oldStatus = node.l diff --git a/packages/react/test/integrations/injectors.test.tsx b/packages/react/test/integrations/injectors.test.tsx index 3a25a03d..4ca00055 100644 --- a/packages/react/test/integrations/injectors.test.tsx +++ b/packages/react/test/integrations/injectors.test.tsx @@ -268,7 +268,7 @@ describe('injectors', () => { operation: 'injectSignal', reasons: [], source: ecosystem.find('@signal(1)'), - type: 'state changed', + type: 'change', }, ], [ @@ -278,7 +278,7 @@ describe('injectors', () => { operation: 'injectSignal', reasons: [], source: ecosystem.find('@signal(1)'), - type: 'state changed', + type: 'change', }, ], ]) diff --git a/packages/react/test/integrations/signals.test.tsx b/packages/react/test/integrations/signals.test.tsx index 5e2b33df..7c57163b 100644 --- a/packages/react/test/integrations/signals.test.tsx +++ b/packages/react/test/integrations/signals.test.tsx @@ -2,8 +2,10 @@ import { api, As, atom, - injectMappedSignal, + ChangeEvent, + GraphNode, injectSignal, + Signal, StateOf, Transaction, } from '@zedux/atoms' @@ -98,11 +100,17 @@ describe('signals', () => { calls.push(['mutate', transactions, eventMap]) }) + type GenericsOf = T extends Signal ? G : never + + type ExpectedChangeEvent = ChangeEvent< + GenericsOf & { + Params: undefined + Template: undefined + } + > + instance1.exports.signal.on('change', (change, eventMap) => { - expectTypeOf(change).toEqualTypeOf<{ - newState: StateOf - oldState: StateOf - }>() + expectTypeOf(change).toEqualTypeOf() calls.push(['change', change, eventMap]) }) @@ -116,7 +124,7 @@ describe('signals', () => { operation: 'on', reasons: [], source: instance1.exports.signal, - type: 'state changed', + type: 'change', } const expectedEvents = { @@ -172,60 +180,3 @@ describe('signals', () => { expect(testInstance.getOnce()).toBe(2) }) }) - -describe('mapped signals', () => { - test('mapped signals forward state updates to inner signals', () => { - const values: string[] = [] - - const atom1 = atom('1', () => { - const a = injectSignal('a') - const b = injectSignal('b') - - values.push(a.get(), b.get()) - - const mappedSignal = injectMappedSignal({ a, b }) - - return mappedSignal - }) - - expectTypeOf>().toEqualTypeOf<{ - a: string - b: string - }>() - - const instance1 = ecosystem.getNode(atom1) - - expect(instance1.get()).toEqual({ a: 'a', b: 'b' }) - expect(values).toEqual(['a', 'b']) - - instance1.set(state => ({ ...state, a: 'aa' })) - - expect(instance1.get()).toEqual({ a: 'aa', b: 'b' }) - expect(values).toEqual(['a', 'b', 'aa', 'b']) - - instance1.mutate(state => { - state.b = 'bb' - }) - - expect(instance1.get()).toEqual({ a: 'aa', b: 'bb' }) - expect(values).toEqual(['a', 'b', 'aa', 'b', 'aa', 'bb']) - }) -}) - -// const signalA = injectSignal('a', { -// events: { eventA: As, eventC: As }, -// }) -// const signalB = injectSignal('b', { -// events: { eventA: As, eventB: As<1 | 2> }, -// }) - -// const result = injectMappedSignal({ -// a: signalA, -// b: signalB, -// }) - -// type TEvents = EventsOf -// type Tuple = UnionToTuple> -// type TEvents4 = TupleToEvents - -// result.on('eventB', (test, map) => 2) diff --git a/packages/react/test/stores/graph.test.tsx b/packages/react/test/stores/graph.test.tsx index dae08e1b..11e9a909 100644 --- a/packages/react/test/stores/graph.test.tsx +++ b/packages/react/test/stores/graph.test.tsx @@ -256,7 +256,7 @@ describe('graph', () => { operation: 'get', reasons: [], source: expect.any(Object), - type: 'state changed', + type: 'change', }, { newState: 'aab', @@ -269,11 +269,11 @@ describe('graph', () => { operation: 'get', reasons: [], source: expect.any(Object), - type: 'state changed', + type: 'change', }, ], source: expect.any(Object), - type: 'state changed', + type: 'change', }, ]) }) diff --git a/packages/react/test/stores/injectors.test.tsx b/packages/react/test/stores/injectors.test.tsx index a2b40514..56bdb3e6 100644 --- a/packages/react/test/stores/injectors.test.tsx +++ b/packages/react/test/stores/injectors.test.tsx @@ -262,7 +262,7 @@ describe('injectors', () => { operation: undefined, reasons: undefined, source: undefined, - type: 'state changed', + type: 'change', }, ], [ @@ -272,7 +272,7 @@ describe('injectors', () => { operation: undefined, reasons: undefined, source: undefined, - type: 'state changed', + type: 'change', }, ], ]) diff --git a/packages/react/test/types.test.tsx b/packages/react/test/types.test.tsx index a9c294c1..fc35affe 100644 --- a/packages/react/test/types.test.tsx +++ b/packages/react/test/types.test.tsx @@ -32,6 +32,8 @@ import { As, None, Transaction, + ChangeEvent, + AnyNodeGenerics, } from '@zedux/react' import { expectTypeOf } from 'expect-type' import { ecosystem, snapshotNodes } from './utils/ecosystem' @@ -763,10 +765,7 @@ describe('react types', () => { b: undefined batch: boolean mutate: Transaction[] - change: { - newState: number - oldState: number - } + change: ChangeEvent> }> const calls: any[] = [] @@ -799,7 +798,7 @@ describe('react types', () => { operation: 'on', reasons: [], source: signal, - type: 'state changed', + type: 'change', } expect(calls).toEqual([ diff --git a/packages/stores/src/AtomInstance.ts b/packages/stores/src/AtomInstance.ts index 64c27221..52fd0488 100644 --- a/packages/stores/src/AtomInstance.ts +++ b/packages/stores/src/AtomInstance.ts @@ -179,7 +179,7 @@ export class AtomInstance< * Force this atom instance to reevaluate. */ public invalidate() { - this.r({ t: Invalidate }, false) + this.r({ s: this, t: Invalidate }, false) // run the scheduler synchronously after invalidation this.e._scheduler.flush() diff --git a/packages/stores/src/injectPromise.ts b/packages/stores/src/injectPromise.ts index 04570896..d7dd42fe 100644 --- a/packages/stores/src/injectPromise.ts +++ b/packages/stores/src/injectPromise.ts @@ -110,7 +110,7 @@ export const injectPromise: { if ( runOnInvalidate && // injectWhy is an unrestricted injector - using it conditionally is fine: - injectWhy().some(reason => reason.type === 'cache invalidated') + injectWhy().some(reason => reason.type === 'invalidate') ) { refs.current.counter++ } From a623f19e13ae95c5c6c43e6c5153d332454d63d5 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Mon, 13 Jan 2025 09:32:18 -0500 Subject: [PATCH 5/8] add mapped signal test suite --- .../test/integrations/mapped-signals.test.tsx | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 packages/react/test/integrations/mapped-signals.test.tsx diff --git a/packages/react/test/integrations/mapped-signals.test.tsx b/packages/react/test/integrations/mapped-signals.test.tsx new file mode 100644 index 00000000..f1511b47 --- /dev/null +++ b/packages/react/test/integrations/mapped-signals.test.tsx @@ -0,0 +1,117 @@ +import { + api, + atom, + injectMappedSignal, + injectSignal, + StateOf, +} from '@zedux/atoms' +import { ecosystem } from '../utils/ecosystem' +import { expectTypeOf } from 'expect-type' + +describe('mapped signals', () => { + test('forward state updates to inner signals', () => { + const values: string[] = [] + + const atom1 = atom('1', () => { + const a = injectSignal('a') + const b = injectSignal('b') + + values.push(a.get(), b.get()) + + const mappedSignal = injectMappedSignal({ a, b }) + + return mappedSignal + }) + + expectTypeOf>().toEqualTypeOf<{ + a: string + b: string + }>() + + const instance1 = ecosystem.getNode(atom1) + + expect(instance1.get()).toEqual({ a: 'a', b: 'b' }) + expect(values).toEqual(['a', 'b']) + + instance1.set(state => ({ ...state, a: 'aa' })) + + expect(instance1.get()).toEqual({ a: 'aa', b: 'b' }) + expect(values).toEqual(['a', 'b', 'aa', 'b']) + + instance1.mutate(state => { + state.b = 'bb' + }) + + expect(instance1.get()).toEqual({ a: 'aa', b: 'bb' }) + expect(values).toEqual(['a', 'b', 'aa', 'b', 'aa', 'bb']) + }) + + test('forward state updates from inner signals to observers', () => { + const atom1 = atom('1', () => { + const a = injectSignal('a') + const b = injectSignal({ nested: 'b' }) + + const mappedSignal = injectMappedSignal({ a, b }) + + return api(mappedSignal).setExports({ a, b }) + }) + + const instance1 = ecosystem.getNode(atom1) + const calls: any[] = [] + + instance1.on('change', ({ newState, oldState, type }) => { + calls.push({ newState, oldState, type }) + }) + + const expectedState1 = { a: 'a', b: { nested: 'b' } } + + expect(instance1.getOnce()).toEqual(expectedState1) + + instance1.exports.a.set('aa') + + const expectedState2 = { a: 'aa', b: { nested: 'b' } } + + expect(instance1.get()).toEqual(expectedState2) + expect(calls).toEqual([ + { + newState: expectedState2, + oldState: expectedState1, + type: 'change', + }, + ]) + calls.splice(0, 1) + + instance1.exports.b.mutate(state => { + state.nested = 'bb' + }) + + const expectedState3 = { a: 'aa', b: { nested: 'bb' } } + + expect(instance1.getOnce()).toEqual(expectedState3) + expect(calls).toEqual([ + { + newState: expectedState3, + oldState: expectedState2, + type: 'change', + }, + ]) + }) +}) + +// const signalA = injectSignal('a', { +// events: { eventA: As, eventC: As }, +// }) +// const signalB = injectSignal('b', { +// events: { eventA: As, eventB: As<1 | 2> }, +// }) + +// const result = injectMappedSignal({ +// a: signalA, +// b: signalB, +// }) + +// type TEvents = EventsOf +// type Tuple = UnionToTuple> +// type TEvents4 = TupleToEvents + +// result.on('eventB', (test, map) => 2) From 43047afc55df4790ef37543e65282b42ccc39bb8 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Tue, 21 Jan 2025 09:20:45 -0500 Subject: [PATCH 6/8] undo `GraphNode#K`eep --- packages/atoms/src/classes/GraphNode.ts | 8 -------- packages/atoms/src/utils/graph.ts | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/atoms/src/classes/GraphNode.ts b/packages/atoms/src/classes/GraphNode.ts index 3a95b7b1..c2989289 100644 --- a/packages/atoms/src/classes/GraphNode.ts +++ b/packages/atoms/src/classes/GraphNode.ts @@ -45,14 +45,6 @@ export abstract class GraphNode */ public izn = true - /** - * `K`eep - whether to prevent this node from ever being destroyed. When - * creating signals/selectors/atom instances outside of reactive contexts, you - * almost never want the node to be destroyed. Set this to true to keep it - * around regardless of ref count. - */ - public K = false - /** * @see Job.T */ diff --git a/packages/atoms/src/utils/graph.ts b/packages/atoms/src/utils/graph.ts index bf1ee9e2..37b97ac6 100644 --- a/packages/atoms/src/utils/graph.ts +++ b/packages/atoms/src/utils/graph.ts @@ -189,7 +189,7 @@ export const scheduleDependents = ( * When a node's refCount hits 0, schedule destruction of that node. */ export const scheduleNodeDestruction = (node: GraphNode) => - node.o.size || node.l !== 'Active' || node.K || node.m() + node.o.size || node.l !== 'Active' || node.m() export const setNodeStatus = (node: GraphNode, newStatus: LifecycleStatus) => { const oldStatus = node.l From 63f06c3622584daa29be8428f581ea9b82045d84 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Tue, 21 Jan 2025 11:12:46 -0500 Subject: [PATCH 7/8] fix SignalMap type generic --- packages/atoms/src/classes/MappedSignal.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/atoms/src/classes/MappedSignal.ts b/packages/atoms/src/classes/MappedSignal.ts index 681ceb41..068ae34b 100644 --- a/packages/atoms/src/classes/MappedSignal.ts +++ b/packages/atoms/src/classes/MappedSignal.ts @@ -1,5 +1,6 @@ import { Settable } from '@zedux/core' import { + AnyNodeGenerics, AtomGenerics, ExplicitEvents, InternalEvaluationReason, @@ -17,7 +18,7 @@ import { Ecosystem } from './Ecosystem' import { Signal } from './Signal' import { recursivelyMutate, recursivelyProxy } from './proxies' -export type SignalMap = Record +export type SignalMap = Record> export class MappedSignal< G extends Pick & { From 6d1d67b022f3b32a3021c0c80e11af199b3913e5 Mon Sep 17 00:00:00 2001 From: Joshua Claunch Date: Tue, 21 Jan 2025 11:13:22 -0500 Subject: [PATCH 8/8] add tests for `.get` and `.getOnce` --- .../test/integrations/mapped-signals.test.tsx | 25 +++++++++ .../react/test/integrations/signals.test.tsx | 52 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/packages/react/test/integrations/mapped-signals.test.tsx b/packages/react/test/integrations/mapped-signals.test.tsx index f1511b47..05486023 100644 --- a/packages/react/test/integrations/mapped-signals.test.tsx +++ b/packages/react/test/integrations/mapped-signals.test.tsx @@ -3,6 +3,7 @@ import { atom, injectMappedSignal, injectSignal, + ion, StateOf, } from '@zedux/atoms' import { ecosystem } from '../utils/ecosystem' @@ -96,6 +97,30 @@ describe('mapped signals', () => { }, ]) }) + + test('atoms can be used as inner signals', () => { + const atom1 = atom('1', 'a') + + const atom2 = ion('2', ({ getNode }) => { + const node1 = getNode(atom1) + const signalB = injectSignal('b') + + const signal = injectMappedSignal({ + a: node1, + b: signalB, + }) + + return signal + }) + + const node1 = ecosystem.getNode(atom2) + + expect(node1.get()).toEqual({ a: 'a', b: 'b' }) + + ecosystem.getNode(atom1).set('aa') + + expect(node1.get()).toEqual({ a: 'aa', b: 'b' }) + }) }) // const signalA = injectSignal('a', { diff --git a/packages/react/test/integrations/signals.test.tsx b/packages/react/test/integrations/signals.test.tsx index 7c57163b..53ade8ee 100644 --- a/packages/react/test/integrations/signals.test.tsx +++ b/packages/react/test/integrations/signals.test.tsx @@ -5,6 +5,7 @@ import { ChangeEvent, GraphNode, injectSignal, + ion, Signal, StateOf, Transaction, @@ -179,4 +180,55 @@ describe('signals', () => { expect(testInstance.get()).toBe(2) expect(testInstance.getOnce()).toBe(2) }) + + test('calling `.get()` in a reactive context registers graph edges', () => { + const atom1 = atom('1', 'a') + + const atom2 = ion('2', ({ getNode }) => { + const node1 = getNode(atom1) + + const signal = injectSignal('') + signal.set(node1.get() + 'b') + + return signal + }) + + const node1 = ecosystem.getNode(atom1) + const node2 = ecosystem.getNode(atom2) + + expect(node2.get()).toBe('ab') + + node1.set('aa') + + expect(node2.get()).toBe('aab') + + node2.set('c') // essentially a no-op; node2's state is always derived + + expect(node2.get()).toBe('aab') + + node1.set('aaa') + + expect(node2.get()).toBe('aaab') + }) + + test('calling `.getOnce()` in a reactive context does not register graph edges', () => { + const atom1 = atom('1', 'a') + + const atom2 = ion('2', ({ getNode }) => { + const node1 = getNode(atom1) + + const signal = injectSignal('') + signal.set(node1.getOnce() + 'b') + + return signal + }) + + const node2 = ecosystem.getNode(atom2) + + expect(node2.get()).toBe('ab') + + ecosystem.getNode(atom1).set('aa') + + expect(node2.get()).toBe('ab') + }) })