Skip to content

Commit ab4caef

Browse files
seefeldbclaude
andcommitted
feat(api): Change .for() polarity to fail by default
Change the .for() method to throw an error by default if a cause is already set, with an optional boolean parameter to allow treating it as a suggestion. Changes: - Renamed parameter from `options: { force?: boolean }` to `allowIfSet?: boolean` - Default behavior: throw error if cause/link already exists - Pass `true` to silently ignore if cause already set (treat as suggestion) - Moved .for() signature to IAnyCell interface for consistency - Updated tests to verify new behavior This makes the API safer by preventing accidental overwrites while still allowing flexibility when needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 3b2f90a commit ab4caef

File tree

3 files changed

+34
-27
lines changed

3 files changed

+34
-27
lines changed

packages/api/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,14 @@ type IsThisArray =
5656
* IAnyCell is an interface that is used by all calls and to which the runner
5757
* attaches the internal methods..
5858
*/
59-
// deno-lint-ignore no-empty-interface
6059
export interface IAnyCell<T> {
60+
/**
61+
* Set a cause for this cell. Used to create a link when the cell doesn't have
62+
* one yet.
63+
* @param cause - The cause to associate with this cell
64+
* @returns This cell for method chaining
65+
*/
66+
for(cause: unknown): Cell<T>;
6167
}
6268

6369
/**

packages/runner/src/cell.ts

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,7 @@ declare module "@commontools/api" {
8686
* on Cell in the runner runtime.
8787
*/
8888
interface IAnyCell<out T> {
89-
/**
90-
* Set a cause for this cell. Used to create a link when the cell doesn't have one yet.
91-
* @param cause - The cause to associate with this cell
92-
* @param options - Optional configuration
93-
* @returns This cell for method chaining
94-
*/
95-
for(cause: unknown, options?: { force?: boolean }): Cell<T>;
89+
for(cause: unknown, allowIfSet?: boolean): Cell<T>;
9690
asSchema<S extends JSONSchema = JSONSchema>(
9791
schema: S,
9892
): Cell<Schema<S>>;
@@ -272,23 +266,26 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
272266
* This affects all sibling cells (created via .key(), .asSchema(), .withTx()) since they
273267
* share the same container.
274268
* @param cause - The cause to associate with this cell
275-
* @param options - Optional configuration
276-
* @param options.force - If true, will create an extension if link already exists. If false (default), ignores the call if link already exists.
269+
* @param allowIfSet - If true, treat as suggestion and silently ignore if cause already set. If false (default), throw error if cause already set.
277270
* @returns This cell for method chaining
278271
*/
279-
for(cause: unknown, options?: { force?: boolean }): Cell<T> {
280-
const force = options?.force ?? false;
281-
282-
// If cause or id already exists and force is false, silently ignore
283-
if ((this._causeContainer.id || this._causeContainer.cause) && !force) {
284-
return this as unknown as Cell<T>;
272+
for(cause: unknown, allowIfSet?: boolean): Cell<T> {
273+
// If cause or id already exists, either fail or silently ignore based on allowIfSet
274+
if (this._causeContainer.id || this._causeContainer.cause) {
275+
if (allowIfSet) {
276+
// Treat as suggestion - silently ignore
277+
return this as unknown as Cell<T>;
278+
} else {
279+
// Fail by default
280+
throw new Error(
281+
"Cannot set cause: cell already has a cause or link. Pass true as second parameter to allow this as a suggestion.",
282+
);
283+
}
285284
}
286285

287286
// Store the cause in the shared container - all siblings will see this
288287
this._causeContainer.cause = cause;
289288

290-
// TODO(seefeld): Implement extension creation when force is true and link exists
291-
292289
return this as unknown as Cell<T>;
293290
}
294291

packages/runner/test/cell-optional-link.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ describe("Cell with Optional Link", () => {
4545
expect(result).toBe(cell);
4646
});
4747

48-
it("should ignore .for() if link already exists", () => {
48+
it("should throw error if link already exists (default behavior)", () => {
4949
const existingCell = runtime.getCell<number>(
5050
space,
5151
"test-cell-with-link",
@@ -55,28 +55,32 @@ describe("Cell with Optional Link", () => {
5555
existingCell.set(42);
5656

5757
const cause = { type: "test-cause" };
58-
const result = existingCell.for(cause);
5958

60-
// Should return the cell for chaining
61-
expect(result).toBe(existingCell);
59+
// Should throw by default
60+
expect(() => existingCell.for(cause)).toThrow(
61+
"Cannot set cause: cell already has a cause or link",
62+
);
63+
6264
// Cell should still work normally
6365
expect(existingCell.get()).toBe(42);
6466
});
6567

66-
it("should allow force option to override existing link behavior", () => {
68+
it("should allow allowIfSet option to treat as suggestion", () => {
6769
const existingCell = runtime.getCell<number>(
6870
space,
69-
"test-cell-force",
71+
"test-cell-allow",
7072
undefined,
7173
tx,
7274
);
7375
existingCell.set(42);
7476

7577
const cause = { type: "test-cause" };
76-
const result = existingCell.for(cause, { force: true });
78+
const result = existingCell.for(cause, true); // allowIfSet = true
7779

78-
// Should return the cell for chaining
80+
// Should return the cell for chaining without throwing
7981
expect(result).toBe(existingCell);
82+
// Cell should still work normally
83+
expect(existingCell.get()).toBe(42);
8084
});
8185

8286
it("should support method chaining", () => {
@@ -248,7 +252,7 @@ describe("Cell with Optional Link", () => {
248252
cell.set(42);
249253

250254
// Should not affect the cell
251-
cell.for({ type: "ignored-cause" });
255+
cell.for({ type: "ignored-cause" }, true);
252256

253257
expect(cell.get()).toBe(42);
254258
});

0 commit comments

Comments
 (0)