From 9885d937499d185650aa6d7565fffcf92e827a33 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Wed, 15 Oct 2025 16:57:14 -0700 Subject: [PATCH 01/10] fix(runner): ensure IDs are added to array elements and default values This change ensures that when writing arrays to cells or pushing to array cells, all objects are given IDs so they can be stored as separate documents with links. This applies to both explicitly written arrays and default values from schemas. Changes to cell.ts: 1. Modified the `set` method to call `addIDIfNeeded` on each element when writing an array, ensuring all objects get IDs before being processed by `diffAndUpdate`. 2. Enhanced the `push` method to: - Process default values from schemas using `processDefaultValue` to ensure they're properly materialized with IDs - Apply `addIDIfNeeded` to all elements (both existing defaults and newly pushed values) to ensure consistent ID assignment 3. Improved the `update` method's schema validation to: - Use `resolveSchema` to properly handle schema references - Check for undefined schemas (which allow objects) - Consolidate the schema validation logic to determine if objects are allowed 4. Added new `addIDIfNeeded` helper function that: - Checks if a value is an object without an ID - Generates a new ID from the frame's counter if needed - Preserves existing IDs when present New tests in cell.test.ts: - "set operations with arrays" suite: - Tests that objects written as arrays each get their own document ID - Verifies that existing IDs are preserved during array writes - Uses `asSchema` with `asCell: true` to read back as cells and verify each element has a distinct document ID - "push operations with default values" suite: - Tests that default values from schemas are properly materialized with IDs - Verifies all objects (defaults + pushed) get unique IDs - Tests push operations both with and without schema defaults These changes ensure that array operations consistently create separate documents for each object, maintaining proper referential structure in the storage layer. --- packages/runner/src/cell.ts | 89 +++++++++----- packages/runner/src/schema.ts | 2 +- packages/runner/test/cell.test.ts | 188 +++++++++++++++++++++++++++--- 3 files changed, 235 insertions(+), 44 deletions(-) diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index 7a9199d68..3eaf4ed2d 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -24,7 +24,11 @@ import { diffAndUpdate } from "./data-updating.ts"; import { resolveLink } from "./link-resolution.ts"; import { ignoreReadForScheduling, txToReactivityLog } from "./scheduler.ts"; import { type Cancel, isCancel, useCancelGroup } from "./cancel.ts"; -import { validateAndTransform } from "./schema.ts"; +import { + processDefaultValue, + resolveSchema, + validateAndTransform, +} from "./schema.ts"; import { toURI } from "./uri-utils.ts"; import { type LegacyJSONCellLink, @@ -448,6 +452,11 @@ export class RegularCell implements Cell { // retry on conflict. if (!this.synced) this.sync(); + // If writing a whole array, make sure each object become their own doc. + if (Array.isArray(newValue)) { + newValue = newValue.map((val) => addIDIfNeeded(val)) as typeof newValue; + } + // TODO(@ubik2) investigate whether i need to check classified as i walk down my own obj diffAndUpdate( this.runtime, @@ -486,29 +495,30 @@ export class RegularCell implements Cell { const resolvedLink = resolveLink(this.tx, this.link); const currentValue = this.tx.readValueOrThrow(resolvedLink); - // If there's no current value, initialize based on schema + // If there's no current value, initialize based on schema, even if there is + // no default value. if (currentValue === undefined) { - if (isObject(this.schema)) { - // Check if schema allows objects - const allowsObject = ContextualFlowControl.isTrueSchema(this.schema) || - this.schema.type === "object" || - (Array.isArray(this.schema.type) && - this.schema.type.includes("object")) || - (this.schema.anyOf && - this.schema.anyOf.some((s) => - typeof s === "object" && s.type === "object" - )); - - if (!allowsObject) { - throw new Error( - "Cannot update with object value - schema does not allow objects", - ); - } - } else if (this.schema === false) { + const resolvedSchema = resolveSchema(this.schema, this.rootSchema); + + // TODO(seefeld,ubik2): This should all be moved to schema helpers. This + // just wants to know whether the value could be an object. + const allowsObject = resolvedSchema === undefined || + ContextualFlowControl.isTrueSchema(resolvedSchema) || + (isObject(resolvedSchema) && + (resolvedSchema.type === "object" || + (Array.isArray(resolvedSchema.type) && + resolvedSchema.type.includes("object")) || + (resolvedSchema.anyOf && + resolvedSchema.anyOf.some((s) => + typeof s === "object" && s.type === "object" + )))); + + if (!allowsObject) { throw new Error( "Cannot update with object value - schema does not allow objects", ); } + this.tx.writeValueOrThrow(resolvedLink, {}); } @@ -546,14 +556,6 @@ export class RegularCell implements Cell { throw new Error("Can't push into non-array value"); } - // If this is an object and it doesn't have an ID, add one. - const valuesToWrite = value.map((val: any) => - (!isLink(val) && isObject(val) && - (val as { [ID]?: unknown })[ID] === undefined && getTopFrame()) - ? { [ID]: getTopFrame()!.generatedIdCounter++, ...val } - : val - ); - // If there is no array yet, create it first. We have to do this as a // separate operation, so that in the next steps [ID] is properly anchored // in the array. @@ -565,17 +567,27 @@ export class RegularCell implements Cell { [], cause, ); - array = isObject(this.schema) && Array.isArray(this.schema?.default) - ? this.schema.default + const resolvedSchema = resolveSchema(this.schema, this.rootSchema); + array = isObject(resolvedSchema) && Array.isArray(resolvedSchema?.default) + ? processDefaultValue( + this.runtime, + this.tx, + this.link, + resolvedSchema.default, + ) : []; } + // Make sure all objects have IDs, whether they were previously written or + // came from a default value. + const valuesToWrite = [...array, ...value].map((val) => addIDIfNeeded(val)); + // Append the new values to the array. diffAndUpdate( this.runtime, this.tx, resolvedLink, - [...array, ...valuesToWrite], + valuesToWrite, cause, ); } @@ -877,6 +889,23 @@ function subscribeToReferencedDocs( }; } +function addIDIfNeeded(value: T): T { + if ( + (!isLink(value) && isObject(value) && + (value as { [ID]?: unknown })[ID] === undefined && getTopFrame()) + ) { + return { [ID]: getTopFrame()!.generatedIdCounter++, ...value }; + } else { + return value; + } +} + +/** + * Converts cells and objects that can be turned to cells to links. + * + * @param value - The value to convert. + * @returns The converted value. + */ export function convertCellsToLinks( value: readonly any[] | Record | any, path: string[] = [], diff --git a/packages/runner/src/schema.ts b/packages/runner/src/schema.ts index 6859069ca..626a79d63 100644 --- a/packages/runner/src/schema.ts +++ b/packages/runner/src/schema.ts @@ -102,7 +102,7 @@ export function resolveSchema( * * For `required` objects and arrays assume {} and [] as default value. */ -function processDefaultValue( +export function processDefaultValue( runtime: IRuntime, tx: IExtendedStorageTransaction | undefined, link: NormalizedFullLink, diff --git a/packages/runner/test/cell.test.ts b/packages/runner/test/cell.test.ts index 7765717bd..497a4d96f 100644 --- a/packages/runner/test/cell.test.ts +++ b/packages/runner/test/cell.test.ts @@ -1850,13 +1850,7 @@ describe("asCell with schema", () => { expect(arrayCell.get()).toEqualIgnoringSymbols([10, 20, 30, 40]); }); - it("should push values to undefined array with schema default has stable IDs", () => { - const schema = { - type: "array", - items: { type: "object", properties: { value: { type: "number" } } }, - default: [{ [ID]: "test", value: 10 }, { [ID]: "test2", value: 20 }], - } as const satisfies JSONSchema; - + it("should push values to undefined array with reused IDs", () => { const c = runtime.getCell<{ items?: any[] }>( space, "push-to-undefined-schema-stable-id", @@ -1864,20 +1858,16 @@ describe("asCell with schema", () => { tx, ); c.set({}); - const arrayCell = c.key("items").asSchema(schema); + const arrayCell = c.key("items"); arrayCell.push({ [ID]: "test3", "value": 30 }); expect(arrayCell.get()).toEqualIgnoringSymbols([ - { "value": 10 }, - { "value": 20 }, { "value": 30 }, ]); - arrayCell.push({ [ID]: "test", "value": 40 }); + arrayCell.push({ [ID]: "test3", "value": 40 }); expect(arrayCell.get()).toEqualIgnoringSymbols([ { "value": 40 }, // happens to overwrite, because IDs are the same - { "value": 20 }, - { "value": 30 }, { "value": 40 }, ]); }); @@ -3327,4 +3317,176 @@ describe("Cell success callbacks", () => { const status = tx.status(); expect(status.status).toBe("error"); }); + + describe("set operations with arrays", () => { + it("should add IDs to objects when setting an array", () => { + const frame = pushFrame(); + const cell = runtime.getCell<{ name: string; value: number }[]>( + space, + "array-set-test", + { type: "array" }, + tx, + ); + + const objects = [ + { name: "first", value: 1 }, + { name: "second", value: 2 }, + ]; + + cell.set(objects); + popFrame(frame); + + const result = cell.asSchema({ + type: "array", + items: { + type: "object", + properties: { name: { type: "string" }, value: { type: "number" } }, + asCell: true, + }, + }).get(); + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBe(2); + expect(isCell(result[0])).toBe(true); + expect(isCell(result[1])).toBe(true); + const link0 = result[0].getAsNormalizedFullLink(); + const link1 = result[1].getAsNormalizedFullLink(); + expect(link0.id).not.toBe(link1.id); + expect(link0.path).toEqual([]); + expect(link1.path).toEqual([]); + expect(result[0].get().name).toBe("first"); + expect(result[1].get().name).toBe("second"); + }); + + it("should preserve existing IDs when setting an array", () => { + const initialDataCell = runtime.getCell<{ name: string; value: number }>( + space, + "array-set-preserve-id-test-initial", + { + type: "object", + properties: { name: { type: "string" }, value: { type: "number" } }, + }, + tx, + ); + initialDataCell.set({ name: "first", value: 1 }); + + const frame = pushFrame(); + const cell = runtime.getCell<{ name: string; value: number }[]>( + space, + "array-set-preserve-id-test", + { type: "array" }, + tx, + ); + + const objects = [ + initialDataCell, + { name: "second", value: 2 }, + ]; + + cell.set(objects); + popFrame(frame); + + const result = cell.asSchema({ + type: "array", + items: { + type: "object", + properties: { name: { type: "string" }, value: { type: "number" } }, + asCell: true, + }, + }).get(); + expect(isCell(result[0])).toBe(true); + expect(isCell(result[1])).toBe(true); + const link0 = result[0].getAsNormalizedFullLink(); + const link1 = result[1].getAsNormalizedFullLink(); + expect(link0.id).toBe(initialDataCell.getAsNormalizedFullLink().id); + expect(link0.id).not.toBe(link1.id); + }); + }); + + describe("push operations with default values", () => { + it("should use default values from schema when pushing to empty array", () => { + const frame = pushFrame(); + const cell = runtime.getCell<{ name: string; count: number }[]>( + space, + "push-with-defaults-test", + { + type: "array", + default: [{ name: "default", count: 0 }], + }, + tx, + ); + + cell.push({ name: "new", count: 5 }); + popFrame(frame); + + const result = cell.get(); + expect(result.length).toBe(2); + expect(result[0].name).toBe("default"); + expect(result[0].count).toBe(0); + expect(result[1].name).toBe("new"); + expect(result[1].count).toBe(5); + }); + + it("should add IDs to default values from schema", () => { + const frame = pushFrame(); + const cell = runtime.getCell<{ name: string }[]>( + space, + "push-defaults-with-id-test", + { + type: "array", + default: [{ name: "default1" }, { name: "default2" }], + }, + tx, + ); + + cell.push({ name: "new" }); + popFrame(frame); + + const result = cell.asSchema({ + type: "array", + items: { + type: "object", + properties: { name: { type: "string" } }, + asCell: true, + }, + }).get(); + expect(result.length).toBe(3); + expect(isCell(result[0])).toBe(true); + expect(isCell(result[1])).toBe(true); + expect(isCell(result[2])).toBe(true); + const link0 = result[0].getAsNormalizedFullLink(); + const link1 = result[1].getAsNormalizedFullLink(); + const link2 = result[2].getAsNormalizedFullLink(); + expect(link0.id).not.toBe(link1.id); + expect(link1.id).not.toBe(link2.id); + expect(link0.id).not.toBe(link2.id); + }); + + it("should push objects with IDs even without schema defaults", () => { + const frame = pushFrame(); + const cell = runtime.getCell<{ value: number }[]>( + space, + "push-no-defaults-test", + { type: "array" }, + tx, + ); + + cell.push({ value: 1 }, { value: 2 }); + popFrame(frame); + + const result = cell.asSchema({ + type: "array", + items: { + type: "object", + properties: { value: { type: "number" } }, + asCell: true, + }, + }).get(); + expect(result.length).toBe(2); + expect(isCell(result[0])).toBe(true); + expect(isCell(result[1])).toBe(true); + const link0 = result[0].getAsNormalizedFullLink(); + const link1 = result[1].getAsNormalizedFullLink(); + expect(link0.id).not.toBe(link1.id); + }); + }); }); From 36949dbd442b7481311ffcadce0b5a6eb12063f6 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Wed, 15 Oct 2025 17:50:38 -0700 Subject: [PATCH 02/10] make adding IDs recursive, since the calendar use-case sets objects with arrays --- packages/runner/src/cell.ts | 83 +++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index 3eaf4ed2d..7ab96e515 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -452,10 +452,8 @@ export class RegularCell implements Cell { // retry on conflict. if (!this.synced) this.sync(); - // If writing a whole array, make sure each object become their own doc. - if (Array.isArray(newValue)) { - newValue = newValue.map((val) => addIDIfNeeded(val)) as typeof newValue; - } + // Looks for arrays and makes sure each object gets its own doc. + newValue = recursivelyAddIDIfNeeded(newValue); // TODO(@ubik2) investigate whether i need to check classified as i walk down my own obj diffAndUpdate( @@ -525,7 +523,7 @@ export class RegularCell implements Cell { // Now update each property for (const [key, value] of Object.entries(values)) { // Workaround for type checking, since T can be Cell<> and that's fine. - (this.key as any)(key).set(value); + (this.key as any)(key).set(recursivelyAddIDIfNeeded(value)); } } @@ -578,16 +576,12 @@ export class RegularCell implements Cell { : []; } - // Make sure all objects have IDs, whether they were previously written or - // came from a default value. - const valuesToWrite = [...array, ...value].map((val) => addIDIfNeeded(val)); - // Append the new values to the array. diffAndUpdate( this.runtime, this.tx, resolvedLink, - valuesToWrite, + recursivelyAddIDIfNeeded([...array, ...value]), cause, ); } @@ -889,14 +883,69 @@ function subscribeToReferencedDocs( }; } -function addIDIfNeeded(value: T): T { - if ( - (!isLink(value) && isObject(value) && - (value as { [ID]?: unknown })[ID] === undefined && getTopFrame()) - ) { - return { [ID]: getTopFrame()!.generatedIdCounter++, ...value }; +/** + * Recursively adds IDs elements in arrays, unless they are already a link. + * + * This ensures that mutable arrays only consist of links to documents, at least + * when written to only via .set, .update and .push above. + * + * TODO(seefeld): When an array has default entries and is rewritten as [...old, + * new], this will still break, because the previous entries will point back to + * the array itself instead of being new entries. + * + * @param value - The value to add IDs to. + * @returns The value with IDs added. + */ +function recursivelyAddIDIfNeeded( + value: T, + seen: Map = new Map(), +): T { + // Can't add IDs without top frame. + if (!getTopFrame()) return value; + + // Already a link, no need to add IDs. Not a record, no need to add IDs. + if (!isRecord(value) || isLink(value)) return value; + + // Already seen, return previously annotated result. + if (seen.has(value)) return seen.get(value) as T; + + if (Array.isArray(value)) { + const result: unknown[] = []; + + // Set before traversing, otherwise we'll infinite recurse. + seen.set(value, result); + + result.push(...value.map((v) => { + const value = recursivelyAddIDIfNeeded(v, seen); + // For objects on arrays only: Add ID if not already present. + if ( + isObject(value) && !isLink(value) && !(ID in value) + ) { + return { [ID]: getTopFrame()!.generatedIdCounter++, ...value }; + } else { + return value; + } + })); + return result as T; } else { - return value; + const result: Record = {}; + + // Set before traversing, otherwise we'll infinite recurse. + seen.set(value, result); + + Object.entries(value).forEach(([key, v]) => { + result[key] = recursivelyAddIDIfNeeded(v, seen); + }); + + // Copy supported symbols from original value. + if (ID in value) { + (result as { [ID]: unknown })[ID] = value[ID]; + } + if (ID_FIELD in value) { + (result as { [ID_FIELD]: unknown })[ID_FIELD] = value[ID_FIELD]; + } + + return result as T; } } From d65872b59875888540507131b055286a174692de Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Wed, 15 Oct 2025 18:07:22 -0700 Subject: [PATCH 03/10] make toCell non-enumerable so that {...obj} doesn't copy them and accidentally points at the old object --- packages/runner/src/schema.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/runner/src/schema.ts b/packages/runner/src/schema.ts index 626a79d63..fa7b8b56c 100644 --- a/packages/runner/src/schema.ts +++ b/packages/runner/src/schema.ts @@ -310,8 +310,15 @@ function annotateWithBackToCellSymbols( isRecord(value) && !isCell(value) && !isStream(value) && !isQueryResultForDereferencing(value) ) { - value[toCell] = () => createCell(runtime, link, tx); - value[toOpaqueRef] = () => makeOpaqueRef(link); + // Non-enumerable, so that {...obj} won't copy these symbols + Object.defineProperty(value, toCell, { + value: () => createCell(runtime, link, tx), + enumerable: false, + }); + Object.defineProperty(value, toOpaqueRef, { + value: () => makeOpaqueRef(link), + enumerable: false, + }); Object.freeze(value); } return value; From edf4e45c4a1cd5318c7f29eec35f60a3c4bb8067 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 12:21:03 -0700 Subject: [PATCH 04/10] add test for non-enumerability of toCell --- packages/runner/test/schema.test.ts | 112 ++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/packages/runner/test/schema.test.ts b/packages/runner/test/schema.test.ts index ea4e4c1be..1a0f1de18 100644 --- a/packages/runner/test/schema.test.ts +++ b/packages/runner/test/schema.test.ts @@ -2750,4 +2750,116 @@ describe("Schema Support", () => { expect(remainingLink.id).toBe(links[1].id); }); }); + + describe("toCell symbol non-enumerable behavior", () => { + it("should not copy toCell symbol when spreading object", () => { + const cell = runtime.getCell<{ name: string; value: number }>( + space, + "spread-test", + { + type: "object", + properties: { + name: { type: "string" }, + value: { type: "number" }, + }, + }, + tx, + ); + + cell.set({ name: "original", value: 42 }); + const obj = cell.get(); + + // Verify the object has toCell + expect((obj as any)[toCell]).toBeDefined(); + expect(typeof (obj as any)[toCell]).toBe("function"); + + // Spread the object + const spread = { ...obj }; + + // The spread object should NOT have toCell + expect((spread as any)[toCell]).toBeUndefined(); + + // The original object should still have toCell + expect((obj as any)[toCell]).toBeDefined(); + }); + + it("should not copy toCell when modifying object with spread", () => { + const cell = runtime.getCell<{ name: string; value: number }>( + space, + "spread-modify-test", + { + type: "object", + properties: { + name: { type: "string" }, + value: { type: "number" }, + }, + }, + tx, + ); + + cell.set({ name: "original", value: 42 }); + const obj = cell.get(); + + // Create a modified copy using spread + const modified = { ...obj, value: 100 }; + + // The modified object should not have toCell + expect((modified as any)[toCell]).toBeUndefined(); + + // The original should still have toCell pointing to the correct cell + const originalCell = (obj as any)[toCell](); + expect(isCell(originalCell)).toBe(true); + expect(originalCell.get()).toEqual({ name: "original", value: 42 }); + }); + + it("should not enumerate toCell in Object.keys", () => { + const cell = runtime.getCell<{ name: string; value: number }>( + space, + "keys-test", + { + type: "object", + properties: { + name: { type: "string" }, + value: { type: "number" }, + }, + }, + tx, + ); + + cell.set({ name: "test", value: 123 }); + const obj = cell.get(); + + // toCell should not appear in Object.keys + const keys = Object.keys(obj); + expect(keys).toEqual(["name", "value"]); + expect(keys).not.toContain(toCell); + }); + + it("should not enumerate toCell in for...in loop", () => { + const cell = runtime.getCell<{ name: string; value: number }>( + space, + "forin-test", + { + type: "object", + properties: { + name: { type: "string" }, + value: { type: "number" }, + }, + }, + tx, + ); + + cell.set({ name: "test", value: 456 }); + const obj = cell.get(); + + // Collect keys from for...in + const keys: string[] = []; + for (const key in obj) { + keys.push(key); + } + + expect(keys).toEqual(["name", "value"]); + expect(keys).not.toContain(toCell as any); + }); + }); }); From 284379a75d19618aa4db3f91cec1e69f757da7cf Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 13:55:37 -0700 Subject: [PATCH 05/10] minor cleanups --- packages/runner/src/builder/types.ts | 1 + packages/runner/src/cell.ts | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/runner/src/builder/types.ts b/packages/runner/src/builder/types.ts index 8145b934a..0e50903c6 100644 --- a/packages/runner/src/builder/types.ts +++ b/packages/runner/src/builder/types.ts @@ -49,6 +49,7 @@ export { AuthSchema } from "./schema-lib.ts"; export { ID, ID_FIELD, + type IDFields, NAME, type Schema, schema, diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index 7ab96e515..e8169ec89 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -5,6 +5,7 @@ import { type Cell, ID, ID_FIELD, + type IDFields, isStreamValue, type JSONSchema, type OpaqueRef, @@ -453,14 +454,14 @@ export class RegularCell implements Cell { if (!this.synced) this.sync(); // Looks for arrays and makes sure each object gets its own doc. - newValue = recursivelyAddIDIfNeeded(newValue); + const transformedValue = recursivelyAddIDIfNeeded(newValue); // TODO(@ubik2) investigate whether i need to check classified as i walk down my own obj diffAndUpdate( this.runtime, this.tx, resolveLink(this.tx, this.link, "writeRedirect"), - newValue, + transformedValue, getTopFrame()?.cause, ); @@ -522,8 +523,7 @@ export class RegularCell implements Cell { // Now update each property for (const [key, value] of Object.entries(values)) { - // Workaround for type checking, since T can be Cell<> and that's fine. - (this.key as any)(key).set(recursivelyAddIDIfNeeded(value)); + (this as Cell).key(key).set(value); } } @@ -903,7 +903,7 @@ function recursivelyAddIDIfNeeded( // Can't add IDs without top frame. if (!getTopFrame()) return value; - // Already a link, no need to add IDs. Not a record, no need to add IDs. + // Not a record, no need to add IDs. Already a link, no need to add IDs. if (!isRecord(value) || isLink(value)) return value; // Already seen, return previously annotated result. @@ -938,12 +938,12 @@ function recursivelyAddIDIfNeeded( }); // Copy supported symbols from original value. - if (ID in value) { - (result as { [ID]: unknown })[ID] = value[ID]; - } - if (ID_FIELD in value) { - (result as { [ID_FIELD]: unknown })[ID_FIELD] = value[ID_FIELD]; - } + [ID, ID_FIELD].forEach((symbol) => { + if (symbol in value) { + (result as IDFields)[symbol as keyof IDFields] = + value[symbol as keyof IDFields]; + } + }); return result as T; } From 6c54745afaae7ac3ea06cfd3c917bd9baddff275 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 15:37:34 -0700 Subject: [PATCH 06/10] fix test that was previously broken, but with the bug hidden by our bug --- .../integration/patterns/compliance-checklist.pattern.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/generated-patterns/integration/patterns/compliance-checklist.pattern.ts b/packages/generated-patterns/integration/patterns/compliance-checklist.pattern.ts index 6d7df9ebd..f8486dfc5 100644 --- a/packages/generated-patterns/integration/patterns/compliance-checklist.pattern.ts +++ b/packages/generated-patterns/integration/patterns/compliance-checklist.pattern.ts @@ -576,12 +576,8 @@ export const complianceChecklist = recipe( const insights = lift(computeInsights)(currentTasks); const tasksView = lift(cloneTasks)(currentTasks); - const categorySummaries = lift((snapshot: ComplianceInsights) => - snapshot.categories.map((entry) => ({ ...entry })) - )(insights); - const gapDetails = lift((snapshot: ComplianceInsights) => - snapshot.gapList.map((entry) => ({ ...entry })) - )(insights); + const categorySummaries = insights.categories; + const gapDetails = insights.gapList; const coveragePercent = lift((snapshot: ComplianceInsights) => snapshot.coveragePercent From df03fd37930501bb90f66f1f9e4f9dd0cf9f6287 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 15:41:43 -0700 Subject: [PATCH 07/10] feat(runner): implement Symbol.iterator for array query result proxies Add custom Symbol.iterator implementation for array query result proxies to support spreading and for...of iteration. When iterating over an array proxy, each element is now wrapped in its own query result proxy, maintaining reactivity and cell references throughout the iteration. This enables patterns like: - const spread = [...proxy.items] - for (const item of proxy.items) { ... } Each iterated element maintains its query result proxy wrapper, allowing access to toCell() and preserving the link to the underlying cell data. --- packages/runner/src/query-result-proxy.ts | 20 +++++ packages/runner/test/cell.test.ts | 90 +++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/packages/runner/src/query-result-proxy.ts b/packages/runner/src/query-result-proxy.ts index 1d17c7d1c..d3443d673 100644 --- a/packages/runner/src/query-result-proxy.ts +++ b/packages/runner/src/query-result-proxy.ts @@ -123,6 +123,26 @@ export function createQueryResultProxy( return () => createCell(runtime, link, tx, true); } else if (prop === toOpaqueRef) { return () => makeOpaqueRef(link); + } else if (prop === Symbol.iterator && Array.isArray(target)) { + return function () { + let index = 0; + return { + next() { + if (index < target.length) { + const result = { + value: createQueryResultProxy(runtime, tx, { + ...link, + path: [...link.path, String(index)], + }, depth + 1), + done: false, + }; + index++; + return result; + } + return { done: true }; + }, + }; + }; } const readTx = (tx?.status().status === "ready") ? tx : runtime.edit(); diff --git a/packages/runner/test/cell.test.ts b/packages/runner/test/cell.test.ts index 497a4d96f..581345449 100644 --- a/packages/runner/test/cell.test.ts +++ b/packages/runner/test/cell.test.ts @@ -590,6 +590,96 @@ describe("createProxy", () => { expect(proxy.nested.arrays[0][0]).toBe(1); }); + it("should support spreading array query results with for...of", () => { + const c = runtime.getCell<{ items: { name: string }[] }>( + space, + "should support spreading array query results", + undefined, + tx, + ); + c.set({ items: [{ name: "a" }, { name: "b" }, { name: "c" }] }); + const proxy = c.getAsQueryResult(); + + // Use for...of loop to iterate over the array + const collected: any[] = []; + for (const item of proxy.items) { + collected.push(item); + } + + expect(collected.length).toBe(3); + + // Each element should be a query result proxy (for objects) + expect(isQueryResult(collected[0])).toBe(true); + expect(isQueryResult(collected[1])).toBe(true); + expect(isQueryResult(collected[2])).toBe(true); + + // We can access properties through the proxies + expect(collected[0].name).toBe("a"); + expect(collected[1].name).toBe("b"); + expect(collected[2].name).toBe("c"); + }); + + it("should support spreading array query results with spread operator", () => { + const c = runtime.getCell<{ items: { id: number }[] }>( + space, + "should support spreading array with spread operator", + undefined, + tx, + ); + c.set({ items: [{ id: 1 }, { id: 2 }, { id: 3 }] }); + const proxy = c.getAsQueryResult(); + + // Spread the array into a new array + const spread = [...proxy.items]; + + expect(spread.length).toBe(3); + + // Each element should be a query result proxy (for objects) + expect(isQueryResult(spread[0])).toBe(true); + expect(isQueryResult(spread[1])).toBe(true); + expect(isQueryResult(spread[2])).toBe(true); + + // Verify we can access properties + expect(spread[0].id).toBe(1); + expect(spread[1].id).toBe(2); + expect(spread[2].id).toBe(3); + }); + + it("should support spreading nested array query results", () => { + const c = runtime.getCell<{ nested: { data: { value: number }[][] } }>( + space, + "should support spreading nested arrays", + undefined, + tx, + ); + c.set({ + nested: { + data: [[{ value: 1 }, { value: 2 }], [{ value: 3 }, { value: 4 }]], + }, + }); + const proxy = c.getAsQueryResult(); + + // Spread the outer array + const outerSpread = [...proxy.nested.data]; + expect(outerSpread.length).toBe(2); + + // Each inner array should be a query result proxy + expect(isQueryResult(outerSpread[0])).toBe(true); + expect(isQueryResult(outerSpread[1])).toBe(true); + + // Spread an inner array + const innerSpread = [...outerSpread[0]]; + expect(innerSpread.length).toBe(2); + + // Elements of the inner array should also be query result proxies (for objects) + expect(isQueryResult(innerSpread[0])).toBe(true); + expect(isQueryResult(innerSpread[1])).toBe(true); + + // Verify we can access properties + expect(innerSpread[0].value).toBe(1); + expect(innerSpread[1].value).toBe(2); + }); + it.skip("should support pop() and only read the popped element", () => { const c = runtime.getCell<{ a: number[] }>( space, From 2b98584fffc12e0de1f98d3239d51d1ca71eafa1 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 15:44:34 -0700 Subject: [PATCH 08/10] test(runner): add tests for Symbol.iterator in array query result proxies Add comprehensive tests verifying Symbol.iterator behavior for array query result proxies: - for...of iteration with object elements - Spread operator with object elements - Nested array spreading with proper proxy preservation - Arrays containing cell references (links to other cells) Tests confirm that iteration and spreading maintain query result proxy wrappers for each element, allowing access to toCell() and preserving reactivity throughout iteration. The cell references test specifically validates that link resolution works correctly when iterating over arrays of cell links. --- packages/runner/test/cell.test.ts | 63 +++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/packages/runner/test/cell.test.ts b/packages/runner/test/cell.test.ts index 581345449..2477944c4 100644 --- a/packages/runner/test/cell.test.ts +++ b/packages/runner/test/cell.test.ts @@ -680,6 +680,69 @@ describe("createProxy", () => { expect(innerSpread[1].value).toBe(2); }); + it("should support spreading arrays with cell references", () => { + // Create individual cells to reference + const cell1 = runtime.getCell<{ name: string; value: number }>( + space, + "ref-cell-1", + undefined, + tx, + ); + cell1.set({ name: "first", value: 100 }); + + const cell2 = runtime.getCell<{ name: string; value: number }>( + space, + "ref-cell-2", + undefined, + tx, + ); + cell2.set({ name: "second", value: 200 }); + + const cell3 = runtime.getCell<{ name: string; value: number }>( + space, + "ref-cell-3", + undefined, + tx, + ); + cell3.set({ name: "third", value: 300 }); + + // Create an array cell containing references to other cells + const arrayCell = runtime.getCell( + space, + "array-with-refs", + undefined, + tx, + ); + arrayCell.set([cell1, cell2, cell3]); + + const proxy = arrayCell.getAsQueryResult(); + + // Spread the array + const spread = [...proxy]; + + expect(spread.length).toBe(3); + + // Each element should be a query result proxy + expect(isQueryResult(spread[0])).toBe(true); + expect(isQueryResult(spread[1])).toBe(true); + expect(isQueryResult(spread[2])).toBe(true); + + // Verify we can access the referenced cells' data + expect(spread[0].name).toBe("first"); + expect(spread[0].value).toBe(100); + expect(spread[1].name).toBe("second"); + expect(spread[1].value).toBe(200); + expect(spread[2].name).toBe("third"); + expect(spread[2].value).toBe(300); + + // Use for...of to iterate + const names: string[] = []; + for (const item of proxy) { + names.push(item.name); + } + expect(names).toEqual(["first", "second", "third"]); + }); + it.skip("should support pop() and only read the popped element", () => { const c = runtime.getCell<{ a: number[] }>( space, From 5f8bac1aa93009db8115a589e29f8486c4abc37e Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 16:37:38 -0700 Subject: [PATCH 09/10] log .length read in tx for Symbol.iterator --- packages/runner/src/query-result-proxy.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/runner/src/query-result-proxy.ts b/packages/runner/src/query-result-proxy.ts index d3443d673..18ac80926 100644 --- a/packages/runner/src/query-result-proxy.ts +++ b/packages/runner/src/query-result-proxy.ts @@ -128,7 +128,14 @@ export function createQueryResultProxy( let index = 0; return { next() { - if (index < target.length) { + const readTx = (tx?.status().status === "ready") + ? tx + : runtime.edit(); + const length = readTx.readValueOrThrow({ + ...link, + path: [...link.path, "length"], + }) as number; + if (index < length) { const result = { value: createQueryResultProxy(runtime, tx, { ...link, From 42b9ec0516b8243a1c0fbb7fa19b984adf977aee Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Thu, 16 Oct 2025 16:46:17 -0700 Subject: [PATCH 10/10] outliner: clone tree before applying edit otherwise Cell.set turns this back into cell references and this can create loops (fwiw, we might want to turn this into a structure where each node is its own document with its own id, in which case we'd actually want this behavior) --- packages/ui/src/v2/components/ct-outliner/tree-operations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/v2/components/ct-outliner/tree-operations.ts b/packages/ui/src/v2/components/ct-outliner/tree-operations.ts index dd0c0fd50..eb32e8717 100644 --- a/packages/ui/src/v2/components/ct-outliner/tree-operations.ts +++ b/packages/ui/src/v2/components/ct-outliner/tree-operations.ts @@ -447,7 +447,7 @@ export const TreeOperations = { // Add nodes after the deleted one newChildren.push(...currentChildren.slice(nodeIndex + 1)); - cell.set(newChildren); + cell.set(JSON.parse(JSON.stringify(newChildren))); }); // Calculate new focus after deletion - need to check the actual updated array