Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions packages/patterns/linkedlist-in-cell.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/// <cts-enable />
import {
h,
Cell,
cell,
Default,
derive,
h,
handler,
NAME,
recipe,
Expand All @@ -30,21 +30,11 @@ interface ListState {
items_list: Cell<LinkedList>;
}

// Helper function to copy a linked list
function copyList(list: LinkedList): LinkedList {
return {
value: list.value,
next: list.next ? copyList(list.next) : undefined,
};
}

// Helper function to add a node to the linked list
// copy the list because it has reactive symbols in it
// FIXME(@ellyxir): why do these symbols break things?
function addNodeToList(list: LinkedList, value: string): LinkedList {
return {
value: value,
next: copyList(list),
next: list,
};
}

Expand Down
138 changes: 135 additions & 3 deletions packages/runner/src/data-updating.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { isRecord } from "@commontools/utils/types";
import { getLogger } from "@commontools/utils/logger";
import { ID, ID_FIELD, type JSONSchema } from "./builder/types.ts";
import { type DocImpl, isDoc } from "./doc.ts";
import { createRef } from "./doc-map.ts";
import { isCell, RegularCell } from "./cell.ts";
import { resolveLink } from "./link-resolution.ts";
import { toCell, toOpaqueRef } from "./back-to-cell.ts";
import {
areLinksSame,
areMaybeLinkAndNormalizedLinkSame,
Expand Down Expand Up @@ -55,6 +57,7 @@ export function diffAndUpdate(
context,
options,
);
diffLogger.debug(() => `[diffAndUpdate] changes: ${JSON.stringify(changes)}`);
applyChangeSet(tx, changes);
return changes.length > 0;
}
Expand Down Expand Up @@ -85,6 +88,34 @@ type ChangeSet = {
* @param context - The context of the change.
* @returns An array of changes that should be written.
*/
const diffLogger = getLogger("normalizeAndDiff", {
enabled: false,
level: "debug",
});

/**
* Returns true if `target` is the immediate parent of `base` in the same document.
*
* Example:
* - base.path = ["internal", "__#1", "next"]
* - target.path = ["internal", "__#1"]
*
* This is used to decide when to collapse a self/parent link that would create
* a tight self-loop (e.g., obj.next -> obj) while allowing references to
* higher ancestors (like an item's `items` pointing to its containing array).
*/
function isImmediateParent(
target: NormalizedFullLink,
base: NormalizedFullLink,
): boolean {
return (
target.id === base.id &&
target.space === base.space &&
target.path.length === base.path.length - 1 &&
target.path.every((seg, i) => seg === base.path[i])
);
}

export function normalizeAndDiff(
runtime: IRuntime,
tx: IExtendedStorageTransaction,
Expand All @@ -96,9 +127,21 @@ export function normalizeAndDiff(
): ChangeSet {
const changes: ChangeSet = [];

// Log entry with value type and symbol presence
const valueType = Array.isArray(newValue) ? "array" : typeof newValue;
const pathStr = link.path.join(".");
diffLogger.debug(() =>
`[DIFF_ENTER] path=${pathStr} type=${valueType} newValue=${
JSON.stringify(newValue as any)
}`
);

// When detecting a circular reference on JS objects, turn it into a cell,
// which below will be turned into a relative link.
if (seen.has(newValue)) {
diffLogger.debug(() =>
`[SEEN_CHECK] Already seen object at path=${pathStr}, converting to cell`
);
newValue = new RegularCell(runtime, seen.get(newValue)!, tx);
}

Expand All @@ -114,6 +157,9 @@ export function normalizeAndDiff(
isRecord(newValue) &&
newValue[ID_FIELD] !== undefined
) {
diffLogger.debug(() =>
`[BRANCH_ID_FIELD] Processing ID_FIELD redirect at path=${pathStr}`
);
const { [ID_FIELD]: fieldName, ...rest } = newValue as
& { [ID_FIELD]: string }
& Record<string, JSONValue>;
Expand Down Expand Up @@ -167,16 +213,35 @@ export function normalizeAndDiff(

// Unwrap proxies and handle special types
if (isQueryResultForDereferencing(newValue)) {
newValue = createSigilLinkFromParsedLink(parseLink(newValue));
const parsedLink = parseLink(newValue);
const sigilLink = createSigilLinkFromParsedLink(parsedLink);
diffLogger.debug(() =>
`[BRANCH_QUERY_RESULT] Converted query result to sigil link at path=${pathStr} link=${sigilLink} parsedLink=${parsedLink}`
);
newValue = sigilLink;
}

if (isDoc(newValue)) {
throw new Error("Docs are not supported anymore");
}
if (isCell(newValue)) newValue = newValue.getAsLink();
// 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.
let linkOriginFromCell = false;
if (isCell(newValue)) {
diffLogger.debug(() =>
`[BRANCH_CELL] Converting cell to link at path=${pathStr}`
);
linkOriginFromCell = true;
newValue = newValue.getAsLink();
}

// If we're about to create a reference to ourselves, no-op
if (areMaybeLinkAndNormalizedLinkSame(newValue, link)) {
diffLogger.debug(() =>
`[BRANCH_SELF_REF] Self-reference detected, no-op at path=${pathStr}`
);
return [];
}

Expand All @@ -189,15 +254,24 @@ export function normalizeAndDiff(
isWriteRedirectLink(currentValue) &&
areNormalizedLinksSame(parseLink(currentValue, link), link)
) {
diffLogger.debug(() =>
`[BRANCH_WRITE_REDIRECT] Same redirect, no-op at path=${pathStr}`
);
return [];
} else {
diffLogger.debug(() =>
`[BRANCH_WRITE_REDIRECT] Different redirect, updating at path=${pathStr}`
);
changes.push({ location: link, value: newValue as JSONValue });
return changes;
}
}

// Handle alias in current value (at this point: if newValue is not an alias)
if (isWriteRedirectLink(currentValue)) {
diffLogger.debug(() =>
`[BRANCH_CURRENT_ALIAS] Following current value alias at path=${pathStr}`
);
// Log reads of the alias, so that changing aliases cause refreshes
const redirectLink = resolveLink(
tx,
Expand All @@ -216,12 +290,47 @@ export function normalizeAndDiff(
}

if (isAnyCellLink(newValue)) {
diffLogger.debug(() =>
`[BRANCH_CELL_LINK] Processing cell link at path=${pathStr} link=${
JSON.stringify(newValue as any)
}`
);
const parsedLink = parseLink(newValue, link);

// Collapse same-document self/parent links created by query-result dereferencing.
// Example: "internal.__#1.next" -> "internal.__#1". Writing that link would
// create a tight self-loop, so we instead embed the target's current value
// (a plain JSON snapshot). Do not collapse when the link came from converting
// a seen cycle to a Cell, and only collapse when the target is the immediate
// parent path.
if (!linkOriginFromCell && isImmediateParent(parsedLink, link)) {
diffLogger.debug(() =>
`[CELL_LINK_COLLAPSE] Same-doc ancestor/self link detected at path=${pathStr} -> embedding snapshot from ${
parsedLink.path.join(".")
}`
);
const snapshot = tx.readValueOrThrow(
parsedLink,
options,
) as unknown;
return normalizeAndDiff(
runtime,
tx,
link,
snapshot,
context,
options,
seen,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does your data looks something like this?

const doc: Cell<List> = {value: 1};
// point next to the top of the doc
doc.key("next").set(doc);

I think doc would look like:

{"value": 1, "next": {"/": {"@link1": {"id": "baed", "path": []}}}

normalizeAndDiff has the seen map, so I'm curious why it's not sufficient here.
Making the snapshot seems ok, since you're only using it to generate the diff (so you don't have to worry about being wrong if it gets out of sync), but I didn't get why it was required.

Is this a case where we're replacing an object with a link, and that replacement occurs on an $alias?

Copy link
Contributor Author

@ellyxir ellyxir Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we turn the "next" value into a link and it looks a bit like this:
[DEBUG][normalizeAndDiff::14:30:46.220] [BRANCH_CELL_LINK] Processing cell link at path=internal.#1.next link={"/":{"link@1":{"path":["internal","#1"],"id":"of:baedreidafjd3rgnpyyxxbwvvf75qc7vk2s5nzsbhw76djm4u2yhwkvptby","space":"did:key:z6MktpWYAnNacQL9AEkhqxaKR1qux7uU5rM6PjBvPst6LEWc"}}}

the problem is that it points to internal.__#1 which causes infinite loop when we traverse later. (and there is no js object for seen to keep track of here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add the link to seen, but I think you've also added the object at ["internal", "#1"] to seen as well.
I think this is just that we don't handle circular links correctly everywhere (for example, one of my traverse functions is traverseDAG, since it does not handle them).
I think we're supposed to be supporting cycles, but I have no issue with disabling them here if that's the simplest approach.

if (parsedLink.id.startsWith("data:")) {
diffLogger.debug(() =>
`[BRANCH_CELL_LINK] Data link detected, treating as contents at path=${pathStr}`
);
// If there is a data link treat it as writing it's contents instead.

// Use the tx code to make sure we read it the same way
let dataValue: any = runtime.edit().readValueOrThrow({
let dataValue: any = tx.readValueOrThrow({
...parsedLink,
path: [],
}, options);
Expand Down Expand Up @@ -260,8 +369,14 @@ export function normalizeAndDiff(
isAnyCellLink(currentValue) &&
areLinksSame(newValue, currentValue, link)
) {
diffLogger.debug(() =>
`[BRANCH_CELL_LINK] Same cell link, no-op at path=${pathStr}`
);
return [];
} else {
diffLogger.debug(() =>
`[BRANCH_CELL_LINK] Different cell link, updating at path=${pathStr}`
);
return [
// TODO(seefeld): Normalize the link to a sigil link?
{ location: link, value: newValue as JSONValue },
Expand All @@ -271,6 +386,9 @@ export function normalizeAndDiff(

// Handle ID-based object (convert to entity)
if (isRecord(newValue) && newValue[ID] !== undefined) {
diffLogger.debug(() =>
`[BRANCH_ID_OBJECT] Processing ID-based object at path=${pathStr}`
);
const { [ID]: id, ...rest } = newValue as
& { [ID]: string }
& Record<string, JSONValue>;
Expand Down Expand Up @@ -329,6 +447,9 @@ export function normalizeAndDiff(

// Handle arrays
if (Array.isArray(newValue)) {
diffLogger.debug(() =>
`[BRANCH_ARRAY] Processing array at path=${pathStr} length=${newValue.length}`
);
// If the current value is not an array, set it to an empty array
if (!Array.isArray(currentValue)) {
changes.push({ location: link, value: [] });
Expand Down Expand Up @@ -385,9 +506,15 @@ export function normalizeAndDiff(

// Handle objects
if (isRecord(newValue)) {
diffLogger.debug(() =>
`[BRANCH_OBJECT] Processing object at path=${pathStr}`
);
// If the current value is not a (regular) object, set it to an empty object
// Note that the alias case is handled above
if (!isRecord(currentValue) || isAnyCellLink(currentValue)) {
diffLogger.debug(() =>
`[BRANCH_OBJECT] Current value is not a record or cell link, setting to empty object at path=${pathStr}`
);
changes.push({ location: link, value: {} });
currentValue = {};
}
Expand All @@ -396,6 +523,11 @@ export function normalizeAndDiff(
seen.set(newValue, link);

for (const key in newValue) {
diffLogger.debug(() => {
const childPath = [...link.path, key].join(".");
return `[DIFF_RECURSE] Recursing into key='${key}' childPath=${childPath}`;
});

const childSchema = runtime.cfc.getSchemaAtPath(
link.schema,
[key],
Expand Down