From 196a29dc6efc6a4ee94fa8a2ebacc61fdf95bfdd Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 05:17:18 +0800 Subject: [PATCH 1/3] fix(loro.js): treat containers under deleted ancestors as deleted isDeleted() only looked at a container's own parent binding, and a tree node only at its own deleted flag. A child of a removed map value, or a node under a deleted tree node, therefore counted as alive: it kept a path, getNodes() listed it, and local edits to it recorded ops on invisible state. Rust's DocState::is_deleted follows the whole chain and rejects such edits with ContainerDeleted. Walk the parent chain (and tree ancestors) for isDeleted, getPathToContainer, and a new guard in #appendAndApply that throws Rust's message. getContainerById keeps its one-level check, since Rust still resolves those IDs. nodes() now returns every node and getNodes() lists alive nodes breadth-first, as Rust does. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/loro-js-deleted-ancestors.md | 5 ++ loro-js/src/runtime/containers.ts | 44 ++++++++++-- loro-js/src/runtime/document.ts | 32 ++++++++- loro-js/tests/deleted-containers.test.ts | 92 ++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 8 deletions(-) create mode 100644 .changeset/loro-js-deleted-ancestors.md create mode 100644 loro-js/tests/deleted-containers.test.ts diff --git a/.changeset/loro-js-deleted-ancestors.md b/.changeset/loro-js-deleted-ancestors.md new file mode 100644 index 000000000..08b94aa1c --- /dev/null +++ b/.changeset/loro-js-deleted-ancestors.md @@ -0,0 +1,5 @@ +--- +"loro.js": patch +--- + +`loro.js` now treats a container as deleted when any ancestor is deleted, as `loro-crdt` does. `isDeleted()` and `getPathToContainer()` account for deleted ancestors, and descendants of a deleted tree node are deleted: `getNodes()` omits them, and `isNodeDeleted()`/`LoroTreeNode.isDeleted()` return `true`. Local edits to a deleted container now throw `The container … is deleted. You cannot apply the op on a deleted container.` instead of recording ops on invisible state. `LoroTree.nodes()` returns every node, including deleted ones, and `getNodes()` lists alive nodes breadth-first from the roots, matching Rust. diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index 9b3979b91..98c08d28d 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -2267,7 +2267,20 @@ export class LoroTree< isNodeDeleted(target: TreeID): boolean { this._ensureHydrated(); - return this._nodes.get(target)?.deleted ?? false; + const record = this._nodes.get(target); + return record !== undefined && this._isNodeHidden(record); + } + + /** Whether the node or one of its ancestors is deleted. */ + _isNodeHidden(record: TreeNodeRecord): boolean { + let current: TreeNodeRecord | undefined = record; + // A parent chain longer than the node count can only be a cycle. + for (let steps = 0; steps <= this._nodes.size; steps += 1) { + if (current === undefined || current.deleted) return true; + if (current.parent === undefined) return false; + current = this._nodes.get(formatTreeId(current.parent)); + } + return true; } enableFractionalIndex(jitter = 0): void { @@ -2291,15 +2304,34 @@ export class LoroTree< return record === undefined ? undefined : new LoroTreeNode(this, record.id); } + /** + * Alive nodes in breadth-first order from the roots, like Rust. With + * `withDeleted`, the deleted nodes and their subtrees follow. + */ getNodes(options: { withDeleted?: boolean } = {}): LoroTreeNode[] { this._ensureHydrated(); - return [...this._nodes.values()] - .filter((record) => options.withDeleted === true || !record.deleted) - .map((record) => new LoroTreeNode(this, record.id)); + const records = this._subtreeRecords(this._childrenOf(undefined)); + if (options.withDeleted === true) { + const deleted = [...this._nodes.values()].filter((record) => record.deleted); + records.push(...this._subtreeRecords(deleted)); + } + return records.map((record) => new LoroTreeNode(this, record.id)); } + /** Every node, including deleted ones, as in Rust. */ nodes(): LoroTreeNode[] { - return this.getNodes(); + this._ensureHydrated(); + return [...this._nodes.values()].map( + (record) => new LoroTreeNode(this, record.id), + ); + } + + _subtreeRecords(starts: readonly TreeNodeRecord[]): TreeNodeRecord[] { + const output = [...starts]; + for (let index = 0; index < output.length; index += 1) { + output.push(...this._childrenOf(output[index]!.id)); + } + return output; } roots(): LoroTreeNode[] { @@ -2565,7 +2597,7 @@ export class LoroTreeNode = Record = Record = Record = Record = Record { + test("treat the subtree of a deleted tree node as deleted", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const parent = tree.createNode(); + const child = tree.createNode(parent.id); + const grandchild = tree.createNode(child.id); + const other = tree.createNode(); + doc.commit(); + tree.delete(parent.id); + doc.commit(); + + expect(tree.getNodes().map((node) => node.id)).toEqual([other.id]); + expect( + tree + .getNodes({ withDeleted: true }) + .map((node) => node.id) + .sort(), + ).toEqual([parent.id, child.id, grandchild.id, other.id].sort()); + expect( + tree + .nodes() + .map((node) => node.id) + .sort(), + ).toEqual([parent.id, child.id, grandchild.id, other.id].sort()); + expect(tree.isNodeDeleted(parent.id)).toBe(true); + expect(tree.isNodeDeleted(child.id)).toBe(true); + expect(tree.getNodeByID(grandchild.id)!.isDeleted()).toBe(true); + expect(tree.getNodeByID(other.id)!.isDeleted()).toBe(false); + expect(grandchild.data.isDeleted()).toBe(true); + expect(() => grandchild.data.set("key", 1)).toThrow(/is deleted/); + + // Moving a node out of the deleted subtree revives it and its children. + // (loro-crdt 1.16 keeps the metadata of a node it already reported as + // deleted marked deleted after such a local move; a reload clears it.) + tree.move(child.id, undefined, 0); + doc.commit(); + expect(tree.isNodeDeleted(grandchild.id)).toBe(false); + grandchild.data.set("key", 1); + doc.commit(); + expect(tree.getNodes().map((node) => node.id)).toEqual([ + child.id, + other.id, + grandchild.id, + ]); + }); + + test("treat children of a removed map value as deleted", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const root = doc.getMap("root"); + const map = root.setContainer("map", new LoroMap()); + const empty = map.setContainer("empty", new LoroText()); + const text = map.setContainer("text", new LoroText()); + text.insert(0, "x"); + doc.commit(); + root.delete("map"); + doc.commit(); + + for (const container of [map, empty, text]) { + expect(container.isDeleted()).toBe(true); + expect(doc.getPathToContainer(container.id)).toBeUndefined(); + expect(doc.getContainerById(container.id)).toBeDefined(); + } + expect(() => text.insert(0, "y")).toThrow( + `The container ${text.id} is deleted. You cannot apply the op on a deleted container.`, + ); + expect(() => empty.insert(0, "y")).toThrow(/is deleted/); + expect(doc.getPendingTxnLength()).toBe(0); + expect(text.toString()).toBe("x"); + }); + + test("rejects edits to a list child removed from its list", () => { + const doc = new LoroDoc(); + const list = doc.getList("list"); + const child = list.insertContainer(0, new LoroMap()); + doc.commit(); + list.delete(0, 1); + doc.commit(); + expect(child.isDeleted()).toBe(true); + expect(() => child.set("key", 1)).toThrow(/is deleted/); + }); +}); From 03dc8dca96c4231a47b7507618ae04c9ff1d8568 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Tue, 29 Sep 2026 19:21:41 +0800 Subject: [PATCH 2/3] fix(loro.js): keep deleted-container checks off #1131's reachability path Keep main's one-level _isContainerDeleted for getContainerById and the diff/event reachability walk, and use the ancestor walk only in isDeleted(), getPathToContainer() and getCursorPos(). Drop the guard on local edits to deleted containers: it threw on a redo that main allows. Co-Authored-By: Claude Opus 5.5 --- .changeset/loro-js-deleted-ancestors.md | 2 +- loro-js/src/runtime/containers.ts | 2 +- loro-js/src/runtime/document.ts | 42 ++++++++++++------------ loro-js/tests/deleted-containers.test.ts | 17 ++++------ 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/.changeset/loro-js-deleted-ancestors.md b/.changeset/loro-js-deleted-ancestors.md index 08b94aa1c..37a053c43 100644 --- a/.changeset/loro-js-deleted-ancestors.md +++ b/.changeset/loro-js-deleted-ancestors.md @@ -2,4 +2,4 @@ "loro.js": patch --- -`loro.js` now treats a container as deleted when any ancestor is deleted, as `loro-crdt` does. `isDeleted()` and `getPathToContainer()` account for deleted ancestors, and descendants of a deleted tree node are deleted: `getNodes()` omits them, and `isNodeDeleted()`/`LoroTreeNode.isDeleted()` return `true`. Local edits to a deleted container now throw `The container … is deleted. You cannot apply the op on a deleted container.` instead of recording ops on invisible state. `LoroTree.nodes()` returns every node, including deleted ones, and `getNodes()` lists alive nodes breadth-first from the roots, matching Rust. +`loro.js` now treats a container as deleted when any ancestor is deleted, as `loro-crdt` does: `isDeleted()`, `getPathToContainer()`, and `getCursorPos()` account for deleted ancestors. Descendants of a deleted tree node are deleted: `getNodes()` omits them, and `isNodeDeleted()`/`LoroTreeNode.isDeleted()` return `true`. `LoroTree.nodes()` returns every node, including deleted ones, and `getNodes()` lists alive nodes breadth-first from the roots, matching Rust. diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index c475a1d1b..59d1b06de 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -149,7 +149,7 @@ export abstract class LoroContainer { } isDeleted(): boolean { - return this._doc?._isContainerDeleted(this) ?? false; + return this._doc?._isContainerUnreachable(this) ?? false; } subscribe(listener: (event: LoroEventBatch) => void): Subscription { diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index 848ab7fd2..1607c756e 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -460,7 +460,7 @@ export class LoroDoc = Record = Record = Record = Record = Record = Record { test("treat the subtree of a deleted tree node as deleted", () => { @@ -37,7 +38,7 @@ describe("deleted containers and tree nodes", () => { expect(tree.getNodeByID(grandchild.id)!.isDeleted()).toBe(true); expect(tree.getNodeByID(other.id)!.isDeleted()).toBe(false); expect(grandchild.data.isDeleted()).toBe(true); - expect(() => grandchild.data.set("key", 1)).toThrow(/is deleted/); + expect(doc.getPathToContainer(grandchild.data.id)).toBeUndefined(); // Moving a node out of the deleted subtree revives it and its children. // (loro-crdt 1.16 keeps the metadata of a node it already reported as @@ -71,22 +72,18 @@ describe("deleted containers and tree nodes", () => { expect(doc.getPathToContainer(container.id)).toBeUndefined(); expect(doc.getContainerById(container.id)).toBeDefined(); } - expect(() => text.insert(0, "y")).toThrow( - `The container ${text.id} is deleted. You cannot apply the op on a deleted container.`, - ); - expect(() => empty.insert(0, "y")).toThrow(/is deleted/); - expect(doc.getPendingTxnLength()).toBe(0); - expect(text.toString()).toBe("x"); }); - test("rejects edits to a list child removed from its list", () => { + test("treats a list child removed from its list as deleted", () => { const doc = new LoroDoc(); const list = doc.getList("list"); const child = list.insertContainer(0, new LoroMap()); + const grandchild = child.setContainer("text", new LoroText()); doc.commit(); list.delete(0, 1); doc.commit(); expect(child.isDeleted()).toBe(true); - expect(() => child.set("key", 1)).toThrow(/is deleted/); + expect(grandchild.isDeleted()).toBe(true); + expect(doc.getPathToContainer(grandchild.id)).toBeUndefined(); }); }); From c783b882c9b59993a2d8041c9c3fecc8c50d5629 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Tue, 29 Sep 2026 21:28:55 +0800 Subject: [PATCH 3/3] fix(loro.js): list tree nodes without spreading child lists into push() getNodes() spread each child list, and with withDeleted the deleted subtrees, into one push() call, which throws a RangeError past V8's argument limit (about 125k on Node's main thread). Co-Authored-By: Claude Opus 5.5 --- loro-js/src/runtime/containers.ts | 4 +-- loro-js/tests/deleted-containers.test.ts | 34 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index 59d1b06de..e24f9ffe8 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -2962,7 +2962,7 @@ export class LoroTree< const records = this._subtreeRecords(this._childrenOf(undefined)); if (options.withDeleted === true) { const deleted = [...this._nodes.values()].filter((record) => record.deleted); - records.push(...this._subtreeRecords(deleted)); + for (const record of this._subtreeRecords(deleted)) records.push(record); } return records.map((record) => new LoroTreeNode(this, record.id)); } @@ -2978,7 +2978,7 @@ export class LoroTree< _subtreeRecords(starts: readonly TreeNodeRecord[]): TreeNodeRecord[] { const output = [...starts]; for (let index = 0; index < output.length; index += 1) { - output.push(...this._childrenOf(output[index]!.id)); + for (const child of this._childrenOf(output[index]!.id)) output.push(child); } return output; } diff --git a/loro-js/tests/deleted-containers.test.ts b/loro-js/tests/deleted-containers.test.ts index c6add2204..12f226711 100644 --- a/loro-js/tests/deleted-containers.test.ts +++ b/loro-js/tests/deleted-containers.test.ts @@ -86,4 +86,38 @@ describe("deleted containers and tree nodes", () => { expect(grandchild.isDeleted()).toBe(true); expect(doc.getPathToContainer(grandchild.id)).toBeUndefined(); }); + + // V8 caps the arguments of one call (about 125k on Node's main thread), so + // spreading a child list into push() threw a RangeError for a node with that + // many children, or that many deleted nodes with `withDeleted`. Building + // such a tree is too slow for CI, so check the widest push() at 2,000. + test("lists nodes without passing a whole child list to one call", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const parent = tree.createNode(); + for (let index = 0; index < 2_000; index += 1) tree.createNode(parent.id); + doc.commit(); + const widestPush = (list: () => unknown[]): number => { + const push = Array.prototype.push; + let widest = 0; + Array.prototype.push = function (this: unknown[], ...items: unknown[]) { + widest = Math.max(widest, items.length); + return push.apply(this, items); + }; + try { + list(); + } finally { + Array.prototype.push = push; + } + return widest; + }; + + expect(tree.getNodes()).toHaveLength(2_001); + expect(widestPush(() => tree.getNodes())).toBeLessThan(100); + tree.delete(parent.id); + doc.commit(); + expect(tree.getNodes({ withDeleted: true })).toHaveLength(2_001); + expect(widestPush(() => tree.getNodes({ withDeleted: true }))).toBeLessThan(100); + }); });