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 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 7a9199d68..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, @@ -24,7 +25,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,12 +453,15 @@ export class RegularCell implements Cell { // retry on conflict. if (!this.synced) this.sync(); + // Looks for arrays and makes sure each object gets its own doc. + 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, ); @@ -486,36 +494,36 @@ 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, {}); } // 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 as Cell).key(key).set(value); } } @@ -546,14 +554,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,8 +565,14 @@ 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, + ) : []; } @@ -575,7 +581,7 @@ export class RegularCell implements Cell { this.runtime, this.tx, resolvedLink, - [...array, ...valuesToWrite], + recursivelyAddIDIfNeeded([...array, ...value]), cause, ); } @@ -877,6 +883,78 @@ function subscribeToReferencedDocs( }; } +/** + * 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; + + // 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. + 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 { + 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. + [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; + } +} + +/** + * 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/query-result-proxy.ts b/packages/runner/src/query-result-proxy.ts index 1d17c7d1c..18ac80926 100644 --- a/packages/runner/src/query-result-proxy.ts +++ b/packages/runner/src/query-result-proxy.ts @@ -123,6 +123,33 @@ 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() { + 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, + 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/src/schema.ts b/packages/runner/src/schema.ts index 6859069ca..fa7b8b56c 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, @@ -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; diff --git a/packages/runner/test/cell.test.ts b/packages/runner/test/cell.test.ts index 7765717bd..2477944c4 100644 --- a/packages/runner/test/cell.test.ts +++ b/packages/runner/test/cell.test.ts @@ -590,6 +590,159 @@ 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("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, @@ -1850,13 +2003,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 +2011,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 +3470,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); + }); + }); }); 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); + }); + }); }); 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