From e018b1adb2b5384fa8f068c551cbe215fb1e39b1 Mon Sep 17 00:00:00 2001 From: Tony Espinoza <86493411+tonyespinoza1@users.noreply.github.com> Date: Wed, 26 Nov 2025 17:26:59 -0800 Subject: [PATCH 1/2] docs: Add key learnings for pattern development (#2171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents important patterns discovered during development: - Getting Cell access for mutation (Cell<> wrapper in Input) - Exposing actions via handlers for cross-charm calling - Consuming actions via Streams 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Tony Espinoza Co-authored-by: Claude --- docs/common/wip/KeyLearnings.md | 152 ++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 docs/common/wip/KeyLearnings.md diff --git a/docs/common/wip/KeyLearnings.md b/docs/common/wip/KeyLearnings.md new file mode 100644 index 0000000000..f63c2ae263 --- /dev/null +++ b/docs/common/wip/KeyLearnings.md @@ -0,0 +1,152 @@ +# Key Learnings for Pattern Development + +This document captures important learnings discovered during pattern development. These will be reviewed and edited for prompt engineering purposes. + +--- + +## Getting Cell Access for Mutation + +**Goal:** Mutate state from inline event handlers in JSX. + +**Solution:** Declare `Cell<>` around the field in your Input interface. This gives you access to `.set()`, `.get()`, `.update()`, etc. + +```typescript +// ✅ CORRECT: Declare Cell<> in Input to get mutation access +interface Input { + count: Cell>; // Has .set(), .get(), .update() +} + +export default recipe(({ count }) => { + return { + [UI]: ( +
+
{count}
+ {/* Inline mutation works! */} + count.set(count.get() + 1)}> + +1 + +
+ ), + count, + }; +}); +``` + +**Without Cell<>** - read-only (no mutation methods): + +```typescript +// ❌ Without Cell<> wrapper - read-only +interface Input { + count: Default; // No .set(), .get() methods +} + +// This will fail: + count.set(count.get() + 1)}> // ERROR! +``` + +**Error you'll see if you forget Cell<>:** +``` +TypeError: count.set is not a function +``` + +**Key insight:** `Cell<>` in the Input interface indicates write intent. It tells the runtime to provide a Cell reference instead of an opaque ref. Everything is reactive by default - `Cell<>` only signals that you'll call mutation methods. + +--- + +## Exposing Actions via Handlers (for Cross-Charm Calling) + +**Goal:** Make a charm's actions callable by **other linked charms** (not just within the same charm). + +**When to use `handler()` vs `Cell<>` in Input:** + +| Need | Solution | +|------|----------| +| Mutate state within same charm | Declare `Cell` in Input (see above) | +| Expose action for OTHER linked charms | Use `handler()` + return as Stream | + +**Pattern for cross-charm actions:** +1. Define handlers at module level using `handler()` +2. Return handlers bound to state in the recipe's return object +3. Cast to `Stream` in the output interface + +```typescript +import { Cell, Default, handler, NAME, recipe, Stream, UI } from "commontools"; + +interface Output { + count: number; + increment: Stream; // Action exposed as Stream + decrement: Stream; + setCount: Stream<{ value: number }>; +} + +// 1. Define handlers +const increment = handler }>( + (_event, { count }) => { + count.set(count.get() + 1); + }, +); + +const setCount = handler<{ value: number }, { count: Cell }>( + (event, { count }) => { + count.set(event?.value ?? 0); + }, +); + +export default recipe("Action Counter", ({ count }) => { + return { + [NAME]: "Action Counter", + [UI]: ( +
+ +1 +
+ ), + count, + // 2. Return handlers bound to state, cast to Stream + increment: increment({ count }) as unknown as Stream, + setCount: setCount({ count }) as unknown as Stream<{ value: number }>, + }; +}); +``` + +**Key insight:** Handlers become callable action streams when returned in the output. The `as unknown as Stream` cast tells TypeScript that this bound handler will be used as a stream by consumers. + +--- + +## Consuming Actions via Streams + +**Goal:** Call actions exposed by a linked charm. + +### Recommended: Whole Charm Linking (Declare Only What You Need) + +Link the entire source charm to a single input field. In your type, **only declare the fields you actually use** - you don't need to mirror the entire source charm's interface. + +```typescript +// Only declare what you need - not the full CounterCharm interface +interface LinkedCounter { + count: number; // Only if you need to display it + increment: Stream; // Only the actions you'll call + setCount: Stream<{ value: number }>; +} + +interface Input { + counter: Default; +} + +// Link with: ct charm link /counter + +// Usage in JSX - inline .send(): + counter.increment.send()}>+1 + counter.setCount.send({ value: 0 })}>Reset + +// Can also read data: +
Count: {counter.count}
+``` + +**Advantages:** +- Single link gives access to data and actions +- Type documents your actual dependencies +- No need to maintain a full mirror of the source charm's interface + +**Key insight:** `Stream` types have only `.send()` - no `.get()` or `.set()`. They're write-only channels for triggering actions. + +--- From 03c6e190e984fdc47cb11930c303bd296a3f62c9 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Wed, 26 Nov 2025 17:51:47 -0800 Subject: [PATCH 2/2] fix(runner): fix instantiating patterns and saving them via Cell.push in a handler (#2172) * adding test reproducing the bug: pushing a newly created charm onto an array in a handler doesn't result in the runner charm being added. * fix: recipes from cells that are returned via recipe are now being instantiated: - Runner: When outputBinding is a direct link, use the existing cell instead of creating a new one and binding back. This ensures that when a recipe outputs to an existing cell, it writes directly to it. - Recipe: Add check to ensure `outputs` is not a Cell before iterating in `factoryFromRecipe`. - Cell: Fix `createRef` to use `this._frame.cause` for consistent ID generation. - Cell: Change `export` to use `getAsLink` for external cells, fixing potential serialization issues. - Tests: Update `recipes.test.ts` to verify `Cell.push` behavior with `asCell: true` and proper transaction handling. * Ensure recipes created in handlers are collected even if not returned - Track created opaque refs in the current frame in `opaqueRefWithCell`. - Update `Runner` to check for `frame.opaqueRefs` existence when deciding whether to process a result recipe in event handlers and actions. - This allows patterns created and attached via side effects (e.g., `list.push(newPattern)`) to be correctly instantiated without needing to be returned from the handler. - Updated `recipes.test.ts` to verify this behavior by removing the explicit return. --- packages/runner/src/builder/opaque-ref.ts | 2 + packages/runner/src/builder/recipe.ts | 6 +- packages/runner/src/cell.ts | 4 +- packages/runner/src/runner.ts | 59 ++++++++---- packages/runner/test/recipes.test.ts | 107 +++++++++++++++++++++- 5 files changed, 152 insertions(+), 26 deletions(-) diff --git a/packages/runner/src/builder/opaque-ref.ts b/packages/runner/src/builder/opaque-ref.ts index 02387c9991..897111de81 100644 --- a/packages/runner/src/builder/opaque-ref.ts +++ b/packages/runner/src/builder/opaque-ref.ts @@ -57,6 +57,8 @@ function opaqueRefWithCell( cell.setInitialValue(value as T); } + frame.opaqueRefs.add(cell); + // Use the cell's built-in method to get a proxied OpaqueRef return cell.getAsOpaqueRefProxy(); } diff --git a/packages/runner/src/builder/recipe.ts b/packages/runner/src/builder/recipe.ts index 05906585dc..83afb529cf 100644 --- a/packages/runner/src/builder/recipe.ts +++ b/packages/runner/src/builder/recipe.ts @@ -229,7 +229,7 @@ function factoryFromRecipe( }); // First from results - if (isRecord(outputs)) { + if (isRecord(outputs) && !isCell(outputs)) { Object.entries(outputs).forEach(([key, value]: [string, unknown]) => { if (isCell(value)) { const exported = value.export(); @@ -263,8 +263,8 @@ function factoryFromRecipe( }); }); - // [For unsafe bindings] Also collect otherwise disconnected cells and nodes, - // since they might only be mentioned via a code closure in a lifted function. + // Also collect otherwise disconnected cells and nodes, e.g. those that are + // assigned to cells via .set or .push and aren't otherwise connected. getTopFrame()?.opaqueRefs.forEach((ref) => collectCellsAndNodes(ref)); // Then assign paths on the recipe cell for all cells. For now we just assign diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index 51f973c8c0..d5f95c0c3a 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -444,7 +444,7 @@ export class CellImpl implements ICell, IStreamable { } // Create an entity ID from the cause, including the frame's - const id = toURI(createRef({ frame: cause }, cause)); + const id = toURI(createRef({ frame: cause }, this._frame.cause)); // Populate the id in the shared causeContainer // All siblings will see this update @@ -1074,7 +1074,7 @@ export class CellImpl implements ICell, IStreamable { : this._initialValue, name: this._causeContainer.cause as string | undefined, external: this._link.id - ? this.getAsWriteRedirectLink({ + ? this.getAsLink({ baseSpace: this._frame.space, includeSchema: true, }) diff --git a/packages/runner/src/runner.ts b/packages/runner/src/runner.ts index ac48690556..baccd13325 100644 --- a/packages/runner/src/runner.ts +++ b/packages/runner/src/runner.ts @@ -37,6 +37,7 @@ import { areNormalizedLinksSame, createSigilLinkFromParsedLink, isLink, + isSigilLink, isWriteRedirectLink, type NormalizedFullLink, parseLink, @@ -1003,7 +1004,7 @@ export class Runner implements IRunner { const result = fn(argument); const postRun = (result: any) => { - if (containsOpaqueRef(result)) { + if (containsOpaqueRef(result) || frame.opaqueRefs.size > 0) { const resultRecipe = recipeFromFrame( "event handler result", undefined, @@ -1094,7 +1095,7 @@ export class Runner implements IRunner { const result = fn(argument); const postRun = (result: any) => { - if (containsOpaqueRef(result)) { + if (containsOpaqueRef(result) || frame.opaqueRefs.size > 0) { const resultRecipe = recipeFromFrame( "action result", undefined, @@ -1276,24 +1277,44 @@ export class Runner implements IRunner { processCell, ); const inputs = unwrapOneLevelAndBindtoDoc(inputBindings, processCell); - const resultCell = this.runtime.getCell( - processCell.space, - { - recipe: module.implementation, - parent: processCell.entityId, - inputBindings, - outputBindings, - }, - undefined, - tx, - ); + + // If output bindings is a link to a non-redirect cell, + // use that instead of creating a new cell. + let resultCell; + let sendToBindings: boolean; + if (isSigilLink(outputBindings) && !isWriteRedirectLink(outputBindings)) { + resultCell = this.runtime.getCellFromLink( + parseLink(outputBindings, processCell), + recipeImpl.resultSchema, + tx, + ); + sendToBindings = false; + } else { + resultCell = this.runtime.getCell( + processCell.space, + { + recipe: module.implementation, + parent: processCell.entityId, + inputBindings, + outputBindings, + }, + recipeImpl.resultSchema, + tx, + ); + sendToBindings = true; + } + this.run(tx, recipeImpl, inputs, resultCell); - sendValueToBinding( - tx, - processCell, - outputBindings, - resultCell.getAsLink({ base: processCell }), - ); + + if (sendToBindings) { + sendValueToBinding( + tx, + processCell, + outputBindings, + resultCell.getAsLink({ base: processCell }), + ); + } + // TODO(seefeld): Make sure to not cancel after a recipe is elevated to a // charm, e.g. via navigateTo. Nothing is cancelling right now, so leaving // this as TODO. diff --git a/packages/runner/test/recipes.test.ts b/packages/runner/test/recipes.test.ts index d7d68e1ecb..9a88784c61 100644 --- a/packages/runner/test/recipes.test.ts +++ b/packages/runner/test/recipes.test.ts @@ -4,11 +4,15 @@ import "@commontools/utils/equal-ignoring-symbols"; import { Identity } from "@commontools/identity"; import { StorageManager } from "@commontools/runner/storage/cache.deno"; -import { type Cell, type JSONSchema } from "../src/builder/types.ts"; +import { + type Cell, + type JSONSchema, + type Schema, +} from "../src/builder/types.ts"; import { createBuilder } from "../src/builder/factory.ts"; import { Runtime } from "../src/runtime.ts"; import { type ErrorWithContext } from "../src/scheduler.ts"; -import { isCell } from "../src/cell.ts"; +import { isCell, isStream } from "../src/cell.ts"; import { resolveLink } from "../src/link-resolution.ts"; import { isAnyCellLink, parseLink } from "../src/link-utils.ts"; import { type IExtendedStorageTransaction } from "../src/storage/interface.ts"; @@ -23,6 +27,7 @@ describe("Recipe Runner", () => { let lift: ReturnType["commontools"]["lift"]; let derive: ReturnType["commontools"]["derive"]; let recipe: ReturnType["commontools"]["recipe"]; + let pattern: ReturnType["commontools"]["pattern"]; let Cell: ReturnType["commontools"]["Cell"]; let handler: ReturnType["commontools"]["handler"]; let byRef: ReturnType["commontools"]["byRef"]; @@ -45,6 +50,7 @@ describe("Recipe Runner", () => { lift, derive, recipe, + pattern, Cell, handler, byRef, @@ -1340,4 +1346,101 @@ describe("Recipe Runner", () => { expect(charm.key("text").get()).toEqual("b"); }); + + it("should allow Cell.push of newly created charms", async () => { + const InnerSchema = { + type: "object", + properties: { + text: { type: "string" }, + }, + required: ["text"], + } as const satisfies JSONSchema; + + const OuterSchema = { + type: "object", + properties: { + list: { + type: "array", + items: InnerSchema, + default: [], + }, + }, + required: ["list"], + } as const satisfies JSONSchema; + + const HandlerState = { + type: "object", + properties: { + list: { + type: "array", + items: InnerSchema, + default: [], + asCell: true, + }, + }, + required: ["list"], + } as const satisfies JSONSchema; + + const OutputWithHandler = { + type: "object", + properties: { + list: { type: "array", items: InnerSchema, asCell: true }, + add: { ...InnerSchema, asStream: true }, + }, + required: ["add", "list"], + } as const satisfies JSONSchema; + + const charmCell = runtime.getCell>( + space, + "should allow Cell.push of newly created charms", + OutputWithHandler, + tx, + ); + + const innerPattern = pattern( + ({ text }) => { + return { text }; + }, + InnerSchema, + InnerSchema, + ); + + const add = handler( + InnerSchema, + HandlerState, + ({ text }, { list }) => { + const inner = innerPattern({ text }); + list.push(inner); + }, + ); + + const outerPattern = pattern( + ({ list }) => { + return { list, add: add({ list }) }; + }, + OuterSchema, + OutputWithHandler, + ); + + runtime.run(tx, outerPattern, {}, charmCell); + tx.commit(); + + await runtime.idle(); + + tx = runtime.edit(); + + const result = charmCell.withTx(tx).get(); + expect(isCell(result.list)).toBe(true); + expect(result.list.get()).toEqual([]); + expect(isStream(result.add)).toBe(true); + + result.add.withTx(tx).send({ text: "hello" }); + tx.commit(); + + await runtime.idle(); + + tx = runtime.edit(); + const result2 = charmCell.withTx(tx).get(); + expect(result2.list.get()).toEqual([{ text: "hello" }]); + }); });