fix(tree): stop silent page loss on move-to-unloaded-parent + reconnect ghost roots (#159)
Two confirmed P1 data-loss findings in the sidebar tree sync. #1 — Move into an unloaded/collapsed parent silently dropped pages. When a moveTreeNode (or addTreeNode) broadcast targeted a parent whose children were NOT yet lazy-loaded, `insertByPosition` did `kids = parent.children ?? []` and inserted the moved node, MATERIALIZING a misleading partial child list (`[movedNode]`) out of an unloaded (`children === undefined`) parent. The lazy-load gate fetches only when children are absent/empty, so it then refused to fetch — leaving the parent showing ONLY the moved node and HIDING all its other real children (and, when the parent wasn't in the tree at all, the node was removed and never re-fetched). Fix: `insertByPosition` distinguishes `children === undefined` (not loaded) from `[]` (loaded-empty) and, for an unloaded parent, does NOT insert — it leaves children unloaded and just flags `hasChildren`, so expanding fetches the FULL set (including the moved/added node) via the existing lazy-load. #2 — After a socket reconnect, a deleted/moved-away root lingered as a 404 "ghost". `mergeRootTrees` was append-only: it kept every previously-loaded root and only added new ones, so a root removed during the missed-events gap was never dropped. It runs only once all root pages are fetched, so the incoming list is the authoritative complete root set — fix reconciles to it (drop roots absent from incoming) while PRESERVING each surviving root's lazy-loaded subtree and refreshing its own fields. Tests: insertByPosition unloaded-vs-loaded-empty parent; the move reducer keeps a collapsed destination lazy-loadable instead of partial; mergeRootTrees drops a ghost root, preserves a surviving subtree, adds new roots, refreshes fields. The existing "remove when parent not in tree" reducer test still holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import type { TreeNode, SiblingsInfo } from './tree-model.types';
|
||||
import type { TreeNode, SiblingsInfo } from "./tree-model.types";
|
||||
|
||||
function findInternal<T extends object>(
|
||||
nodes: TreeNode<T>[],
|
||||
@@ -19,7 +19,10 @@ export const treeModel = {
|
||||
return findInternal(tree, id)?.node ?? null;
|
||||
},
|
||||
|
||||
path<T extends object>(tree: TreeNode<T>[], id: string): TreeNode<T>[] | null {
|
||||
path<T extends object>(
|
||||
tree: TreeNode<T>[],
|
||||
id: string,
|
||||
): TreeNode<T>[] | null {
|
||||
const found = findInternal(tree, id);
|
||||
if (!found) return null;
|
||||
return [...found.parents, found.node];
|
||||
@@ -123,6 +126,23 @@ export const treeModel = {
|
||||
return treeModel.insert(tree, null, node, index(tree));
|
||||
}
|
||||
const parent = treeModel.find(tree, parentId);
|
||||
// The parent is in the tree but its children have NOT been lazy-loaded yet
|
||||
// (`children === undefined`, distinct from a loaded-but-empty `[]`). Inserting
|
||||
// here would MATERIALIZE a misleading partial child list (`[node]`) that
|
||||
// defeats the lazy-load gate — which fetches only when children are
|
||||
// absent/empty — so the parent's OTHER real children would never load and the
|
||||
// moved/added node would be the only one shown (a silent data loss, #159 #1).
|
||||
// Instead, leave the children unloaded and just flag `hasChildren` so the
|
||||
// chevron appears; expanding fetches the FULL set (including this node).
|
||||
if (parent && parent.children === undefined) {
|
||||
return treeModel.update(
|
||||
tree,
|
||||
parentId,
|
||||
// hasChildren is not part of the generic T constraint; tree nodes carry
|
||||
// it. Cast narrowly so this stays a single, well-understood exception.
|
||||
{ hasChildren: true } as unknown as Omit<Partial<T>, "id" | "children">,
|
||||
);
|
||||
}
|
||||
const kids = (parent?.children as TreeNode<T>[] | undefined) ?? [];
|
||||
return treeModel.insert(tree, parentId, node, index(kids));
|
||||
},
|
||||
@@ -242,9 +262,10 @@ export const treeModel = {
|
||||
move<T extends object>(
|
||||
tree: TreeNode<T>[],
|
||||
sourceId: string,
|
||||
op: import('./tree-model.types').DropOp,
|
||||
): { tree: TreeNode<T>[]; result: import('./tree-model.types').DropResult } {
|
||||
if (sourceId === op.targetId) return { tree, result: { parentId: null, index: 0 } };
|
||||
op: import("./tree-model.types").DropOp,
|
||||
): { tree: TreeNode<T>[]; result: import("./tree-model.types").DropResult } {
|
||||
if (sourceId === op.targetId)
|
||||
return { tree, result: { parentId: null, index: 0 } };
|
||||
if (!treeModel.find(tree, sourceId) || !treeModel.find(tree, op.targetId)) {
|
||||
return { tree, result: { parentId: null, index: 0 } };
|
||||
}
|
||||
@@ -255,7 +276,7 @@ export const treeModel = {
|
||||
let parentId: string | null;
|
||||
let index: number;
|
||||
|
||||
if (op.kind === 'make-child') {
|
||||
if (op.kind === "make-child") {
|
||||
parentId = op.targetId;
|
||||
const target = treeModel.find(tree, op.targetId)!;
|
||||
index = target.children?.length ?? 0;
|
||||
@@ -264,9 +285,8 @@ export const treeModel = {
|
||||
parentId = info.parentId;
|
||||
const sourceInfo = treeModel.siblingsOf(tree, sourceId)!;
|
||||
const sameParent = sourceInfo.parentId === parentId;
|
||||
const adjust =
|
||||
sameParent && sourceInfo.index < info.index ? -1 : 0;
|
||||
index = info.index + adjust + (op.kind === 'reorder-after' ? 1 : 0);
|
||||
const adjust = sameParent && sourceInfo.index < info.index ? -1 : 0;
|
||||
index = info.index + adjust + (op.kind === "reorder-after" ? 1 : 0);
|
||||
}
|
||||
|
||||
const next = treeModel.place(tree, sourceId, { parentId, index });
|
||||
|
||||
Reference in New Issue
Block a user