diff --git a/docs/specs/recipe-construction/rollout-plan.md b/docs/specs/recipe-construction/rollout-plan.md index 96d6d34a6..b7c84b572 100644 --- a/docs/specs/recipe-construction/rollout-plan.md +++ b/docs/specs/recipe-construction/rollout-plan.md @@ -24,20 +24,25 @@ - [ ] Make passing the output of the second into the first work. Tricky because we're doing almost opposite expansions on the type. - [ ] Add ability to create a cell without a link yet. - - [ ] Change constructor for RegularCell to make link optional - - [ ] Add .for method to set a cause (within current context) - - [ ] second parameter to make it optional/flexible: - - [ ] ignores the .for if link already exists + - [x] Merge StreamCell into RegularCell and rename RegularCell to CellImpl + - [x] Primarily this means changing `.set` to first read the resolved value + to see whether we have a stream and then use the stream behavior instead + of regular set. + - [x] Change constructor for RegularCell to make link optional + - [x] Add .for method to set a cause (within current context) + - [x] second parameter to make it optional/flexible: + - [x] ignores the .for if link already exists - [ ] adds extension if cause already exists (see tracker below) - - [ ] Make .key work even if there is no cause yet. - - [ ] Add some method to force creation of cause, which errors if in + - [x] Make .key work even if there is no cause yet. + - [x] Add some method to force creation of cause, which errors if in non-handler context and no other information was given (as e.g. deriving nodes, which do have ids, after asking for them -- this walks the graph up until it hits the passed in cells) - - [ ] For now though throw in non-handler context when needing a link and it + - [x] For now though throw in non-handler context when needing a link and it isn't there, e.g. because we need to create a link to the cell (when passed into `anotherCell.set()` for example). We want to encourage .for use in ambiguous cases. +- [x] Add space and event to Frame - [ ] First merge of OpaqueRef and RegularCell - [ ] Add methods that allow linking to node invocations - [ ] `setPreExisting` can be deprecated (used in toOpaqueRef which itself diff --git a/packages/api/index.ts b/packages/api/index.ts index db875a692..e86ec5175 100644 --- a/packages/api/index.ts +++ b/packages/api/index.ts @@ -56,8 +56,14 @@ type IsThisArray = * IAnyCell is an interface that is used by all calls and to which the runner * attaches the internal methods.. */ -// deno-lint-ignore no-empty-interface export interface IAnyCell { + /** + * Set a cause for this cell. Used to create a link when the cell doesn't have + * one yet. + * @param cause - The cause to associate with this cell + * @returns This cell for method chaining + */ + for(cause: unknown): Cell; } /** diff --git a/packages/html/src/render.ts b/packages/html/src/render.ts index 455fce4a9..f473ae646 100644 --- a/packages/html/src/render.ts +++ b/packages/html/src/render.ts @@ -5,7 +5,6 @@ import { convertCellsToLinks, effect, isCell, - isStream, type JSONSchema, UI, useCancelGroup, @@ -269,7 +268,7 @@ const bindProps = ( const setProperty = options.setProp ?? setProp; const [cancel, addCancel] = useCancelGroup(); for (const [propKey, propValue] of Object.entries(props)) { - if (isCell(propValue) || isStream(propValue)) { + if (isCell(propValue)) { // If prop is an event, we need to add an event listener if (isEventProp(propKey)) { const key = cleanEventProp(propKey); diff --git a/packages/runner/src/builder/recipe.ts b/packages/runner/src/builder/recipe.ts index 62b6d6872..c4c8c85f4 100644 --- a/packages/runner/src/builder/recipe.ts +++ b/packages/runner/src/builder/recipe.ts @@ -421,13 +421,17 @@ export function pushFrame(frame?: Frame): Frame { export function pushFrameFromCause( cause: any, unsafe_binding?: UnsafeBinding, + inHandler: boolean = false, ): Frame { const frame = { parent: getTopFrame(), cause, generatedIdCounter: 0, opaqueRefs: new Set(), + // Extract space from unsafe_binding if available and set it on frame + ...(unsafe_binding?.space && { space: unsafe_binding.space }), ...(unsafe_binding ? { unsafe_binding } : {}), + ...(inHandler && { inHandler }), }; frames.push(frame); return frame; diff --git a/packages/runner/src/builder/types.ts b/packages/runner/src/builder/types.ts index 130f9883c..181199349 100644 --- a/packages/runner/src/builder/types.ts +++ b/packages/runner/src/builder/types.ts @@ -260,6 +260,8 @@ export type Frame = { parent?: Frame; cause?: unknown; generatedIdCounter: number; + space?: MemorySpace; + inHandler?: boolean; opaqueRefs: Set>; unsafe_binding?: UnsafeBinding; }; diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index c75550701..3bb6b1afb 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -32,6 +32,7 @@ import { validateAndTransform, } from "./schema.ts"; import { toURI } from "./uri-utils.ts"; +import { createRef } from "./create-ref.ts"; import { type LegacyJSONCellLink, LINK_V1_TAG, @@ -45,6 +46,7 @@ import { createSigilLinkFromParsedLink, findAndInlineDataURILinks, type NormalizedFullLink, + type NormalizedLink, } from "./link-utils.ts"; import type { IExtendedStorageTransaction, @@ -84,6 +86,7 @@ declare module "@commontools/api" { * on Cell in the runner runtime. */ interface IAnyCell { + for(cause: unknown, allowIfSet?: boolean): Cell; asSchema( schema: S, ): Cell>; @@ -164,128 +167,206 @@ export type { MemorySpace } from "@commontools/memory/interface"; export function createCell( runtime: IRuntime, - link: NormalizedFullLink, + link?: NormalizedFullLink, tx?: IExtendedStorageTransaction, - noResolve = false, synced = false, ): Cell { - let { schema, rootSchema } = link; - - // Resolve the path to check whether it's a stream. - const readTx = runtime.readTx(tx); - const resolvedLink = noResolve ? link : resolveLink(readTx, link); - const value = readTx.readValueOrThrow(resolvedLink, { - meta: ignoreReadForScheduling, - }); - - // Use schema from alias if provided and no explicit schema was set - if (!schema && resolvedLink.schema) { - schema = resolvedLink.schema; - rootSchema = resolvedLink.rootSchema || resolvedLink.schema; - } - - if (isStreamValue(value)) { - return new StreamCell( - runtime, - { ...resolvedLink, schema, rootSchema }, - tx, - ) as unknown as Cell; - } else { - return new RegularCell( - runtime, - { ...link, schema, rootSchema }, - tx, - synced, - ) as unknown as Cell; - } + return new CellImpl( + runtime, + tx, + link, // Pass the link directly (or undefined) + synced, + undefined, // No shared causeContainer + ) as unknown as Cell; // Cast to set brand +} + +/** + * Shared container for entity ID and cause information across sibling cells. + * When cells are created via .asSchema(), .withTx(), they share the same + * logical identity (same entity id) but may have different paths or schemas. + * The container stores only the entity reference parts that need to be synchronized. + */ +interface CauseContainer { + // Entity reference - shared across all siblings + id: URI | undefined; + space: MemorySpace | undefined; + // Cause for creating the entity ID + cause: unknown | undefined; } -class StreamCell implements IStreamable { +/** + * CellImpl - Unified cell implementation that handles both regular cells and + * streams. + */ +export class CellImpl implements ICell, IStreamable { + private readOnlyReason: string | undefined; + + // Stream-specific fields private listeners = new Set< (event: AnyCellWrapping) => Cancel | undefined >(); private cleanup: Cancel | undefined; + // Each cell has its own link (space, path, schema) + private _link: NormalizedLink; + + // Shared container for entity ID and cause - siblings share the same instance + private _causeContainer: CauseContainer; + constructor( public readonly runtime: IRuntime, - private readonly link: NormalizedFullLink, - private readonly tx?: IExtendedStorageTransaction, - ) {} - - get schema(): JSONSchema | undefined { - return this.link.schema; + public readonly tx: IExtendedStorageTransaction | undefined, + link?: NormalizedLink, + private synced: boolean = false, + causeContainer?: CauseContainer, + ) { + // Store this cell's own link + this._link = link ?? { path: [] }; + + // Use provided container or create one + // If link has an id, extract it to the container + this._causeContainer = causeContainer ?? { + id: this._link.id, + space: this._link.space, + cause: undefined, + }; } - get rootSchema(): JSONSchema | undefined { - return this.link.rootSchema; + /** + * Get the full link for this cell, ensuring it has id and space. + * This will attempt to create a full link if one doesn't exist and we're in a valid context. + */ + private get link(): NormalizedFullLink { + // Check if we have a full entity ID and space + if (!this.hasFullLink()) { + // Try to ensure we have a full link + this.ensureLink(); + + // If still no full link after ensureLink, throw + if (!this.hasFullLink()) { + throw new Error( + "Cell link could not be created. Use .for() to set a cause before accessing the cell.", + ); + } + } + + // Combine causeContainer id with link's space/path/schema + return this._link as NormalizedFullLink; } - send( - event: AnyCellWrapping, - onCommit?: (tx: IExtendedStorageTransaction) => void, - ): void { - event = convertCellsToLinks(event) as AnyCellWrapping; + /** + * Check if this cell has a full link (with id and space) + */ + private hasFullLink(): boolean { + return this._link.id !== undefined && this._link.space !== undefined; + } - // Use runtime from doc if available - this.runtime.scheduler.queueEvent(this.link, event, undefined, onCommit); + /** + * Set a cause for this cell. This is used to create a link when the cell doesn't have one yet. + * This affects all sibling cells (created via .key(), .asSchema(), .withTx()) since they + * share the same container. + * @param cause - The cause to associate with this cell + * @param allowIfSet - If true, treat as suggestion and silently ignore if cause already set. If false (default), throw error if cause already set. + * @returns This cell for method chaining + */ + for(cause: unknown, allowIfSet?: boolean): Cell { + // If cause or id already exists, either fail or silently ignore based on allowIfSet + if (this._causeContainer.id || this._causeContainer.cause) { + if (allowIfSet) { + // Treat as suggestion - silently ignore + return this as unknown as Cell; + } else { + // Fail by default + throw new Error( + "Cannot set cause: cell already has a cause or link.", + ); + } + } - this.cleanup?.(); - const [cancel, addCancel] = useCancelGroup(); - this.cleanup = cancel; + // Store the cause in the shared container - all siblings will see this + this._causeContainer.cause = cause; - this.listeners.forEach((callback) => addCancel(callback(event))); + return this as unknown as Cell; } - sink(callback: (event: AnyCellWrapping) => Cancel | undefined): Cancel { - this.listeners.add(callback); - return () => this.listeners.delete(callback); - } + /** + * Force creation of a full link for this cell from the stored cause. + * This method populates id if it doesn't exist, using information from: + * - The stored cause (from .for()) + * - The current handler context + * - Derived information from the graph (for deriving nodes) + * + * Updates the shared causeContainer, so all siblings will see the new id. + * + * @throws Error if not in a handler context and no cause was provided + */ + private ensureLink(): void { + // If we already have a full link (id and space) in the container, just copy + // it over to our link. + if (this._causeContainer.id && this._causeContainer.space) { + this._link = { + ...this._link, + id: this._causeContainer.id, + space: this._causeContainer.space, + }; + return; + } - // sync: No-op for streams, but maybe eventually it might mean wait for all - // events to have been processed - sync(): Stream { - return this as unknown as Stream; - } + // Otherwise, let's attempt to derive the id: - getRaw(options?: IReadOptions): Immutable | undefined { - // readValueOrThrow requires JSONValue, while we require T - return this.runtime.readTx(this.tx).readValueOrThrow( - this.link, - options, - ) as Immutable | undefined; - } + const frame = getTopFrame(); - getAsNormalizedFullLink(): NormalizedFullLink { - return this.link; - } + // We must be in a frame context to derive the id. + if (!frame) { + throw new Error( + "Cannot create cell link: no frame context.\n" + + "This typically happens when:\n" + + " - A cell is passed to another cell's .set() method without a link\n" + + " - A cell is used outside of a handler or lift context\n", + ); + } - getAsLink( - options?: { - base?: Cell; - baseSpace?: MemorySpace; - includeSchema?: boolean; - }, - ): SigilLink { - return createSigilLinkFromParsedLink(this.link, options); - } + const space = this._link.space ?? this._causeContainer.space ?? + frame?.space; - withTx(_tx?: IExtendedStorageTransaction): Stream { - return this as unknown as Stream; // No-op for streams - } -} + // We need a space to create a link + if (!space) { + throw new Error( + "Cannot create cell link: space is required.\n" + + "When creating cells without links, you must provide a space in the frame.\n", + ); + } -export class RegularCell implements ICell { - private readOnlyReason: string | undefined; + // Used passed in cause (via .for()), for events fall back to per-frame + // counter. + const cause = this._causeContainer.cause ?? + (frame.inHandler ? { count: frame.generatedIdCounter++ } : undefined); + + if (!cause) { + throw new Error( + "Cannot create cell link: not in a handler context and no cause was provided.\n" + + "This typically happens when:\n" + + " - A cell is passed to another cell's .set() method without a link\n" + + " - A cell is used outside of a handler context\n" + + "Solution: Use .for(cause) to set a cause before using the cell in ambiguous cases.", + ); + } - constructor( - public readonly runtime: IRuntime, - private readonly link: NormalizedFullLink, - public readonly tx: IExtendedStorageTransaction | undefined, - private synced: boolean = false, - ) {} + // Create an entity ID from the cause, including the frame's + const id = toURI(createRef({ frame: cause }, cause)); + + // Populate the id in the shared causeContainer + // All siblings will see this update + this._causeContainer.id = id; + this._causeContainer.space = space; + + // Update this cell's link + this._link = { ...this._link, id, space }; + } get space(): MemorySpace { - return this.link.space; + return this._link.space ?? this._causeContainer.space ?? + getTopFrame()?.space!; } get path(): readonly PropertyKey[] { @@ -293,11 +374,53 @@ export class RegularCell implements ICell { } get schema(): JSONSchema | undefined { - return this.link.schema; + if (this._link.schema) return this._link.schema; + + // If no schema is defined, resolve link and get schema from there (which is + // what .get() would do). + if (this.hasFullLink()) { + const resolvedLink = resolveLink( + this.runtime.readTx(this.tx), + this.link, + "writeRedirect", + ); + return resolvedLink.schema; + } + + return undefined; } get rootSchema(): JSONSchema | undefined { - return this.link.rootSchema; + if (this._link.rootSchema) return this._link.rootSchema; + + // If no root schema is defined, resolve link and get root schema from there + // (which is what .get() would do). + if (this.hasFullLink()) { + const resolvedLink = resolveLink( + this.runtime.readTx(this.tx), + this.link, + "writeRedirect", + ); + return resolvedLink.rootSchema; + } + + return undefined; + } + + /** + * Check if this cell contains a stream value + */ + private isStream(resolvedToValueLink?: NormalizedFullLink): boolean { + const tx = this.runtime.readTx(this.tx); + + if (!resolvedToValueLink) { + resolvedToValueLink = resolveLink(tx, this.link); + } + + const value = tx.readValueOrThrow(resolvedToValueLink, { + meta: ignoreReadForScheduling, + }); + return isStreamValue(value); } get(): Readonly { @@ -309,6 +432,33 @@ export class RegularCell implements ICell { newValue: AnyCellWrapping | T, onCommit?: (tx: IExtendedStorageTransaction) => void, ): void { + const resolvedToValueLink = resolveLink( + this.runtime.readTx(this.tx), + this.link, + ); + + // Check if we're dealing with a stream + if (this.isStream(resolvedToValueLink)) { + // Stream behavior + const event = convertCellsToLinks(newValue) as AnyCellWrapping; + + // Trigger on fully resolved link + this.runtime.scheduler.queueEvent( + resolvedToValueLink, + event, + undefined, + onCommit, + ); + + this.cleanup?.(); + const [cancel, addCancel] = useCancelGroup(); + this.cleanup = cancel; + + this.listeners.forEach((callback) => addCancel(callback(event))); + return; + } + + // Regular cell behavior if (!this.tx) throw new Error("Transaction required for set"); // No await for the sync, just kicking this off, so we have the data to @@ -334,10 +484,10 @@ export class RegularCell implements ICell { } send( - newValue: AnyCellWrapping | T, + event: AnyCellWrapping, onCommit?: (tx: IExtendedStorageTransaction) => void, ): void { - this.set(newValue, onCommit); + this.set(event, onCommit); } update | AnyCellWrapping>)>( @@ -448,21 +598,29 @@ export class RegularCell implements ICell { key( valueKey: K, ): KeyResultType { - const childSchema = this.runtime.cfc.getSchemaAtPath( - this.schema, - [valueKey.toString()], - this.rootSchema, - ); - return createCell( + // Get child schema if we have one + const childSchema = this._link.schema + ? this.runtime.cfc.getSchemaAtPath( + this._link.schema, + [valueKey.toString()], + this._link.rootSchema, + ) + : undefined; + + // Create a child link with extended path + const childLink: NormalizedLink = { + ...this._link, + path: [...this._link.path, valueKey.toString()] as string[], + schema: childSchema, + rootSchema: childSchema ? this._link.rootSchema : undefined, + }; + + return new CellImpl( this.runtime, - { - ...this.link, - path: [...this.path, valueKey.toString()] as string[], - schema: childSchema, - }, this.tx, - false, + childLink, this.synced, + this._causeContainer, ) as unknown as KeyResultType; } @@ -473,37 +631,64 @@ export class RegularCell implements ICell { schema?: JSONSchema, ): Cell; asSchema(schema?: JSONSchema): Cell { - return new RegularCell( + // asSchema creates a sibling with same identity but different schema + // Create a new link with modified schema + const siblingLink: NormalizedLink = { + ...this._link, + schema: schema, + rootSchema: schema, + }; + + return new CellImpl( this.runtime, - { ...this.link, schema: schema, rootSchema: schema }, this.tx, - false, // Reset synced flag, since schmema is changing + siblingLink, + false, // Reset synced flag, since schema is changing + this._causeContainer, // Share the causeContainer with siblings ) as unknown as Cell; } withTx(newTx?: IExtendedStorageTransaction): Cell { - return new RegularCell( + // withTx creates a sibling with same identity but different transaction + // Share the causeContainer so .for() calls propagate + return new CellImpl( this.runtime, - this.link, newTx, + this._link, // Use the same link this.synced, + this._causeContainer, // Share the causeContainer with siblings ) as unknown as Cell; } sink(callback: (value: Readonly) => Cancel | undefined): Cancel { - if (!this.synced) this.sync(); // No await, just kicking this off - return subscribeToReferencedDocs(callback, this.runtime, this.link); + // Check if this is a stream + if (this.isStream()) { + // Stream behavior: add listener + this.listeners.add( + callback as (event: AnyCellWrapping) => Cancel | undefined, + ); + return () => + this.listeners.delete( + callback as (event: AnyCellWrapping) => Cancel | undefined, + ); + } else { + // Regular cell behavior: subscribe to changes + if (!this.synced) this.sync(); // No await, just kicking this off + return subscribeToReferencedDocs(callback, this.runtime, this.link); + } } sync(): Promise> | Cell { this.synced = true; - if (this.link.id.startsWith("data:")) return this as unknown as Cell; + if (this.link.id.startsWith("data:")) { + return this as unknown as Cell; + } return this.runtime.storageManager.syncCell(this as unknown as Cell); } resolveAsCell(): Cell { const link = resolveLink(this.runtime.readTx(this.tx), this.link); - return createCell(this.runtime, link, this.tx, true, this.synced); + return createCell(this.runtime, link, this.tx, this.synced); } getAsQueryResult( @@ -831,7 +1016,7 @@ export function convertCellsToLinks( if (isQueryResultForDereferencing(value)) { value = getCellOrThrow(value).getAsLink(); - } else if (isCell(value) || isStream(value)) { + } else if (isCell(value)) { value = value.getAsLink(); } else if (isRecord(value) || typeof value === "function") { // Only add here, otherwise they are literals or already cells: @@ -873,7 +1058,7 @@ export function convertCellsToLinks( * @returns {boolean} */ export function isCell(value: any): value is Cell { - return value instanceof RegularCell; + return value instanceof CellImpl; } /** @@ -883,7 +1068,7 @@ export function isCell(value: any): value is Cell { * @returns {boolean} */ export function isAnyCell(value: any): value is AnyCell { - return value instanceof RegularCell || value instanceof StreamCell; + return value instanceof CellImpl; } /** @@ -892,7 +1077,7 @@ export function isAnyCell(value: any): value is AnyCell { * @returns True if the value is a Stream */ export function isStream(value: any): value is Stream { - return value instanceof StreamCell; + return (value instanceof CellImpl && (value as any).isStream?.()); } export type DeepKeyLookup = Path extends [] ? T diff --git a/packages/runner/src/data-updating.ts b/packages/runner/src/data-updating.ts index ddeb76014..f14115236 100644 --- a/packages/runner/src/data-updating.ts +++ b/packages/runner/src/data-updating.ts @@ -2,7 +2,7 @@ import { isRecord } from "@commontools/utils/types"; import { getLogger } from "@commontools/utils/logger"; import { ID, ID_FIELD, type JSONSchema } from "./builder/types.ts"; import { createRef } from "./create-ref.ts"; -import { isCell, isStream, RegularCell } from "./cell.ts"; +import { CellImpl, isCell } from "./cell.ts"; import { resolveLink } from "./link-resolution.ts"; import { areLinksSame, @@ -118,7 +118,7 @@ export function normalizeAndDiff( diffLogger.debug(() => `[SEEN_CHECK] Already seen object at path=${pathStr}, converting to cell` ); - newValue = new RegularCell(runtime, seen.get(newValue)!, tx); + newValue = new CellImpl(runtime, tx, seen.get(newValue)!); } // ID_FIELD redirects to an existing field and we do something like DOM @@ -197,12 +197,13 @@ export function normalizeAndDiff( newValue = sigilLink; } - // Track whether this link originates from a Cell value (either a cycle we wrapped - // into a RegularCell above, or a user-supplied Cell). For Cell-origin links we - // preserve the link (do NOT collapse). For links created via query-result - // dereferencing (non-Cell), we may collapse immediate-parent self-links. + // Track whether this link originates from a Cell value (either a cycle we + // wrapped into a CellImpl above, or a user-supplied Cell). For Cell-origin + // links we preserve the link (do NOT collapse). For links created via + // query-result dereferencing (non-Cell), we may collapse immediate-parent + // self-links. let linkOriginFromCell = false; - if (isCell(newValue) || isStream(newValue)) { + if (isCell(newValue)) { diffLogger.debug(() => `[BRANCH_CELL] Converting cell to link at path=${pathStr}` ); diff --git a/packages/runner/src/link-utils.ts b/packages/runner/src/link-utils.ts index 484909fcc..55044924b 100644 --- a/packages/runner/src/link-utils.ts +++ b/packages/runner/src/link-utils.ts @@ -4,7 +4,6 @@ import { type Cell, isAnyCell, isCell, - isStream, type MemorySpace, type Stream, } from "./cell.ts"; @@ -147,7 +146,6 @@ export function isLink( isQueryResultForDereferencing(value) || isAnyCellLink(value) || isCell(value) || - isStream(value) || (isRecord(value) && "/" in value && typeof value["/"] === "string") // EntityId format ); } @@ -234,7 +232,7 @@ export function parseLink( // see userland "/". if (isQueryResultForDereferencing(value)) value = getCellOrThrow(value); - if (isCell(value) || isStream(value)) return value.getAsNormalizedFullLink(); + if (isCell(value)) return value.getAsNormalizedFullLink(); // Handle new sigil format if (isSigilLink(value)) { diff --git a/packages/runner/src/query-result-proxy.ts b/packages/runner/src/query-result-proxy.ts index 18ac80926..cad156b42 100644 --- a/packages/runner/src/query-result-proxy.ts +++ b/packages/runner/src/query-result-proxy.ts @@ -120,7 +120,7 @@ export function createQueryResultProxy( if (typeof prop === "symbol") { if (prop === toCell) { - return () => createCell(runtime, link, tx, true); + return () => createCell(runtime, link, tx); } else if (prop === toOpaqueRef) { return () => makeOpaqueRef(link); } else if (prop === Symbol.iterator && Array.isArray(target)) { diff --git a/packages/runner/src/runner.ts b/packages/runner/src/runner.ts index d618fa8b0..b6f3ae89f 100644 --- a/packages/runner/src/runner.ts +++ b/packages/runner/src/runner.ts @@ -969,13 +969,17 @@ export class Runner implements IRunner { tx, ); - const frame = pushFrameFromCause(cause, { - recipe, - materialize: (path: readonly PropertyKey[]) => - processCell.getAsQueryResult(path), - space: processCell.space, - tx, - }); + const frame = pushFrameFromCause( + cause, + { + recipe, + materialize: (path: readonly PropertyKey[]) => + processCell.getAsQueryResult(path), + space: processCell.space, + tx, + }, + true, // Set the event on the frame + ); const argument = module.argumentSchema ? inputsCell.asSchema(module.argumentSchema).get() diff --git a/packages/runner/src/schema.ts b/packages/runner/src/schema.ts index 9e7652adc..cc8e1920a 100644 --- a/packages/runner/src/schema.ts +++ b/packages/runner/src/schema.ts @@ -4,7 +4,7 @@ import { isObject, isRecord } from "@commontools/utils/types"; import { JSONSchemaMutable } from "@commontools/runner"; import { ContextualFlowControl } from "./cfc.ts"; import { type JSONSchema, type JSONValue } from "./builder/types.ts"; -import { createCell, isCell, isStream } from "./cell.ts"; +import { createCell, isCell } from "./cell.ts"; import { readMaybeLink, resolveLink } from "./link-resolution.ts"; import { type IExtendedStorageTransaction } from "./storage/interface.ts"; import { type IRuntime } from "./runtime.ts"; @@ -311,8 +311,7 @@ function annotateWithBackToCellSymbols( tx: IExtendedStorageTransaction | undefined, ) { if ( - isRecord(value) && !isCell(value) && !isStream(value) && - !isQueryResultForDereferencing(value) + isRecord(value) && !isCell(value) && !isQueryResultForDereferencing(value) ) { // Non-enumerable, so that {...obj} won't copy these symbols Object.defineProperty(value, toCell, { diff --git a/packages/runner/test/cell-optional-link.test.ts b/packages/runner/test/cell-optional-link.test.ts new file mode 100644 index 000000000..006b81901 --- /dev/null +++ b/packages/runner/test/cell-optional-link.test.ts @@ -0,0 +1,449 @@ +import { afterEach, beforeEach, describe, it } from "@std/testing/bdd"; +import { expect } from "@std/expect"; +import "@commontools/utils/equal-ignoring-symbols"; + +import { Identity } from "@commontools/identity"; +import { StorageManager } from "@commontools/runner/storage/cache.deno"; +import { CellImpl } from "../src/cell.ts"; +import { Runtime } from "../src/runtime.ts"; +import { popFrame, pushFrame } from "../src/builder/recipe.ts"; +import { type IExtendedStorageTransaction } from "../src/storage/interface.ts"; + +const signer = await Identity.fromPassphrase("test operator optional link"); +const space = signer.did(); + +describe("Cell with Optional Link", () => { + let runtime: Runtime; + let storageManager: ReturnType; + let tx: IExtendedStorageTransaction; + + beforeEach(() => { + storageManager = StorageManager.emulate({ as: signer }); + + runtime = new Runtime({ + apiUrl: new URL(import.meta.url), + storageManager, + }); + + tx = runtime.edit(); + }); + + afterEach(async () => { + await tx.commit(); + await runtime?.dispose(); + await storageManager?.close(); + }); + + describe(".for() method", () => { + it("should allow setting a cause on a cell", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + const cause = { type: "test-cause" }; + + const result = cell.for(cause); + + // Should return the cell for chaining + expect(result).toBe(cell); + }); + + it("should throw error if link already exists (default behavior)", () => { + const existingCell = runtime.getCell( + space, + "test-cell-with-link", + undefined, + tx, + ); + existingCell.set(42); + + const cause = { type: "test-cause" }; + + // Should throw by default + expect(() => existingCell.for(cause)).toThrow( + "Cannot set cause: cell already has a cause or link", + ); + + // Cell should still work normally + expect(existingCell.get()).toBe(42); + }); + + it("should allow allowIfSet option to treat as suggestion", () => { + const existingCell = runtime.getCell( + space, + "test-cell-allow", + undefined, + tx, + ); + existingCell.set(42); + + const cause = { type: "test-cause" }; + const result = existingCell.for(cause, true); // allowIfSet = true + + // Should return the cell for chaining without throwing + expect(result).toBe(existingCell); + // Cell should still work normally + expect(existingCell.get()).toBe(42); + }); + + it("should support method chaining", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + const cause = { type: "test-cause" }; + + // Chain .for() and .key() + const result = cell.for(cause).key("foo"); + + // Should return a cell (even without link) + expect(result).toBeDefined(); + }); + }); + + describe(".key() without link", () => { + it("should allow chaining .key() on cell without link", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + const cause = { type: "test-cause" }; + + const child = cell.for(cause).key("foo"); + + // Should create a child cell without throwing + expect(child).toBeDefined(); + }); + + it("should allow nested .key() calls without link", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + const cause = { type: "test-cause" }; + + const nestedChild = cell.for(cause).key("foo").key("bar").key("baz"); + + // Should create nested cells without throwing + expect(nestedChild).toBeDefined(); + }); + + it("should inherit cause from parent when using .key()", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + const cause = { type: "test-cause" }; + + cell.for(cause); + const child = cell.key("foo"); + + // Child should inherit the cause + // We can't directly test private fields, but we can verify it doesn't throw + expect(child).toBeDefined(); + }); + }); + + describe("ensureLink() error handling", () => { + it("should throw error when accessing cell without frame context", () => { + const cell = new CellImpl(runtime, tx); + + // Trying to get the cell value without a link should throw + expect(() => cell.get()).toThrow( + "Cannot create cell link: no frame context.", + ); + }); + + it("should take space from link if no id provided", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + + // Even without id provided, take space from link. + expect(cell.space).toEqual(space); + }); + + it("should take space from frame if no id provided", () => { + const cell = new CellImpl(runtime, tx); + + pushFrame({ + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + // Take space from frame. + expect(cell.space).toEqual(space); + + popFrame(); + }); + + it("should throw error when accessing path without cause", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + + // Without a cause, should throw + expect(() => cell.path).toThrow( + "Cannot create cell link: no frame context.", + ); + + pushFrame({ + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + expect(() => cell.path).toThrow( + "Cannot create cell link: not in a handler context and no cause was provided", + ); + + popFrame(); + }); + + it("should create link when accessing cell with cause and space", () => { + const cell = new CellImpl(runtime, tx); + const cause = { type: "test-cause" }; + + pushFrame({ + cause: { type: "lift-cause" }, + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + cell.for(cause); + + // With both cause and space, link should be created automatically + expect(cell.space).toBe(space); + expect(cell.path).toEqual([]); + + popFrame(); + }); + + it("should create link using frame cause when in handler context", () => { + const cell = new CellImpl(runtime, tx); + + // Create a frame with a cause + pushFrame({ + cause: { type: "handler-cause" }, + space, + inHandler: true, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + try { + // With frame cause and space, link should be created automatically + expect(cell.space).toBe(space); + expect(cell.path).toEqual([]); + } finally { + popFrame(); + } + }); + }); + + describe("CellImpl backward compatibility", () => { + it("should work with existing link-based cells", () => { + const cell = runtime.getCell<{ value: number; nested?: number }>( + space, + "backward-compat-test", + undefined, + tx, + ); + + cell.set({ value: 100 }); + expect(cell.get()).toEqual({ value: 100 }); + + const child = cell.key("nested"); + child.set(200); + expect(child.get()).toBe(200); + }); + + it("should handle .for() on cells with existing links gracefully", () => { + const cell = runtime.getCell( + space, + "for-on-existing-link", + undefined, + tx, + ); + + cell.set(42); + + // Should not affect the cell + cell.for({ type: "ignored-cause" }, true); + + expect(cell.get()).toBe(42); + }); + }); + + describe("Error messages", () => { + it("should provide helpful error when cell is passed to .set() without link", () => { + const targetCell = runtime.getCell( + space, + "target-cell", + undefined, + tx, + ); + const cellWithoutLink = new CellImpl( + runtime, + tx, + { path: [], space }, + false, + ); + + // When trying to convert cellWithoutLink to a link, it should throw + expect(() => targetCell.set(cellWithoutLink)).toThrow(); + }); + + it("should suggest using .for() in error messages", () => { + const cell = new CellImpl(runtime, tx, { path: [], space }, false); + + pushFrame({ + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + try { + cell.get(); + expect(true).toBe(false); // Should not reach here + } catch (error: any) { + expect(error.message).toContain(".for(cause)"); + expect(error.message).toContain("cause"); + } finally { + popFrame(); + } + }); + }); + + describe("Stream cells with optional links", () => { + it("should handle streams created through CellImpl", () => { + // Create a stream cell + const streamCell = runtime.getCell( + space, + "test-stream", + undefined, + tx, + ); + + // Set it to a stream value + streamCell.setRaw({ $stream: true }); + + const receivedEvents: any[] = []; + streamCell.sink((event: any) => { + receivedEvents.push(event); + }); + + streamCell.send({ type: "test-event" }); + + expect(receivedEvents.length).toBe(1); + expect(receivedEvents[0]).toEqual({ type: "test-event" }); + }); + }); + + describe("Sibling cells share link creation", () => { + it("should share cause across siblings created with .asSchema()", () => { + const cell1 = new CellImpl(runtime, tx); + const cell2 = cell1.asSchema({ type: "object" }); + + const cause = { type: "shared-cause" }; + + pushFrame({ + cause: { type: "frame-cause" }, + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + // Set cause on cell2 + cell2.for(cause); + + try { + // Both should now have the same entity ID + const id1 = cell1.entityId; + const id2 = cell2.entityId; + + expect(id1).toBeDefined(); + expect(id2).toBeDefined(); + expect(id1).toEqual(id2); + } finally { + popFrame(); + } + }); + + it("should share cause across siblings created with .withTx()", () => { + const cell1 = new CellImpl(runtime, tx); + const cell2 = cell1.withTx(tx); + + const cause = { type: "shared-cause" }; + + pushFrame({ + cause: { type: "frame-cause" }, + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + // Set cause on cell1 + cell1.for(cause); + + try { + // Both should now have the same entity ID + const id1 = cell1.entityId; + const id2 = cell2.entityId; + + expect(id1).toBeDefined(); + expect(id2).toBeDefined(); + expect(id1).toEqual(id2); + } finally { + popFrame(); + } + }); + + it("should share link creation across siblings", () => { + const cell1 = new CellImpl(runtime, tx); + const cell2 = cell1.asSchema({ type: "object" }); + const cell3 = cell2.withTx(tx); + + const cause = { type: "shared-cause" }; + + pushFrame({ + cause: { type: "frame-cause" }, + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + // Set cause on cell3 + cell3.for(cause); + + try { + // All three should have the same entity ID and space + const id1 = cell1.entityId; + const id2 = cell2.entityId; + const id3 = cell3.entityId; + + expect(id1).toEqual(id2); + expect(id2).toEqual(id3); + + // Accessing space on any sibling should work + expect(cell1.space).toBe(space); + expect(cell2.space).toBe(space); + expect(cell3.space).toBe(space); + } finally { + popFrame(); + } + }); + + it("should NOT share cause with children created via .key()", () => { + const parent = new CellImpl(runtime, tx); + const child = parent.key("child"); + + const cause = { type: "parent-cause" }; + + pushFrame({ + cause: { type: "frame-cause" }, + space, + generatedIdCounter: 0, + opaqueRefs: new Set(), + }); + + // Set cause on parent + parent.for(cause); + + try { + // Parent and child should have DIFFERENT entity IDs + const parentId = parent.entityId; + const childId = child.entityId; + + expect(parentId).toBeDefined(); + expect(childId).toBeDefined(); + expect(parentId).not.toBe(childId); + } finally { + popFrame(); + } + }); + }); +}); diff --git a/packages/runner/test/cell.test.ts b/packages/runner/test/cell.test.ts index 5ecae1d4b..02c45371b 100644 --- a/packages/runner/test/cell.test.ts +++ b/packages/runner/test/cell.test.ts @@ -894,9 +894,6 @@ describe("Proxy", () => { const streamCell = c.key("stream"); expect(streamCell).toHaveProperty("send"); - expect(streamCell).not.toHaveProperty("get"); - expect(streamCell).not.toHaveProperty("set"); - expect(streamCell).not.toHaveProperty("key"); let lastEventSeen: any = null; let eventCount = 0; diff --git a/packages/runner/test/schema.test.ts b/packages/runner/test/schema.test.ts index b6f118861..40fc748aa 100644 --- a/packages/runner/test/schema.test.ts +++ b/packages/runner/test/schema.test.ts @@ -2278,9 +2278,6 @@ describe("Schema Support", () => { expect(value.name).toBe("Test Doc"); expect(isStream(value.events)).toBe(true); - - // Verify it's a stream, i.e. no get functio - expect((value as any).events.get).toBe(undefined); }); it("should handle nested streams in objects", () => {