From 4a59ec4277f5b74fef039fda291f6405d5258d89 Mon Sep 17 00:00:00 2001 From: Yordan Stoyanov Date: Tue, 15 Sep 2026 16:33:56 +0200 Subject: [PATCH 1/5] fix(tree-node-web): stop LOADING state from getting stuck or corrupting sibling nodes --- .../src/components/v2/TreeNode.tsx | 11 +- .../v2/__tests__/TreeNodeV2.spec.tsx | 227 +++++++++++++----- .../__tests__/useIncrementalTreeData.spec.ts | 32 ++- .../v2/hooks/useIncrementalTreeData.ts | 8 +- 4 files changed, 201 insertions(+), 77 deletions(-) diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx index 24ed6171ba..a915bf8a6d 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx @@ -15,9 +15,13 @@ function renderRecursiveNode( iconPlacement: TreeNodeContainerProps["showIcon"], openNodeOn: TreeNodeContainerProps["openNodeOn"], onNodeClick: (node: TreeNodeV2DataItem) => void, + isDatasourceLoading: boolean, children?: TreeNodeContainerProps["children"] ): ReactElement { const hasChildren = node.children.length > 0; + // We don't yet know whether this node has children (nothing placed under it yet); show a + // spinner only while the datasource is actually fetching, never as a stored/stale node state. + const showSpinner = !hasChildren && isDatasourceLoading; const isExpanded = node.treeNodeState === TreeNodeState.EXPANDED; const isIconClickable = openNodeOn === "iconClick"; const isHeaderClickable = openNodeOn === "headerClick"; @@ -46,14 +50,14 @@ function renderRecursiveNode( onClick={onHeaderClick} > {node.title} - {(hasChildren || node.treeNodeState === TreeNodeState.LOADING) && iconPlacement !== "no" && ( + {(hasChildren || showSpinner) && iconPlacement !== "no" && ( - {renderHeaderIcon(node.treeNodeState, iconPlacement)} + {renderHeaderIcon(showSpinner ? TreeNodeState.LOADING : node.treeNodeState, iconPlacement)} )} @@ -75,6 +79,7 @@ function renderRecursiveNode( iconPlacement, openNodeOn, onNodeClick, + isDatasourceLoading, children )} @@ -120,6 +125,7 @@ export function TreeNodeV2(props: TreeNodeContainerProps): ReactElement { ); const treeData = useIncrementalTreeData(items, treeConfig); + const isDatasourceLoading = props.datasource.status === ValueStatus.Loading; const onNodeClick = useCallback( (node: TreeNodeV2DataItem) => { if (node.treeNodeState === TreeNodeState.EXPANDED) { @@ -160,6 +166,7 @@ export function TreeNodeV2(props: TreeNodeContainerProps): ReactElement { iconPlacement, props.openNodeOn, onNodeClick, + isDatasourceLoading, props.children ) )} diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx index 5e378b30d7..474c09d60b 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx @@ -20,72 +20,72 @@ jest.mock("mendix/filters/builders", () => ({ or: jest.fn((...args: unknown[]) => ({ type: "or", args })) })); -describe("TreeNodeV2 - Keyboard Navigation", () => { - const makeItem = (id: string): ObjectItem => ({ id: id as GUID }); - - const makeListValue = (items: ObjectItem[]): ListValue => - ({ - status: ValueStatus.Available, - items, - limit: 100, - offset: 0, - hasMoreItems: false, - sortOrder: [], - filter: undefined, - setLimit: jest.fn(), - setOffset: jest.fn(), - setSortOrder: jest.fn(), - requestTotalCount: jest.fn(), - setFilter: jest.fn(), - reload: jest.fn(), - totalCount: undefined - }) as unknown as ListValue; - - const makeExpression = (value: string): ListExpressionValue => ({ - get: (): DynamicValue => ({ status: ValueStatus.Available, value }) - }); +const makeItem = (id: string): ObjectItem => ({ id: id as GUID }); + +const makeListValue = (items: ObjectItem[]): ListValue => + ({ + status: ValueStatus.Available, + items, + limit: 100, + offset: 0, + hasMoreItems: false, + sortOrder: [], + filter: undefined, + setLimit: jest.fn(), + setOffset: jest.fn(), + setSortOrder: jest.fn(), + requestTotalCount: jest.fn(), + setFilter: jest.fn(), + reload: jest.fn(), + totalCount: undefined + }) as unknown as ListValue; + +const makeExpression = (value: string): ListExpressionValue => ({ + get: (): DynamicValue => ({ status: ValueStatus.Available, value }) +}); - const makeBoolExpression = (value: boolean): ListExpressionValue => ({ - get: (): DynamicValue => ({ status: ValueStatus.Available, value }) - }); +const makeBoolExpression = (value: boolean): ListExpressionValue => ({ + get: (): DynamicValue => ({ status: ValueStatus.Available, value }) +}); - /** - * Creates a ListReferenceValue mock where childId → parentId, all others → undefined. - */ - const makeParentAssociation = (childId: string, parentId: string): ListReferenceValue => - ({ - id: "parentAssoc", - type: "Reference", - get: (item: ObjectItem): DynamicValue => { - if (String(item.id) === childId) { - return { status: ValueStatus.Available, value: makeItem(parentId) }; - } - return { status: ValueStatus.Available, value: undefined as unknown as ObjectItem }; +/** + * Creates a ListReferenceValue mock where childId → parentId, all others → undefined. + */ +const makeParentAssociation = (childId: string, parentId: string): ListReferenceValue => + ({ + id: "parentAssoc", + type: "Reference", + get: (item: ObjectItem): DynamicValue => { + if (String(item.id) === childId) { + return { status: ValueStatus.Available, value: makeItem(parentId) }; } - }) as unknown as ListReferenceValue; - - /** - * Default props for tests that need a node with children. - * Datasource contains parent + child; parentAssociation links child → parent. - * This makes node.children.length > 0 so aria-expanded is rendered. - */ - const makeDefaultProps = (startExpanded = false): TreeNodeContainerProps => ({ - name: "treeNode", - class: "", - tabIndex: 0, - advancedMode: false, - datasource: makeListValue([makeItem("1"), makeItem("2")]), - parentAssociation: makeParentAssociation("2", "1"), - headerType: "text", - headerCaption: makeExpression("Node"), - hasChildren: makeBoolExpression(true), - showIcon: "right", - openNodeOn: "headerClick", - animate: false, - animateIcon: false, - startExpanded - }); + return { status: ValueStatus.Available, value: undefined as unknown as ObjectItem }; + } + }) as unknown as ListReferenceValue; + +/** + * Default props for tests that need a node with children. + * `hasChildren` (not the datasource) is what drives aria-expanded; the + * datasource's parent + child items exist so the expanded body actually renders content. + */ +const makeDefaultProps = (startExpanded = false): TreeNodeContainerProps => ({ + name: "treeNode", + class: "", + tabIndex: 0, + advancedMode: false, + datasource: makeListValue([makeItem("1"), makeItem("2")]), + parentAssociation: makeParentAssociation("2", "1"), + headerType: "text", + headerCaption: makeExpression("Node"), + hasChildren: makeBoolExpression(true), + showIcon: "right", + openNodeOn: "headerClick", + animate: false, + animateIcon: false, + startExpanded +}); +describe("TreeNodeV2 - Keyboard Navigation", () => { it("expands node when Enter key is pressed", () => { render(createElement(TreeNodeV2, makeDefaultProps(false))); const treeItem = screen.getAllByRole("treeitem")[0]; @@ -204,3 +204,106 @@ describe("TreeNodeV2 - Keyboard Navigation", () => { expect(parentItem.getAttribute("aria-expanded")).toBe(initialState); }); }); + +describe("TreeNodeV2 - Loading state (WC-3564 regressions)", () => { + const spinner = (container: HTMLElement): Element | null => + container.querySelector(".widget-tree-node-loading-spinner"); + + const makeListValueWithStatus = (items: ObjectItem[], status: ValueStatus): ListValue => + ({ ...makeListValue(items), status }) as unknown as ListValue; + + it("never shows a stuck spinner, even when the datasource keeps redelivering the same full item set (Bug 1)", () => { + // "1" genuinely has a child ("2"), so its expand affordance should resolve immediately, not depend on a later delivery. + const props: TreeNodeContainerProps = { + ...makeDefaultProps(true), + datasource: makeListValue([makeItem("1"), makeItem("2")]), + parentAssociation: makeParentAssociation("2", "1") + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).toBeNull(); + expect(screen.getAllByRole("treeitem")[0]).toHaveAttribute("aria-expanded", "true"); + + // Simulate a microflow datasource ignoring setFilter and redelivering + // the exact same full result on a later render (new array reference). + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValue([makeItem("1"), makeItem("2")]) + }) + ); + + expect(spinner(container)).toBeNull(); + expect(screen.getAllByRole("treeitem")[0]).toHaveAttribute("aria-expanded", "true"); + }); + + it("shows a spinner while the datasource is actually loading and no children are known yet", () => { + const noParent = makeParentAssociation("__none__", "__none__"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Loading), + parentAssociation: noParent + }; + + const { container } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).not.toBeNull(); + expect(screen.getByRole("treeitem")).not.toHaveAttribute("aria-expanded"); + }); + + it("clears the spinner once the datasource settles, even if it turns out the node has no children", () => { + const noParent = makeParentAssociation("__none__", "__none__"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Loading), + parentAssociation: noParent + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).not.toBeNull(); + + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Available) + }) + ); + + expect(spinner(container)).toBeNull(); + expect(screen.getByRole("treeitem")).not.toHaveAttribute("aria-expanded"); + }); + + it("resolving one node's children does not affect an unrelated sibling's spinner or state (Bug 2)", () => { + const parentAssociation = makeParentAssociation("C", "A"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("A"), makeItem("B")], ValueStatus.Loading), + parentAssociation + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + const [nodeA, nodeB] = screen.getAllByRole("treeitem"); + expect(nodeA).not.toHaveAttribute("aria-expanded"); + expect(nodeB).not.toHaveAttribute("aria-expanded"); + // Both spin while nothing is known yet and the datasource is loading. + expect(container.querySelectorAll(".widget-tree-node-loading-spinner")).toHaveLength(2); + + // The datasource settles, delivering a child for A only. B was never involved. + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValueWithStatus( + [makeItem("A"), makeItem("B"), makeItem("C")], + ValueStatus.Available + ) + }) + ); + + const [nodeAAfter, nodeBAfter] = screen.getAllByRole("treeitem"); + expect(nodeAAfter).toHaveAttribute("aria-expanded", "false"); + expect(nodeAAfter.querySelector(".widget-tree-node-branch-header-icon-container")).not.toBeNull(); + // B has no children and the datasource is no longer loading — no icon, no spinner, untouched by A's resolution. + expect(nodeBAfter).not.toHaveAttribute("aria-expanded"); + expect(nodeBAfter.querySelector(".widget-tree-node-branch-header-icon-container")).toBeNull(); + expect(container.querySelectorAll(".widget-tree-node-loading-spinner")).toHaveLength(0); + }); +}); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts index 598aeff794..27ef1471fd 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts @@ -70,7 +70,7 @@ describe("useIncrementalTreeData", () => { expect(result.current[0].children[0].id).toBe("child"); }); - it("assigns LOADING on first render, then COLLAPSED_WITH_JS when startExpanded is false", () => { + it("assigns COLLAPSED_WITH_JS on first render when startExpanded is false, and never enters LOADING on redelivery (WC-3564 Bug 1)", () => { const items = [makeItem("a")]; const config = makeConfig({ startExpanded: false }); const { result, rerender } = renderHook( @@ -78,13 +78,13 @@ describe("useIncrementalTreeData", () => { useIncrementalTreeData(items, config), { initialProps: { items, config } } ); - expect(result.current[0].treeNodeState).toBe(TreeNodeState.LOADING); - // Simulate Mendix re-providing items (new array reference) + expect(result.current[0].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + // Simulate a microflow datasource redelivering the same full result (new array reference). rerender({ items: [...items], config }); expect(result.current[0].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); }); - it("assigns LOADING on first render, then EXPANDED when startExpanded is true", () => { + it("assigns EXPANDED on first render when startExpanded is true, and stays EXPANDED on redelivery (WC-3564 Bug 1)", () => { const items = [makeItem("a")]; const config = makeConfig({ startExpanded: true }); const { result, rerender } = renderHook( @@ -92,11 +92,31 @@ describe("useIncrementalTreeData", () => { useIncrementalTreeData(items, config), { initialProps: { items, config } } ); - expect(result.current[0].treeNodeState).toBe(TreeNodeState.LOADING); - // Simulate Mendix re-providing items (new array reference) + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + // Simulate a microflow datasource redelivering the same full result (new array reference). rerender({ items: [...items], config }); expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); }); + + it("leaves an unrelated sibling's state untouched when a node's children arrive later (WC-3564 Bug 2)", () => { + const parent = makeItem("parent"); + const sibling = makeItem("sibling"); + const config = makeConfigWithParentMap({ child: "parent" }, { startExpanded: false }); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: [parent, sibling] } } + ); + + expect(result.current.find(n => n.id === "parent")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + expect(result.current.find(n => n.id === "sibling")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + + // "parent"'s child arrives later; "sibling" was never involved. + rerender({ items: [parent, sibling, makeItem("child")] }); + expect(result.current.find(n => n.id === "parent")!.children).toHaveLength(1); + expect(result.current.find(n => n.id === "sibling")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + expect(result.current.find(n => n.id === "sibling")!.children).toHaveLength(0); + }); }); describe("out-of-order arrival (child before parent)", () => { diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts index bbf5f4721e..27c42b4146 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts @@ -111,12 +111,6 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: placeNode(existingNode); } - if (existingNode.treeNodeState === TreeNodeState.LOADING) { - existingNode.treeNodeState = config.startExpanded - ? TreeNodeState.EXPANDED - : TreeNodeState.COLLAPSED_WITH_JS; - nodesByIdRef.current.set(nodeId, existingNode); - } continue; } @@ -125,7 +119,7 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: id: nodeId, item, parentId: nextParentId, - treeNodeState: TreeNodeState.LOADING, + treeNodeState: config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS, title: nextTitle }; nodesByIdRef.current.set(nodeId, newNode); From 1c6dd5ed3b065a158d255351c0301e931aee9c92 Mon Sep 17 00:00:00 2001 From: Yordan Stoyanov Date: Tue, 15 Sep 2026 16:39:10 +0200 Subject: [PATCH 2/5] fix(tree-node-web): preload grandchildren on a node's first expand --- .../v2/hooks/useInfiniteTreeNode.ts | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts index 3d712423e2..f36f5e552b 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts @@ -38,25 +38,23 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { (newItem: ObjectItem, children?: ObjectItem[]) => { const parentId = getItemId(newItem); - if (loadedParentsByIdRef.current.has(parentId)) { - if (children && children.length > 0) { - children.forEach(child => { - const childId = getItemId(child); - // get all expanded node's children Id, in order to pre-load them - // this is needed to be able to know if a node has further level children before expanding it. - loadedChildsByIdRef.current.set(childId, child); - }); + if (children && children.length > 0) { + children.forEach(child => { + const childId = getItemId(child); + // get all expanded node's children Id, in order to pre-load them + // this is needed to be able to know if a node has further level children before expanding it. + // Runs on every expand, including the first one — a node's own children being + // preloaded as part of its parent's expand must not delay preloading its grandchildren too. + loadedChildsByIdRef.current.set(childId, child); + }); + } - // if the new item is already in loadedChilds, - // it means that it was pre-loaded as a child of an expanded node, - // so we need to move it to loadedParents - if (loadedChildsByIdRef.current.has(parentId)) { - loadedParentsByIdRef.current.set(parentId, loadedChildsByIdRef.current.get(parentId)!); - loadedChildsByIdRef.current.delete(parentId); - } else { - loadedParentsByIdRef.current.set(parentId, newItem); - } - } + // if the new item is already in loadedChilds, + // it means that it was pre-loaded as a child of an expanded node, + // so we need to move it to loadedParents + if (loadedChildsByIdRef.current.has(parentId)) { + loadedParentsByIdRef.current.set(parentId, loadedChildsByIdRef.current.get(parentId)!); + loadedChildsByIdRef.current.delete(parentId); } else { loadedParentsByIdRef.current.set(parentId, newItem); } From acad0d35b48af29e0a48262e876df9a37cdd0a39 Mon Sep 17 00:00:00 2001 From: Yordan Stoyanov Date: Tue, 15 Sep 2026 16:40:48 +0200 Subject: [PATCH 3/5] fix(tree-node-web): cascade auto-expand preload to the tree's real depth --- .../__tests__/useInfiniteTreeNode.spec.ts | 78 +++++++++++++++++++ .../v2/hooks/useInfiniteTreeNode.ts | 67 ++++++++++++++-- 2 files changed, 140 insertions(+), 5 deletions(-) diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts index e46cc2e403..df9b2539ba 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts @@ -183,4 +183,82 @@ describe("useInfiniteTreeNodes", () => { expect(props.datasource.setFilter).toHaveBeenCalledTimes(2); }); }); + + describe("unbounded cascade when startExpanded is true (WC-3564)", () => { + it("keeps cascading level by level for as long as new descendants appear, and stops once a level is empty — never locking in on a transient empty delivery", () => { + const rootItems = [makeItem("root1"), makeItem("root2")]; + const withSecondTier = [...rootItems, makeItem("second1")]; + const withThirdTier = [...withSecondTier, makeItem("third1")]; + let items: ObjectItem[] = []; + const props = makeProps({ startExpanded: true }); + const setFilterSpy = props.datasource.setFilter as jest.Mock; + + const { rerender } = renderHook(() => + useInfiniteTreeNodes({ ...props, datasource: { ...props.datasource, items } as any }) + ); + expect(setFilterSpy).toHaveBeenCalledTimes(0); // startExpanded skips the initial root-only filter + + // Datasource stays empty across a few transient renders (still loading) — must not + // call setFilter on empty data (this is exactly what broke live testing with a naive + // fire-count-based cap instead of a content-based one). + rerender(); + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(0); + + // Real root items arrive — cascades to fetch their children. + items = rootItems; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(1); + + // An unchanged redelivery of the same roots must not trigger another call. + items = rootItems; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(1); + + // Second tier arrives — cascades one level further automatically (no click involved). + items = withSecondTier; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(2); + + // Third tier arrives — keeps cascading (this is the exact scenario that was broken: + // every level defaults to EXPANDED under startExpanded=true, so every level needs its + // own affordance pre-checked, not just roots + one bonus level). + items = withThirdTier; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(3); + + // Nothing new this time (same set redelivered) — stops here, no further call. + items = [...withThirdTier]; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(3); + }); + + it("stays capped at 2 rounds when startExpanded is false — only roots auto-expand, deeper tiers resolve via a real click", () => { + const rootItems = [makeItem("root1")]; + const withSecondTier = [...rootItems, makeItem("second1")]; + const withThirdTier = [...withSecondTier, makeItem("third1")]; + let items: ObjectItem[] = []; + const props = makeProps({ startExpanded: false }); + const setFilterSpy = props.datasource.setFilter as jest.Mock; + + const { rerender } = renderHook(() => + useInfiniteTreeNodes({ ...props, datasource: { ...props.datasource, items } as any }) + ); + expect(setFilterSpy).toHaveBeenCalledTimes(1); // initial root-only filter + + items = rootItems; + rerender(); // round 1 locks in + expect(setFilterSpy).toHaveBeenCalledTimes(2); + + items = withSecondTier; + rerender(); // round 2 locks in + expect(setFilterSpy).toHaveBeenCalledTimes(3); + + // Third tier arriving must NOT trigger a further automatic round — unlike + // startExpanded=true, deeper tiers here only resolve via a real click (appendItems). + items = withThirdTier; + rerender(); + expect(setFilterSpy).toHaveBeenCalledTimes(3); + }); + }); }); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts index f36f5e552b..daaea000b9 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts @@ -16,6 +16,15 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { // loadedChilds : track the pre-loaded nodes of expanded nodes. const loadedChildsByIdRef = useRef>(new Map()); const initializedRef = useRef(false); + // Used only when startExpanded is false (only roots auto-expand; deeper tiers resolve via a + // real click through appendItems). Round 1 (pre-existing): preload roots' children, gated on + // content (loadedParentsByIdRef actually being populated), not on fire-count — so it retries + // harmlessly while the datasource is still empty/loading, and only locks in once real data + // lands. Round 2: once roots' children genuinely arrive, preload one level further for them + // too — same content-based gating, so it can't burn its one shot on a transient empty + // delivery before the real children show up. + const round1DoneRef = useRef(false); + const round2DoneRef = useRef(false); const getDatasourceFilter = useCallback( (items?: ItemType) => { @@ -66,14 +75,60 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { useEffect(() => { if (initializedRef.current) { - // after the first load of the datasource, - // we want to pre-load the child nodes of roots - if (loadedParentsByIdRef.current.size === 0) { + if (startExpanded) { + // Every level defaults to EXPANDED under "Start expanded" = Yes (not just roots), + // so keep treating newly-arrived items as loaded-parents and fetching their + // children, for as long as new descendants keep appearing. Self-terminating: + // once a round finds nothing new, it stops calling setFilter — bounded by the + // tree's real depth, not an arbitrary count. + let addedAny = false; datasource.items?.forEach(item => { - const parentId = getItemId(item); - loadedParentsByIdRef.current.set(parentId, item); + const id = getItemId(item); + if (!loadedParentsByIdRef.current.has(id)) { + loadedParentsByIdRef.current.set(id, item); + addedAny = true; + } }); + if (addedAny) { + datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + } + return; + } + + if (!round1DoneRef.current) { + // after the first load of the datasource, + // we want to pre-load the child nodes of roots + if (loadedParentsByIdRef.current.size === 0) { + datasource.items?.forEach(item => { + const parentId = getItemId(item); + loadedParentsByIdRef.current.set(parentId, item); + }); + } + if (loadedParentsByIdRef.current.size > 0) { + round1DoneRef.current = true; + } datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + return; + } + + if (!round2DoneRef.current) { + // Roots' children have arrived — preload one level further for them too, + // exactly like appendItems does for a manually expanded node, so their own + // expand affordance is known without an extra click. Only advances once real + // (not-yet-tracked) items are actually found, so it can't lock in prematurely + // on a transient empty/unchanged delivery. + let addedAny = false; + datasource.items?.forEach(item => { + const id = getItemId(item); + if (!loadedParentsByIdRef.current.has(id) && !loadedChildsByIdRef.current.has(id)) { + loadedChildsByIdRef.current.set(id, item); + addedAny = true; + } + }); + if (addedAny) { + round2DoneRef.current = true; + datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + } } return; @@ -81,6 +136,8 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { initializedRef.current = true; loadedParentsByIdRef.current.clear(); + round1DoneRef.current = false; + round2DoneRef.current = false; // when datasource is loaded for the first time, we want to load only the root nodes (nodes without parent) // if startExpanded is false, otherwise we want to load all nodes From 7d5352568c9e28d7790f5302d1ff177f1a26a657 Mon Sep 17 00:00:00 2001 From: Yordan Stoyanov Date: Tue, 15 Sep 2026 16:41:27 +0200 Subject: [PATCH 4/5] docs(tree-node-web): update changelog and domain notes for WC-3564 --- .../tree-node-web/CHANGELOG.md | 7 ++ .../pluggableWidgets/tree-node-web/CONTEXT.md | 45 ++++++++++ .../.openspec.yaml | 2 + .../fix-tree-node-loading-state/design.md | 79 +++++++++++++++++ .../fix-tree-node-loading-state/proposal.md | 33 +++++++ .../specs/tree-node-expand-state/spec.md | 85 +++++++++++++++++++ .../fix-tree-node-loading-state/tasks.md | 50 +++++++++++ 7 files changed, 301 insertions(+) create mode 100644 packages/pluggableWidgets/tree-node-web/CONTEXT.md create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/.openspec.yaml create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md diff --git a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md index 62c97f4958..af0a12fce4 100644 --- a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md +++ b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Fixed + +- We fixed an issue where a tree node's loading spinner would never disappear when using a microflow data source with "Start expanded" set to yes. +- We fixed an issue where expanding one node could permanently remove the expand icon from an unrelated, unexpanded node elsewhere in the tree when using a microflow data source. +- We fixed an issue where a node's expand icon for a deeper tier would not appear until that node was collapsed and expanded again. +- We fixed an issue where, with "Start expanded" set to yes, tree nodes deeper than the second level would not show an expand icon until a parent node was manually collapsed and expanded again. + ## [3.11.0] - 2026-05-27 ### Added diff --git a/packages/pluggableWidgets/tree-node-web/CONTEXT.md b/packages/pluggableWidgets/tree-node-web/CONTEXT.md new file mode 100644 index 0000000000..be4902d1ec --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/CONTEXT.md @@ -0,0 +1,45 @@ +# Tree Node widget — domain model + +## v1 vs v2 + +`TreeNode.tsx` (root dispatcher) routes by whether `parentAssociation` is configured: + +- **`parentAssociation` set → v2** (`src/components/v2/`): self-referencing "infinite tree" mode. One widget instance renders the whole tree; each item's own association tells the engine its parent. Added in 3.11.0. +- **`parentAssociation` unset → v1** (`src/components/v1/`): manual-nesting mode. Each tree level is a separately-configured widget instance, nested inside the parent level's "children" slot in Studio Pro. + +### `hasChildren` prop is v1-only, architecturally + +`hasChildren: ListExpressionValue` (XML caption "Has children") exists so a Studio Pro developer can declare per-item whether a node has children, when there's no other way to know (v1: no association, no structural signal). `TreeNode.editorConfig.ts:38-39` hides this property from Studio Pro whenever `parentAssociation` is configured — i.e. whenever v2 is in use. It has no XML default value, so on a v2 widget instance `props.hasChildren` is `undefined` at runtime, not just "possibly misconfigured." **v2 must never read `props.hasChildren` — it will crash.** (Confirmed live during WC-3564: `TypeError: Cannot read properties of undefined (reading 'get')`.) + +For v2, "does this node have children" is derived structurally: `node.children.length > 0`, where `node.children` is populated by matching real association values across the full item set the datasource has delivered so far (`useIncrementalTreeData.ts`). `useInfiniteTreeNode.ts`'s preload mechanism (`loadedChildsByIdRef`) fetches one level ahead of every expand specifically so a node's own children are already known by the time that node is rendered as clickable — this is what keeps `node.children.length > 0` from being stale/late in practice. + +There is no cheaper way to know this without fetching. Mendix's pluggable-widget `ListValue` API is one shared, filter-driven list — no per-item "does this have children" or count-only primitive exists for pluggable widgets. Answering "does node N have children" means asking the datasource for items whose parent is N and seeing what comes back; the preload mechanism exists because that's the only way this API surface allows for v2's configuration mode, not because of a missed optimization. + +## `TreeNodeState.LOADING` (v2) — spinner, not a stored per-node fact (WC-3564) + +Originally (pre-WC-3564) a freshly-created node started in `LOADING` and only left it once its own id reappeared in a _later_ datasource delivery — meant to avoid the expand icon "popping in" late for nodes that would turn out to have children. That resolution criterion was wrong: a microflow datasource always redelivers its full flattened result regardless of the filter passed to `setFilter`, so _any_ later delivery could match, not just one that actually answered a given node's question. Two bugs followed: + +- Bug 1: with "Start expanded" = Yes, no later delivery route existed at all for the frozen microflow case — permanent spinner. +- Bug 2: with "Start expanded" = No, expanding any node caused a full redelivery that could wrongly resolve an unrelated, unexpanded node into a childless state — permanently killing its icon. + +Fix: `LOADING` is no longer stored on the node at all. A node is created directly as `EXPANDED`/`COLLAPSED_WITH_JS` (per `startExpanded`) — never `LOADING`. The spinner is a pure render-time decision in `TreeNode.tsx`: show it when a node has no known children yet (`node.children.length === 0`) **and** `props.datasource.status === ValueStatus.Loading` — Mendix's own, real-time, first-party "is this datasource actually fetching right now" signal. No per-node bookkeeping, so nothing can be "stuck" (the flag is never persisted) and nothing about one node's resolution can affect another's (it's a single global flag, not a per-node mutation). + +## `useInfiniteTreeNode.ts`'s one-level-lookahead preload — content-gated, not fire-count-gated + +`appendItems` (called on every click-to-expand) and the bootstrap `useEffect` (auto-expand for `startExpanded = Yes`) both implement the same idea: when a node's children become known, also preload _their_ children's existence one level further ahead, so an already-visible child's own expand affordance is correct without the user needing to click into it first. Both had the same class of bug (WC-3564, found during manual verification, pre-existing on `main`, unrelated to the `LOADING`/`hasChildren` mechanism above): + +- `appendItems` gated the preload step behind "was this node already a loaded-parent" — true only from a node's _second_ expand onward, so the first expand never preloaded its children's children. A collapse+re-expand of the _same_ node was required to see a deeper tier. **Fixed**: removed that gate — the preload now runs unconditionally whenever children are passed in. Verified live. +- The bootstrap effect capped itself at exactly _one_ automatic round ever (`loadedParentsByIdRef.current.size === 0`, true only once) — so `startExpanded = Yes` roots got their own children preloaded, but never one level further. A collapse+re-expand of a _root_ node was required to see a deeper tier. + + **First attempt at a fix broke live testing**: extended the cap to exactly 2 rounds via a plain counter that advanced on every effect firing. Passed unit tests against a mock, but broke the real "Expanded bug" repro project — the tree stopped rendering anything past the root level. Root-caused with temporary per-widget-tagged debug logging (three tree widgets mount simultaneously on that page regardless of active tab, so untagged logs were unreadable): three tree widgets on the page were logging interleaved, and once tagged, the trace showed both rounds fired — and locked themselves in — while `datasource.items` was still transiently empty during initial load, before the real root items ever arrived. A counter can't tell "fired" apart from "fired with something worth preloading." + + **Second attempt**: replaced the counter with two content-based flags (`round1DoneRef`, `round2DoneRef`) that only flip once real, not-yet-tracked items are actually found — mirroring the pre-existing round-1 gate's own self-correcting semantics (checked _after_ attempting to populate, so it harmlessly retries on an empty delivery instead of locking in early). Verified live against a 2-tier "Expanded bug" dataset at the time — worked. Against a deeper (4-tier) dataset, it turned out still insufficient: the 3rd tier showed up as content but without its own icon, needing a real click on an ancestor to reveal — the fixed 2-round cap was itself the bug, just less obviously than the original one-round cap. + + **Final fix**: since every level defaults to `EXPANDED` (not just roots) under `startExpanded = Yes`, replaced the round cap entirely with an unbounded, self-terminating cascade — keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants appear, stop once a round finds nothing new (bounded by the tree's real depth, not a count). Scoped specifically to `startExpanded === true` (confirmed by asking — see decisions log): `startExpanded = false` keeps the original capped round1+round2 behavior unchanged, since deeper tiers there stay collapsed by default and already resolve correctly via a single click. Verified live against the real 4-tier dataset: every tier shows its correct expand affordance automatically, no manual toggle needed anywhere. + +**Lesson, still worth keeping in mind**: a change here can look bounded/safe by code inspection and pass every mocked unit test, yet still break against a real datasource, because mocks don't reproduce the transient "still loading, items temporarily empty" window real ones do — and a fix verified against a shallow test dataset can still be wrong at greater depth. Any change to `setFilter` call timing/count in this file needs live verification against a real repro project at real depth, not just mocked unit tests, before being trusted. + +## Decisions log + +- 2026-09-15 (WC-3564): initially decided to wire up `props.hasChildren` as the icon-visibility source. **Reverted after live testing crashed the widget** — discovered `hasChildren` is architecturally unavailable in v2 (see above). Corrected to derive `hasChildren` from `node.children.length > 0` (the pre-existing, structurally-correct signal) and drive the spinner from `datasource.status` instead of any per-node stored state. +- 2026-09-15 (WC-3564, found during manual verification): discovered and fixed two further pre-existing bugs in `useInfiniteTreeNode.ts`'s one-level-lookahead preload (see above) — bundled into the same change with explicit user sign-off, since found/understood/fixed during the same verification pass. The second one required three attempts: a fire-count-based cap broke live testing and was reverted; a content-based 2-round cap fixed that but was itself too shallow against deeper data; an unbounded content-gated cascade, scoped to `startExpanded = Yes` only (confirmed via explicit question — user rejected applying it to `startExpanded = No` too, since that would eagerly prefetch descendants of still-collapsed, not-yet-visible branches), fixed it correctly at real depth, verified live. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/.openspec.yaml b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/.openspec.yaml new file mode 100644 index 0000000000..96db9a43b6 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-15 diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md new file mode 100644 index 0000000000..5cd3aff415 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md @@ -0,0 +1,79 @@ +## Context + +`TreeNode.tsx` (root dispatcher) routes to v2 (`src/components/v2/`) whenever `parentAssociation` is configured — the self-referencing "infinite tree" mode this ticket's repro projects use. `TreeNode.editorConfig.ts:38-39` hides the `hasChildren` widget property from Studio Pro whenever `parentAssociation` is set, and it has no XML default value — so on every v2 instance, `props.hasChildren` is `undefined` at runtime. (Confirmed live: an initial attempt to read it crashed the widget with `TypeError: Cannot read properties of undefined (reading 'get')`.) v2 must derive "does this node have children" structurally, from `node.children.length > 0` — there is no other signal available to it. + +Why this requires fetching at all, rather than a cheap existence check: Mendix's pluggable-widget data API (`ListValue`) gives a widget exactly one shared, filter-driven list — there is no lighter-weight "does association X have any related record" or per-item count primitive exposed to pluggable widgets. Determining "does node N have children" means asking the datasource for items whose parent is N and checking whether anything comes back; there's no way to get that answer without the datasource actually returning (at least) the matching item(s). This is why the whole preload mechanism (`loadedParentsByIdRef`/`loadedChildsByIdRef`, both in `appendItems` and the bootstrap effect below) exists at all — it's not a workaround for a missed optimization, it's the only mechanism this API surface provides for v2's configuration mode. + +`useIncrementalTreeData.ts:114-119` (pre-fix) flipped a node out of `TreeNodeState.LOADING` the moment its id reappeared in _any_ subsequent `items` delivery — not specifically a delivery meant to answer "does this node have children." A microflow datasource (`useInfiniteTreeNode.ts`) always redelivers its full flattened result regardless of the filter passed to `datasource.setFilter(...)`, since microflow datasources ignore filters entirely. Two consequences, confirmed via live instrumented repro against customer-attached repro projects for WC-3564: + +- **Bug 1**: with "Start expanded" = Yes, a node stays `LOADING` forever if the only mechanism meant to resolve it (a filtered re-delivery) never distinguishes "answered" from "not yet answered" — the spinner never clears. +- **Bug 2**: with "Start expanded" = No, expanding any node triggers `appendItems` → `setFilter`, which (because the microflow ignores the filter) redelivers everything, including the ids of _unrelated_, not-yet-clicked nodes. Those nodes flip out of `LOADING` per lines 114-119, landing in `COLLAPSED_WITH_JS` with an empty `children` array — permanently killing their icon, since the icon-render condition (`TreeNode.tsx:49`, pre-fix) was `hasChildren || treeNodeState === LOADING` and neither is true anymore. + +Git history (`dcbf9bdb51`, "fix: add empty message, loading, and keyboard nav") shows `LOADING` was added later, purely for polish: before that commit, a new node was created directly as `EXPANDED`/`COLLAPSED_WITH_JS`, and a node that would turn out to have children simply showed no icon for one render until its children got placed — a minor "icon pops in" flicker. `LOADING` was introduced to bridge that instant with a spinner instead of nothing, but the _resolution_ criterion it shipped with was wrong, and that wrong criterion is the root cause of both WC-3564 bugs. + +## Goals / Non-Goals + +**Goals:** + +- Expand-icon visibility for v2 must never depend on a widget property that is architecturally unavailable in v2's own configuration mode (`hasChildren`). +- A node's spinner state must never be "stuck" (Bug 1) or capable of corrupting an unrelated node's state (Bug 2). +- Preserve the original polish goal (avoid an abrupt icon pop-in) using a signal that cannot exhibit either bug. +- Both WC-3564 bugs fixed by the same underlying mechanism (not two separate patches). + +**Non-Goals:** + +- Not changing v1 behavior — v1 correctly reads `hasChildren` (it has no association-based alternative) and is untouched by this change. +- Not fixing incorrect Studio Pro configuration of `hasChildren` — irrelevant to v2 now, since v2 never reads it. +- Not guaranteeing a spinner ever shows for every conceivable timing gap — the fix only guarantees the spinner is never stuck and never corrupts a sibling; if the datasource's `status` never reports `Loading` for a given fetch, no spinner shows for it (functionally harmless, matches original pre-`LOADING`-commit behavior). + +## Decisions + +### D1 (revised): `hasChildren` stays derived from `node.children.length > 0`; `props.hasChildren` is never read in v2 + +Initial design used `props.hasChildren.get(node.item).value` as the icon-visibility source, on the premise that the prop was simply unused, not unusable. Live testing against the actual WC-3564 repro project crashed the widget (`props.hasChildren` is `undefined` for any v2 instance — see Context). Reverted: `TreeNode.tsx`'s `hasChildren` local goes back to `node.children.length > 0`, exactly as before this ticket. `renderRecursiveNode` no longer threads a `hasChildren` expression parameter at all. + +This is safe against both bugs once D2 (below) lands, because `node.children` is only ever mutated by real, structurally-correct placement (`useIncrementalTreeData.ts`'s `placeNode`) — there is no longer a spurious "resolve" step that can zero out a node's children array based on unrelated data. + +### D2 (revised): `LOADING` is a render-time-only spinner decision, never stored per node + +A newly-created node is created directly as `EXPANDED`/`COLLAPSED_WITH_JS` (per `config.startExpanded`) in `useIncrementalTreeData.ts` — `TreeNodeState.LOADING` is never assigned to `treeNodeState` anywhere in that file, and the click handler in `TreeNode.tsx` goes back to unconditionally setting `EXPANDED` (matching pre-`LOADING`-commit behavior; no guard needed since nothing sets `LOADING` on click anymore). + +The spinner is computed fresh on every render in `TreeNode.tsx`: `showSpinner = node.children.length === 0 && props.datasource.status === ValueStatus.Loading`. `datasource.status` is Mendix's own first-party, real-time "is this datasource actually fetching right now" signal (`ListValue.status: ValueStatus`) — not bookkeeping we maintain ourselves. `renderHeaderIcon` receives `showSpinner ? TreeNodeState.LOADING : node.treeNodeState`, so `LOADING` still exists as an icon-rendering signal (satisfying the original "repurpose, don't remove" call), just never persisted on the node. + +This is structurally immune to both bugs: + +- **Bug 1 can't recur**: nothing is ever "waiting" in a stored sense. The spinner shows exactly while `status` says `Loading`, and clears the instant it doesn't — including the case where a microflow's `setFilter` call is a genuine no-op and `status` never even transitions to `Loading` (spinner correctly never shows, rather than showing forever). +- **Bug 2 can't recur**: there's no per-node mutable "resolved" flag to wrongly flip. Every render recomputes `hasChildren` fresh from the current `children` array and `showSpinner` fresh from the single global `status` flag — one node's expand action can only ever change _its own_ children array (via real placement) or the shared `status` (which, if it flips, affects all unresolved nodes' spinners equally and correctly, not selectively/incorrectly). + +**Alternatives considered**: + +- _Track per-node fetch-request ids from `useInfiniteTreeNode.ts` and gate resolution on that._ Rejected — solves the wrong layer; still lets a node start `LOADING` and wait indefinitely if the tracked request never resolves (Bug 1's actual mechanism), and adds bookkeeping complexity for no additional correctness over the `datasource.status` approach. +- _Resolve `LOADING` immediately whenever a node's first child is placed, entered via click when `children.length === 0`._ Rejected — requires guarding the click handler against genuine leaves (ambiguous: `children.length === 0` means both "confirmed leaf" and "real parent not yet preloaded," indistinguishable from node state alone), and turned out to be moot anyway once `hasChildren` was reverted to being structurally-derived (a node is only ever clickable once its children are already known, via the existing one-level-lookahead preload — see Context). +- _Drop `LOADING`/the spinner entirely._ Considered when it looked like the preload design left no genuine "waiting" window at all. Rejected per explicit user request to keep a spinner "just in case" — `datasource.status` gives a correct way to do that without reintroducing either bug. + +### D3 (added — found during manual verification, not part of the original two bugs): the one-level-lookahead preload had two gaps, both closed + +While verifying D1/D2 live, manual testing surfaced that a node's expand affordance for a _deeper_ tier sometimes didn't appear until the user collapsed and re-expanded a node — a real, pre-existing bug on `main`, unrelated to the `LOADING`/`hasChildren` mechanism above (it lives entirely in `useInfiniteTreeNode.ts`, which D1/D2 never touch). Two separate gaps in the same "preload one level past what's currently expanded" mechanism: + +- **Click-driven gap** (`appendItems`): the grandchildren-preload step (`children.forEach(...)`, adding a node's children to `loadedChildsByIdRef` so _their_ children get fetched too) was nested inside `if (loadedParentsByIdRef.current.has(parentId))` — true only from a node's _second_ expand onward. A node's first-ever expand skipped preloading its children's children, so a deeper tier's expand icon only appeared after a collapse+re-expand. **Fix**: removed that outer gate — the preload step now always runs when children are passed in, regardless of whether this is the first or a later expand. Verified live: a single click now reveals a 4th tier that previously needed collapse+re-expand. +- **Bootstrap-path gap** (the second `useEffect`, `startExpanded = Yes` specifically): root nodes auto-expand via a separate path that populates `loadedParentsByIdRef` directly from `datasource.items`, bypassing `appendItems` entirely — so the click-driven fix above doesn't reach them. This path was also capped at exactly one automatic round (`if (loadedParentsByIdRef.current.size === 0)`, true only once ever), so it preloaded roots' children but never went one level further. + + **First attempt, reverted after breaking live**: replaced the one-shot gate with a `bootstrapRoundRef` counter that advanced unconditionally on every effect firing, capped at 2. Passed unit tests against a mocked datasource, but broke the real "Expanded bug" tab live — the tree stopped rendering anything past the root level. Root-caused with temporary per-widget-tagged debug instrumentation (three tree widgets mount simultaneously on that page regardless of which tab is active, so untagged logs were unreadable): both rounds fired, and _locked themselves in_, while `datasource.items` was still transiently empty during initial load — before the real root items ever arrived. A blind counter can't distinguish "this effect fired" from "this effect fired with something worth preloading"; it burned both capped rounds on nothing, permanently disabling the mechanism. + + **Second attempt**: replaced the counter with two content-based flags (`round1DoneRef`, `round2DoneRef`) that only flip once real, not-yet-tracked items are actually found — mirroring the pre-existing round-1 gate's own self-correcting semantics (`loadedParentsByIdRef.current.size === 0`, checked _after_ attempting to populate: harmlessly retries on an empty delivery, locks in only once real data lands). Round 2 only locks in once its scan finds at least one item that isn't already a known parent or child. Verified live: the "Expanded bug" tab (2-tier test data at the time) showed all tiers automatically on load, with no regressions to Bug 1/Bug 2/the `appendItems` fix. + + **Third attempt (final, kept)**: against a deeper (4-tier) dataset, the 2-round cap turned out insufficient — the 3rd tier appeared as visible content but without its own expand icon, needing one more real click on an ancestor to reveal, one level deeper than the original repro exposed. Root insight: under `startExpanded = Yes`, _every_ level defaults to `EXPANDED`, not just roots (`useIncrementalTreeData.ts`'s node-creation branch — see D2) — so every level needs the same automatic one-level-lookahead treatment, not a fixed count of 2. The original "avoid eagerly walking the whole tree" concern (below) doesn't actually apply to `startExpanded = Yes`: since nothing is collapsed in that mode, walking the whole tree _is_ the correct, intended behavior — bounded by the tree's real depth (a finite, self-terminating cascade), not an unbounded/runaway one. Fixed by replacing the 2-round cap with an unbounded cascade, gated specifically on `startExpanded === true`: keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants keep appearing; stop once a round finds nothing new. `startExpanded = false` keeps the original capped round1+round2 behavior — for that mode, deeper tiers are still collapsed by default and resolve correctly via a single real click already (group 6's fix), so auto-cascading further would only be wasted eager-fetching of not-yet-visible content. Verified live: the "Expanded bug" tab (now the real 4-tier dataset) shows every tier automatically, matching the exact result the user originally showed as expected; the "Collapsed bug" tab's 2-round-capped behavior is unchanged and still passes. + +**Alternative considered**: apply the unbounded cascade regardless of `startExpanded`. Rejected per explicit user decision — would eagerly prefetch descendants of branches that are still collapsed and not visible under `startExpanded = false`, for no user-visible benefit (those tiers already resolve correctly in a single click once actually expanded). +**Alternative considered** (superseded by the "why can't we just know without fetching" question — see Context below): skip preloading and derive "has children" from a cheaper existence check. There is no such check available — see Context. + +## Risks / Trade-offs + +- **[Trade-off]** If a real (non-microflow) datasource's `status` never meaningfully transitions to `Loading` for some fetch (e.g. resolves synchronously from cache), the spinner simply won't show for that fetch — same as the original pre-`LOADING`-commit behavior (brief icon pop-in instead of a spinner). Cosmetic only, not a functional regression. +- **[Risk]** None identified that could reproduce either original bug — see D2's "structurally immune" reasoning above. Covered by unit tests asserting spinner-shows-while-loading, spinner-clears-on-settle (whether or not children arrived), and one node's resolution never affecting a sibling's spinner/children/state. +- **[Risk]** D3's bootstrap round-2 preload fetches one extra level for every currently-known item, regardless of whether the user has looked at it — a bounded, one-time eager-fetch (proportional to tree _width_ at that level, not depth) → **Mitigation**: capped at exactly 2 rounds via content-based flags, verified live and by a unit test asserting a 3rd datasource change does not trigger a 3rd round. This matches the cost the widget already pays for round 1 (unconditionally preloading roots' children regardless of collapse state) — round 2 is the same category of cost, one level deeper, not a new category of risk. +- **[Risk — realized and fixed, kept as a lesson]** A first attempt at this exact fix (a fire-count-based cap) passed every unit test against a mocked datasource but broke a real repro project live, because mocked datasources don't reproduce the transient "still loading, items temporarily empty" window that real ones do. Mitigation going forward: any change to `useInfiniteTreeNode.ts` that touches `setFilter` call timing/count must be live-verified against a real datasource before being trusted, not just unit-tested against a mock. + +## Migration Plan + +No data or config migration. `hasChildren` is no longer read by v2 at all, so existing v2 configurations (where it was always hidden/unset anyway) are unaffected; v1 is untouched. Standard widget version bump + changelog entry per repo convention; no feature flag needed since this is a bug fix restoring intended behavior. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md new file mode 100644 index 0000000000..8a8757f2de --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md @@ -0,0 +1,33 @@ +## Why + +Tree Node v2 stores `LOADING` as a per-node state and resolves it via a broken heuristic: "this node's id reappeared in some later datasource delivery." A microflow datasource — which always redelivers its full flattened result and ignores `setFilter` — breaks that heuristic in two ways (WC-3564): a permanently stuck loading spinner when "Start expanded" is Yes, and a 3rd-tier node silently and permanently losing its expand icon when a sibling is expanded. Both share the same root cause and are fixed by the same change, hence one proposal covering both. + +Manual verification of that fix surfaced two further, separate, pre-existing bugs in the same "preload one level ahead of what's expanded" mechanism (`useInfiniteTreeNode.ts`) — unrelated to `LOADING`/`hasChildren`, but bundled into this same change since they were found, understood, and fixed during the same verification pass: a node's first-ever expand didn't preload its own children's children (needed a collapse+re-expand of that same node to reveal a deeper tier), and the automatic root-expansion path for "Start expanded" = Yes had the same gap, recurring at every level. A first attempt at the second fix (a fixed 2-round cap) broke a live repro project and was reverted before being corrected; a second attempt fixed that but, against a deeper (4-tier) dataset, turned out to still be too shallow — every level defaults to expanded under "Start expanded" = Yes, not just roots, so a fixed round count can't be right at all. The final fix replaces the round cap with an unbounded, self-terminating cascade, scoped specifically to "Start expanded" = Yes. See `design.md` D3 for the full story, including the lesson that a fix here needs live verification against a real datasource, not just mocked unit tests. + +## What Changes + +- `LOADING` is no longer stored on a node at all. Nodes are created already resolved (`EXPANDED`/`COLLAPSED_WITH_JS` per `startExpanded`); the click-to-expand handler goes back to unconditionally setting `EXPANDED`. +- The spinner becomes a pure render-time decision: shown when a node has no known children yet (`node.children.length === 0`) **and** Mendix's own `datasource.status === ValueStatus.Loading` — a real, first-party "is this actually fetching right now" signal, not per-node bookkeeping. +- Expand-icon visibility for v2 stays derived from `node.children.length > 0` (unchanged from before this ticket) — **not** from the `hasChildren` widget property. `hasChildren` is hidden by Studio Pro and unset at runtime whenever `parentAssociation` is configured, which is every v2 instance; reading it crashes the widget (confirmed live during this change's implementation). +- `useInfiniteTreeNode.ts`'s `appendItems`: removed a gate that skipped preloading a node's grandchildren-existence on its first-ever expand (only ran from the second expand onward). +- `useInfiniteTreeNode.ts`'s bootstrap effect: when "Start expanded" is Yes, the preload now cascades level-by-level for as long as new descendants keep appearing (self-terminating once a level introduces nothing new) — matching every level defaulting to expanded in that mode. When "Start expanded" is No, the original capped round1+round2 behavior is unchanged (deeper tiers stay collapsed by default and already resolve correctly via a single real click). +- Update `TreeNodeV2.spec.tsx`, `useIncrementalTreeData.spec.ts`, and `useInfiniteTreeNode.spec.ts` to cover the corrected behavior. + +## Capabilities + +### New Capabilities + +- `tree-node-expand-state`: governs how a v2 tree node decides (a) whether it shows an expand affordance at all, and (b) whether it shows a spinner in place of that affordance, independent of datasource re-delivery timing or datasource type (microflow vs. non-microflow). + +### Modified Capabilities + +(none — no existing `openspec/specs/` in this package prior to this change) + +## Impact + +- `src/components/v2/TreeNode.tsx` — icon-render condition now spinner-vs-chevron based on `datasource.status`; click handler simplified (no `LOADING` entry). +- `src/components/v2/hooks/useIncrementalTreeData.ts` — nodes created pre-resolved; no `LOADING` assignment anywhere in this file. +- `src/components/v2/hooks/useInfiniteTreeNode.ts` — `appendItems`'s preload gate removed; bootstrap effect's preload is now an unbounded, content-gated cascade when `startExpanded` is Yes, and unchanged (capped round1+round2) when it's No. +- `typings/TreeNodeProps.d.ts` — no change. +- `src/components/v2/__tests__/TreeNodeV2.spec.tsx`, `src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts`, `src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts` — updated/added regression tests. +- No XML property changes. `hasChildren` remains untouched for v1 (unaffected by this change). diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md new file mode 100644 index 0000000000..8fb0ba89a2 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md @@ -0,0 +1,85 @@ +## ADDED Requirements + +### Requirement: Expand affordance visibility is driven by known children, not by a load-timing-sensitive stored state + +The v2 Tree Node widget SHALL determine whether a node's expand affordance (chevron/icon) is shown based on whether that node currently has any children placed under it (`node.children.length > 0`), computed fresh on every render — never from the `hasChildren` widget property, which is unavailable in v2's configuration mode (`parentAssociation` set), and never from a stored per-node flag that a later, unrelated datasource delivery could incorrectly mutate. + +#### Scenario: A node with children shows an expand affordance + +- **WHEN** a node has at least one child currently placed under it +- **THEN** the widget renders an expand affordance for that node + +#### Scenario: A node with no known children shows no expand affordance (absent a spinner) + +- **WHEN** a node has no children currently placed under it and the datasource is not currently loading +- **THEN** the widget renders no expand affordance for that node + +#### Scenario: Expanding one node does not affect a sibling's expand affordance + +- **WHEN** a user expands node A, causing a datasource redelivery that includes node B's id (node B was never expanded and has no relation to node A) +- **THEN** node B's expand affordance and underlying children are unchanged from before node A was expanded + +### Requirement: A loading spinner is shown only while the datasource is genuinely fetching, never as stored per-node state + +The v2 Tree Node widget SHALL show a loading spinner in place of the expand affordance for a node that has no known children yet, exactly while `datasource.status === ValueStatus.Loading`. This is a render-time computation only — no per-node "is loading" flag is stored, so the spinner cannot become stuck and cannot be affected by an unrelated node's resolution. + +#### Scenario: Spinner shown while the datasource is loading and children are unknown + +- **WHEN** a node has no children placed under it yet and the datasource's `status` is `Loading` +- **THEN** the widget shows a spinner in place of the expand affordance for that node + +#### Scenario: Spinner clears once the datasource settles, regardless of outcome + +- **WHEN** the datasource's `status` transitions away from `Loading` +- **THEN** every node's spinner clears immediately — showing an expand affordance if children arrived, or no affordance at all if they didn't + +#### Scenario: Spinner never shows for a node that already has children + +- **WHEN** a node already has at least one child placed under it +- **THEN** the widget never shows a spinner for that node, regardless of `datasource.status` + +#### Scenario: A stalled or filter-ignoring datasource never produces a stuck spinner + +- **WHEN** a microflow datasource ignores `setFilter` and its `status` never transitions to `Loading` for a given fetch attempt +- **THEN** no node is left showing a spinner indefinitely as a result of that fetch attempt + +### Requirement: A manually expanded node's own children's expand affordance is known without requiring a collapse-and-reopen + +The v2 Tree Node widget SHALL preload one level past a node's children when that node is expanded by a user click, so each child's own expand affordance is already correct the first time its parent is expanded — never requiring the user to collapse and re-expand that same node to reveal it. This preload is bounded to exactly one level past what's already known for this path; it does not eagerly walk the full tree beyond the node that was actually clicked. + +#### Scenario: A node's own children's children are known on its first expand + +- **WHEN** a user expands a node for the first time (its children are already known, but whether those children themselves have children is not) +- **THEN** each of that node's children already shows its correct expand affordance immediately, without requiring that child to be separately collapsed and re-expanded + +### Requirement: Under "Start expanded" = Yes, every auto-expanded level's own expand affordance is known automatically, all the way to the tree's real depth + +Because every node defaults to expanded (not just roots) when "Start expanded" is Yes, the v2 Tree Node widget SHALL keep preloading one level further for as long as new descendants keep appearing — not a fixed number of levels — so that every already-visible node's expand affordance is correct without any manual collapse-and-reopen, regardless of how deep the actual tree data goes. This cascade is self-terminating: it stops automatically once a level introduces no previously-unseen items, bounded by the tree's real depth rather than an arbitrary count or recursing indefinitely. + +#### Scenario: A 3rd (or deeper) tier's own expand affordance is known automatically + +- **WHEN** "Start expanded" is Yes and the underlying data has 3 or more tiers +- **THEN** every tier's nodes show their correct expand affordance immediately on load, with no tier requiring a manual collapse-and-reopen to reveal the next tier down + +#### Scenario: The cascade stops once the real data is exhausted + +- **WHEN** a subsequent datasource delivery introduces no items beyond what's already known +- **THEN** no further automatic preload round is triggered — the cascade does not continue indefinitely or re-fetch unchanged data + +#### Scenario: A transient empty datasource delivery during initial load does not disable the cascade + +- **WHEN** the datasource is still loading and delivers an empty item set one or more times before the real data arrives +- **THEN** the cascade does not lock itself out on that empty delivery — it only advances once it actually finds real, previously-unseen items, and keeps retrying harmlessly until it does + +### Requirement: The auto-cascade does not apply when "Start expanded" is No + +The v2 Tree Node widget SHALL NOT auto-cascade the preload beyond the existing capped behavior (root's children, plus one level of lookahead) when "Start expanded" is No, since deeper tiers remain collapsed by default and already resolve correctly via a single real click. Auto-cascading further in this mode would only eagerly fetch descendants of branches the user has not opened. + +#### Scenario: A 3rd-tier arrival does not trigger a further automatic round when collapsed by default + +- **WHEN** "Start expanded" is No and a 3rd-tier item arrives as a result of the existing 2-round preload +- **THEN** no further automatic preload round is triggered for it — expanding it further still requires a real click + +### Requirement: Auto-expanded root nodes (`startExpanded = Yes`) — NOT YET IMPLEMENTED + +The equivalent one-level lookahead for automatically auto-expanded root nodes (so a root's children already show their correct expand affordance without needing the root collapsed and re-expanded) was attempted and reverted after it broke a live repro project (see `design.md` D3). No requirement is claimed here for this case. A collapse+re-expand of a root node is currently still needed to reveal a 3rd tier under `startExpanded = Yes`; this is a known, pre-existing, unfixed gap, tracked for a future change once root-caused. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md new file mode 100644 index 0000000000..c0e8656837 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md @@ -0,0 +1,50 @@ +## 1. Resolve-at-creation, drop stored `LOADING` (D2) + +- [x] 1.1 In `useIncrementalTreeData.ts`'s node-creation branch, create new nodes directly as `config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS` instead of `TreeNodeState.LOADING`. +- [x] 1.2 Remove the "existing node in `LOADING` resolves on reappearance" branch entirely — no node is ever assigned `LOADING` in this file anymore, so there is nothing left to resolve. +- [x] 1.3 Revert `TreeNode.tsx`'s click handler to unconditionally set `EXPANDED` on expand-click (matches pre-`LOADING`-commit behavior) — no `LOADING` entry via click, no guard needed. +- [x] 1.4 Compute the spinner at render time in `TreeNode.tsx`: `showSpinner = node.children.length === 0 && props.datasource.status === ValueStatus.Loading`. Pass `showSpinner ? TreeNodeState.LOADING : node.treeNodeState` into `renderHeaderIcon`. No per-node `LOADING` is stored anywhere. + +## 2. `hasChildren` derivation — reverted after live-testing crash + +- [x] 2.1 **Correction, found via live Studio Pro testing (not caught by unit tests):** the initial plan was to read `props.hasChildren` in v2. This crashed the widget — `TreeNode.editorConfig.ts:38-39` hides `hasChildren` from Studio Pro whenever `parentAssociation` is configured (every v2 instance, including both of this ticket's repro projects), and it has no XML default, so `props.hasChildren` is `undefined` at runtime for v2. Reverted `TreeNode.tsx:20`'s `hasChildren` derivation back to `node.children.length > 0` — the same signal it used before this ticket. Removed the `hasChildrenExpr` parameter from `renderRecursiveNode` entirely. +- [x] 2.2 Confirmed `aria-expanded`, icon clickability, and the icon-render condition all still read the single `hasChildren` local (now `node.children.length > 0`) unchanged. +- [x] 2.3 Confirmed the icon-render condition is `(hasChildren || showSpinner) && iconPlacement !== "no"` — a node with children never spins; a childless node spins only while `datasource.status === Loading`. + +## 3. Regression tests + +- [x] 3.1 Rewrote `TreeNodeV2.spec.tsx`'s stale comment (previously claimed the datasource setup existed to satisfy a `hasChildren` prop check — no longer applicable since `hasChildren` isn't read at all). Hoisted shared test helpers to module scope so a new describe block could reuse them. +- [x] 3.2 Added a unit test (`TreeNodeV2.spec.tsx` + `useIncrementalTreeData.spec.ts`): a node is created directly in `EXPANDED`/`COLLAPSED_WITH_JS` per `startExpanded` and never passes through `LOADING`, even when the datasource keeps redelivering the same full item set (simulating a microflow ignoring `setFilter`) — Bug 1 regression. +- [x] 3.3 Added a unit test: a node's own children arriving does not change an unrelated sibling's `treeNodeState`, children, or spinner — Bug 2 regression. Covered in both spec files. +- [x] 3.4 Added unit tests for the spinner itself: shows while `datasource.status === Loading` and no children are known; clears once `status` settles regardless of whether children arrived; never shows for a node that already has children. + +## 4. Manual verification + +- [x] 4.1 Rebuilt against `~/Documents/_tickets/WC-3564-2` and drove it with Playwright. First rebuild (with the `props.hasChildren`-based design) crashed the widget live — see task 2.1. After the correction, rebuilt again and confirmed: expanding "Top level 2" leaves "Second level 1a"/"Second level 1b"'s expand icons intact (icon count unchanged before/after, screenshot-confirmed) — Bug 2 fixed. +- [x] 4.2 Confirmed bug 1's repro (microflow datasource, "Start expanded" = Yes) live: tree renders fully expanded immediately, zero `.widget-tree-node-loading-spinner` elements, no console/page errors. + +## 5. Changelog + +- [x] 5.1 Added a `CHANGELOG.md` entry under `[Unreleased]` describing the user-visible fix (both bugs), no implementation details, per repo changelog conventions. + +## 6. Third finding: `appendItems` off-by-one-click preload gap (found during manual verification) + +Pre-existing on `main`, unrelated to `useIncrementalTreeData.ts`/`TreeNode.tsx` (untouched by groups 1-2). A node's own click-to-expand never preloaded its _own_ children's children — required a collapse+re-expand of that same node before a deeper tier's expand icon appeared. + +- [x] 6.1 In `useInfiniteTreeNode.ts`'s `appendItems`, removed the outer `if (loadedParentsByIdRef.current.has(parentId))` gate around the grandchildren-preload `children.forEach(...)` step — it was skipping that step on a node's _first_ expand (the only time it matters), only running it from the second expand onward. +- [x] 6.2 Added a unit test in `useInfiniteTreeNode.spec.ts` — not needed as a new test; existing "first expansion" describe block continues to cover this since the gate removal doesn't change its assertions, but confirmed no existing test asserted the buggy gated behavior. +- [x] 6.3 Verified live: rebuilt against `~/Documents/_tickets/WC-3564-2`, single click on "Second level 1a" (previously required collapse+re-expand) now immediately reveals "Fourth level 1" under "Third level 1a1" — confirmed via Playwright polling (no click-twice needed). +- [x] 6.4 Re-ran the full rigorous Bug 1 / Bug 2 regression suite live after this change — no regressions. + +## 7. Fourth finding: bootstrap preload capped at one round, never reaches a second (found during manual verification) + +Pre-existing on `main`, in `useInfiniteTreeNode.ts`'s second `useEffect` block — separate from group 6 (that gate was in `appendItems`, click-driven; this one is in the automatic bootstrap path that runs regardless of clicks). For `startExpanded = Yes` specifically, root nodes auto-expand via this bootstrap path rather than via `appendItems`, so group 6's fix doesn't reach them — closing and reopening a root node was required to reveal a 3rd tier. + +- [x] 7.1 **First attempt (reverted):** replaced the one-shot `if (loadedParentsByIdRef.current.size === 0)` gate with a `bootstrapRoundRef` counter that advanced unconditionally on every effect firing, capped at 2. Passed unit tests against a mocked datasource. **Broke live**: rebuilt against `~/Documents/_tickets/WC-3564-2`, the "Expanded bug" tab permanently stopped rendering anything past the root level. Root-caused via temporary debug instrumentation (tagged per-widget-instance to disentangle the 3 tree widgets that mount simultaneously on that page): both rounds fired — and locked themselves in — while `datasource.items` was still transiently empty (still loading), _before_ the real root items ever arrived. The counter had no way to tell "fired" apart from "fired with real data," so it burned both of its capped rounds on nothing. +- [x] 7.2 **Second attempt (this one, kept):** replaced the counter with two content-based booleans (`round1DoneRef`, `round2DoneRef`) that only flip once real, previously-unseen items are actually found — mirroring the original round-1 gate's own self-correcting semantics (`loadedParentsByIdRef.current.size === 0`, checked _after_ attempting to populate, so it harmlessly retries on empty deliveries instead of locking in early). Round 2 only locks in once it finds at least one item that isn't already a known parent or child. +- [x] 7.3 Removed all debug instrumentation added for root-causing 7.1 (tagged `[DEBUG-t3564b]`, per-widget-instance) — confirmed zero references remain. +- [x] 7.4 Added a unit test in `useInfiniteTreeNode.spec.ts` that explicitly exercises the failure mode from 7.1: several transient empty-item rerenders before round 1 locks in, another empty/unchanged rerender before round 2 locks in, then confirms round 2 only advances once real new items appear, and a further identical rerender does not trigger a round 3. +- [x] 7.5 Verified live: rebuilt against `~/Documents/_tickets/WC-3564-2`. "Expanded bug" tab now shows all 3 tiers automatically on load — no manual toggle needed. Re-ran the full rigorous Bug 1 / Bug 2 / group-6 (`appendItems`) regression suite live — all still pass, no regressions. +- [x] 7.6 **Follow-up finding, same session:** the 2-round cap turned out insufficient — with a 4-tier dataset, the 3rd tier appeared as content but without its own expand icon (needed a real click on its own ancestor to reveal, one level deeper than the original repro). Root cause: under `startExpanded = Yes`, _every_ level defaults to `EXPANDED` (not just roots), so every level needs the same automatic preload treatment, not just a fixed 2 rounds. Fixed by replacing the 2-round cap with an unbounded, self-terminating cascade **gated specifically on `startExpanded === true`**: keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants keep appearing, stopping naturally once a round finds nothing new (bounded by the tree's real depth, not an arbitrary count). Explicitly scoped to `startExpanded = true` only, per user decision — `startExpanded = false` keeps the original capped round1+round2 behavior unchanged (deeper tiers there already resolve correctly via a single real click, per group 6's fix; auto-cascading for still-collapsed branches would just be wasted eager-fetching of content the user hasn't opened). +- [x] 7.7 Updated the unit test from 7.4 to match: renamed/rewritten as an unbounded-cascade test asserting 3+ sequential levels each trigger exactly one more `setFilter` call as they arrive, and a repeated/empty delivery triggers none. Added a second test confirming `startExpanded = false` still caps at exactly 2 rounds (3rd-tier arrival triggers no further automatic call). +- [x] 7.8 Verified live again: rebuilt against `~/Documents/_tickets/WC-3564-2`. "Expanded bug" tab (4 levels of data) now shows all 4 tiers automatically on load, with `Third level 1a1` already showing its own expand icon with zero manual interaction — matching the exact screenshot the user originally showed as the expected/desired result. Re-ran the full rigorous Bug 1 / Bug 2 / group-6 / "Collapsed bug" (startExpanded=false, 2-round-cap) regression suite live — all still pass, no regressions. From 8b1c93c1e4e896e8a15d2b9ec9e163d47becb524 Mon Sep 17 00:00:00 2001 From: gjulivan Date: Thu, 17 Sep 2026 10:59:34 +0200 Subject: [PATCH 5/5] fix: combine treenode changes --- .../tree-node-web/CHANGELOG.md | 3 + .../pluggableWidgets/tree-node-web/CONTEXT.md | 36 ++++- .../fix-tree-node-loading-state/design.md | 53 +++++++ .../fix-tree-node-loading-state/proposal.md | 18 ++- .../specs/tree-node-data-refresh/spec.md | 44 ++++++ .../specs/tree-node-expand-state/spec.md | 43 +++++- .../fix-tree-node-loading-state/tasks.md | 66 ++++++++ .../tree-node-web/package.json | 3 +- .../__tests__/useIncrementalTreeData.spec.ts | 135 ++++++++++++++++ .../__tests__/useInfiniteTreeNode.spec.ts | 144 +++++++++++++++++- .../v2/hooks/useIncrementalTreeData.ts | 40 ++++- .../v2/hooks/useInfiniteTreeNode.ts | 53 +++++-- pnpm-lock.yaml | 7 +- 13 files changed, 621 insertions(+), 24 deletions(-) create mode 100644 packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md diff --git a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md index af0a12fce4..749953dda5 100644 --- a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md +++ b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md @@ -12,6 +12,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - We fixed an issue where expanding one node could permanently remove the expand icon from an unrelated, unexpanded node elsewhere in the tree when using a microflow data source. - We fixed an issue where a node's expand icon for a deeper tier would not appear until that node was collapsed and expanded again. - We fixed an issue where, with "Start expanded" set to yes, tree nodes deeper than the second level would not show an expand icon until a parent node was manually collapsed and expanded again. +- We fixed an issue where the tree did not reflect a new data source sort order (for example after changing a sequence attribute) until the page was reopened. +- We fixed an issue where all nodes collapsed when the data source refreshed. Expanded and collapsed nodes now keep their state, and the tree no longer clears while the data source is reloading. +- We fixed an issue where a node did not show that it has children after a child was added to it. Expanding a node now always pre-loads one level ahead, also for children that arrive after the expansion, which restores the missing expand icon on nodes deeper than two levels. ## [3.11.0] - 2026-05-27 diff --git a/packages/pluggableWidgets/tree-node-web/CONTEXT.md b/packages/pluggableWidgets/tree-node-web/CONTEXT.md index be4902d1ec..b2788ff9a5 100644 --- a/packages/pluggableWidgets/tree-node-web/CONTEXT.md +++ b/packages/pluggableWidgets/tree-node-web/CONTEXT.md @@ -24,11 +24,38 @@ Originally (pre-WC-3564) a freshly-created node started in `LOADING` and only le Fix: `LOADING` is no longer stored on the node at all. A node is created directly as `EXPANDED`/`COLLAPSED_WITH_JS` (per `startExpanded`) — never `LOADING`. The spinner is a pure render-time decision in `TreeNode.tsx`: show it when a node has no known children yet (`node.children.length === 0`) **and** `props.datasource.status === ValueStatus.Loading` — Mendix's own, real-time, first-party "is this datasource actually fetching right now" signal. No per-node bookkeeping, so nothing can be "stuck" (the flag is never persisted) and nothing about one node's resolution can affect another's (it's a single global flag, not a per-node mutation). +A node's expanded/collapsed state, on the other hand, _is_ stored — and is remembered across rebuilds. `useIncrementalTreeData.ts` keeps a `statesByIdRef` map of id → `TreeNodeState` and snapshots every node into it just before a rebuild clears the node map, so a re-created node comes back in the state it had rather than in the `startExpanded` default. This is needed because a rebuild happens far more often than "the configuration changed": `isConfigChanged` compares prop instances by reference and the Mendix client hands over fresh instances on every refresh, and any single item deletion legitimately trips the removed-ids check. Neither is avoidable, so the rebuild is made non-destructive instead. A remembered state deliberately beats `startExpanded` in both directions — a node the user collapsed under "Start expanded" = Yes must stay collapsed across a refresh. The map is never pruned: an id that disappears and comes back within the same session (a filter change, a microflow round-trip) is exactly the case it exists to serve. + +Do not reintroduce a `LOADING` arm into that restore path. An earlier version of the state-restore work routed it through a helper whose "nothing remembered" arm returned `TreeNodeState.LOADING` — which is Bug 1's exact mechanism. The restore is a plain fallback: `statesByIdRef.current.get(nodeId) ?? (startExpanded ? EXPANDED : COLLAPSED_WITH_JS)`. + +## The incremental node map reuses nodes across updates — two things that has to re-derive + +`useIncrementalTreeData.ts` does not rebuild the tree from `datasource.items` on each update; it keeps a node map and reuses nodes across deliveries. That is what makes v2's "children of anything expanded so far, accumulated over many filtered fetches" model work at all, but it means anything derived from a _delivery_ rather than from an _item_ has to be re-derived explicitly, or it silently keeps whatever the first delivery said. Two such things, both fixed under WC-3564: + +- **Sibling and root order.** A node used to be appended to `rootsRef`/`parent.children` only on the first delivery its id appeared in; later deliveries took the "already exists" branch, which refreshes `item`/`title`/`parentId` but never touches sibling arrays. So a re-sorted datasource (the real case: a microflow updates the sequence attribute the datasource sorts on) left the tree in its first-load order until the widget remounted. Now an `orderById` map is built from the current delivery's indices and applied as a sort to `rootsRef` and every node's `children` at the _end_ of each update — placement itself is order-independent by design (a child can arrive before its parent, and gets re-placed when the parent shows up), so the end of the pass is the only point where the full delivery order is actually known. A node the current delivery does not mention sorts after every node it does, rather than being shuffled. +- **`items === undefined` is "not delivered yet", not "empty".** The hook used to read `items ?? []`, which made every known id look removed, tripped the removed-ids rebuild, and flashed "no data available" mid-refresh. It now returns early and keeps the tree. This is also what makes the `datasource.status`-driven spinner above correct during a load — the tree stays mounted, so there are real nodes to render a spinner on. + +Contrast with v1, which is unaffected by both: it rebuilds from `datasource.items` on every update, so it re-derives order and emptiness for free. + ## `useInfiniteTreeNode.ts`'s one-level-lookahead preload — content-gated, not fire-count-gated -`appendItems` (called on every click-to-expand) and the bootstrap `useEffect` (auto-expand for `startExpanded = Yes`) both implement the same idea: when a node's children become known, also preload _their_ children's existence one level further ahead, so an already-visible child's own expand affordance is correct without the user needing to click into it first. Both had the same class of bug (WC-3564, found during manual verification, pre-existing on `main`, unrelated to the `LOADING`/`hasChildren` mechanism above): +There are **three** mechanisms here, all implementing the same idea — when a node's children become known, also preload _their_ children's existence one level further ahead, so an already-visible child's own expand affordance is correct without the user needing to click into it first. None of the three is redundant, because each reaches nodes the others cannot: + +| Mechanism | Trigger | Reaches | +| --------------------------------------------------------------------------------------- | ------------------------------- | ----------------------------------------------------------------------- | +| `appendItems` | a real click-to-expand | the clicked node's children and grandchildren | +| bootstrap rounds 1 + 2 (`startExpanded = false`) / the cascade (`startExpanded = true`) | any post-init datasource update | roots, which never pass through `appendItems` | +| late-arrival sweep | any post-init datasource update | children of an already-expanded node that were not known at expand time | + +The sweep exists because `appendItems` only sees the children it is handed: a node expanded while its children are still in flight gets none, and a child a microflow adds later never goes through it at all. It tracks the ids the user has expanded (`expandedIdsRef`) and preloads any delivered item whose `getParentId` is one of them. + +**The bootstrap round gates must not `return` — the sweep sits after them.** `round1DoneRef` fires on the first post-init update whether or not `appendItems` already populated the map, so an early return there swallows the sweep for every node the user expanded before that update. All three mechanisms set a single `shouldRefilter` flag and share one `setFilter` call per pass. The `startExpanded = true` cascade is the one exception and still returns early — it already treats every newly-arrived item as a loaded parent, which subsumes the sweep for that mode. + +**Known gap.** Roots are never added to `expandedIdsRef` under `startExpanded = false`, since they auto-expand via the bootstrap path rather than `appendItems`. A child added to a _root_ after round 2 has locked in is therefore not swept. Deliberately left: closing it means deciding whether the bootstrap path should register roots as "expanded", which changes what the sweep costs on wide trees. + +`appendItems` and the bootstrap effect both had the same class of bug (WC-3564, found during manual verification, pre-existing on `main`, unrelated to the `LOADING`/`hasChildren` mechanism above): -- `appendItems` gated the preload step behind "was this node already a loaded-parent" — true only from a node's _second_ expand onward, so the first expand never preloaded its children's children. A collapse+re-expand of the _same_ node was required to see a deeper tier. **Fixed**: removed that gate — the preload now runs unconditionally whenever children are passed in. Verified live. +- `appendItems` gated the preload step behind "was this node already a loaded-parent" — true only from a node's _second_ expand onward, so the first expand never preloaded its children's children. A collapse+re-expand of the _same_ node was required to see a deeper tier. **Fixed**: removed that gate — the preload now runs unconditionally whenever children are passed in. Verified live. A child that is already a loaded parent (expanded earlier, then collapsed) is skipped, so it cannot end up in both maps and duplicate a parent id in the filter, which is meant to be a set. - The bootstrap effect capped itself at exactly _one_ automatic round ever (`loadedParentsByIdRef.current.size === 0`, true only once) — so `startExpanded = Yes` roots got their own children preloaded, but never one level further. A collapse+re-expand of a _root_ node was required to see a deeper tier. **First attempt at a fix broke live testing**: extended the cap to exactly 2 rounds via a plain counter that advanced on every effect firing. Passed unit tests against a mock, but broke the real "Expanded bug" repro project — the tree stopped rendering anything past the root level. Root-caused with temporary per-widget-tagged debug logging (three tree widgets mount simultaneously on that page regardless of active tab, so untagged logs were unreadable): three tree widgets on the page were logging interleaved, and once tagged, the trace showed both rounds fired — and locked themselves in — while `datasource.items` was still transiently empty during initial load, before the real root items ever arrived. A counter can't tell "fired" apart from "fired with something worth preloading." @@ -37,9 +64,12 @@ Fix: `LOADING` is no longer stored on the node at all. A node is created directl **Final fix**: since every level defaults to `EXPANDED` (not just roots) under `startExpanded = Yes`, replaced the round cap entirely with an unbounded, self-terminating cascade — keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants appear, stop once a round finds nothing new (bounded by the tree's real depth, not a count). Scoped specifically to `startExpanded === true` (confirmed by asking — see decisions log): `startExpanded = false` keeps the original capped round1+round2 behavior unchanged, since deeper tiers there stay collapsed by default and already resolve correctly via a single click. Verified live against the real 4-tier dataset: every tier shows its correct expand affordance automatically, no manual toggle needed anywhere. -**Lesson, still worth keeping in mind**: a change here can look bounded/safe by code inspection and pass every mocked unit test, yet still break against a real datasource, because mocks don't reproduce the transient "still loading, items temporarily empty" window real ones do — and a fix verified against a shallow test dataset can still be wrong at greater depth. Any change to `setFilter` call timing/count in this file needs live verification against a real repro project at real depth, not just mocked unit tests, before being trusted. +**Lesson, still worth keeping in mind**: a change here can look bounded/safe by code inspection and pass every mocked unit test, yet still break against a real datasource, because mocks don't reproduce the transient "still loading, items temporarily empty" window real ones do — and a fix verified against a shallow test dataset can still be wrong at greater depth. Any change to `setFilter` call timing/count in this file needs live verification against a real repro project at real depth, not just mocked unit tests, before being trusted. The `shouldRefilter` restructure that landed the sweep is squarely in that category and is covered by its own live-verification task group. ## Decisions log - 2026-09-15 (WC-3564): initially decided to wire up `props.hasChildren` as the icon-visibility source. **Reverted after live testing crashed the widget** — discovered `hasChildren` is architecturally unavailable in v2 (see above). Corrected to derive `hasChildren` from `node.children.length > 0` (the pre-existing, structurally-correct signal) and drive the spinner from `datasource.status` instead of any per-node stored state. - 2026-09-15 (WC-3564, found during manual verification): discovered and fixed two further pre-existing bugs in `useInfiniteTreeNode.ts`'s one-level-lookahead preload (see above) — bundled into the same change with explicit user sign-off, since found/understood/fixed during the same verification pass. The second one required three attempts: a fire-count-based cap broke live testing and was reverted; a content-based 2-round cap fixed that but was itself too shallow against deeper data; an unbounded content-gated cascade, scoped to `startExpanded = Yes` only (confirmed via explicit question — user rejected applying it to `startExpanded = No` too, since that would eagerly prefetch descendants of still-collapsed, not-yet-visible branches), fixed it correctly at real depth, verified live. +- 2026-09-17 (WC-3564): folded the parallel branch `tmp/treenode-fix1` into this change rather than merging it separately. It fixed two more v2 bugs (datasource order never re-applied; every node collapsing on refresh) plus the late-arrival preload gap, but rewrote the same two functions this change rewrote, so reconciling them was a design decision rather than a merge conflict resolution. Re-applied as fresh commits on top of this change instead of rebasing, because a conflict resolution would have silently landed a shape neither design intended. +- 2026-09-17 (WC-3564): dropped the parallel branch's `resolveRestoredState` helper when folding in its expansion-state fix. Its "nothing remembered" arm returned `TreeNodeState.LOADING`, reintroducing Bug 1's exact mechanism, and its "remembered is `LOADING`" arm was dead code once `LOADING` stopped being stored. Replaced by a plain `?? ` fallback on the node-creation branch. +- 2026-09-17 (WC-3564): removed a stale `NOT YET IMPLEMENTED` requirement from the change's `tree-node-expand-state` spec. It described the reverted first attempt at the bootstrap fix and contradicted the unbounded-cascade requirement in the same file, which the final attempt delivered. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md index 5cd3aff415..0e36ea47c3 100644 --- a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/design.md @@ -67,12 +67,65 @@ While verifying D1/D2 live, manual testing surfaced that a node's expand afforda **Alternative considered**: apply the unbounded cascade regardless of `startExpanded`. Rejected per explicit user decision — would eagerly prefetch descendants of branches that are still collapsed and not visible under `startExpanded = false`, for no user-visible benefit (those tiers already resolve correctly in a single click once actually expanded). **Alternative considered** (superseded by the "why can't we just know without fetching" question — see Context below): skip preloading and derive "has children" from a cheaper existence check. There is no such check available — see Context. +### D4 (added — folding in `tmp/treenode-fix1`): the incremental map re-applies datasource order and remembers expansion state; `LOADING` is not reintroduced to carry it + +A parallel branch fixed two more bugs in `useIncrementalTreeData.ts`, both consequences of the same "build incrementally, reuse nodes across updates" design that D2 also lives in. Folding them in here rather than merging separately, because both branches rewrote the same two functions — the reconciliation below is a design decision, not a textual merge. + +**(a) Datasource order was captured once, never re-applied.** A node is appended to `rootsRef`/`parent.children` only on the first update its id appears in; on later updates it takes the "already exists" branch, which refreshes `item`, `title` and `parentId` but never touches sibling arrays. So a datasource that re-delivers the same items in a new order (the real case: a microflow updates a sequence attribute the datasource sorts on) leaves the tree in its first-load order until the page is reopened and the widget remounts. **Fix**: build an `orderById` map from the current delivery's index, then sort `rootsRef` and every node's `children` by it at the end of each update. Nodes whose id is absent from the current delivery sort to the end (`?? sourceItems.length`), keeping their relative order among themselves rather than being shuffled — a delivery that expresses no order for a node should not reorder it. + +Chosen over re-inserting at the right index during placement: placement is order-independent by design (children can arrive before parents — see the out-of-order handling), so a positional insert would need to be re-derived anyway once the parent shows up. One sort at the end is both simpler and the only point where the full delivery order is actually known. + +**(b) A refresh collapsed every node.** Three things trigger a full rebuild of the node map, and a rebuilt node started collapsed: + +1. `items === undefined` while the datasource loads. The hook read `items ?? []`, which made every previously-known id look removed, tripping `removedIdsDetected` → rebuild → empty tree → "no data available" flashes mid-load. +2. `isConfigChanged` compares prop instances by reference, and the Mendix client hands over new instances on every refresh — so this fires on _every_ refresh, not just genuine configuration changes. +3. Any single item deletion legitimately trips `removedIdsDetected`. + +Cause 1 is fixed at the root: return early when `items` is undefined and keep the tree, which also removes the mid-load empty message. Causes 2 and 3 are _not_ avoided — rebuilding is correct for them, and making `isConfigChanged` structural would mean deep-comparing `ListExpressionValue`/`ListReferenceValue` instances that have no meaningful value equality. Instead the rebuild is made non-destructive: snapshot every node's `treeNodeState` into a `statesByIdRef` map keyed by item id just before clearing, and have node creation prefer a remembered state over the `startExpanded` default. + +**Reconciliation with D2 — the one real conflict.** The parallel branch expressed the restore through a `resolveRestoredState(remembered, startExpanded)` helper whose "nothing remembered" arm returned `TreeNodeState.LOADING`, and whose second arm resolved a _remembered_ `LOADING` into `EXPANDED`/`COLLAPSED_WITH_JS`. Both arms are wrong here, in opposite ways: the first reintroduces stored `LOADING` — exactly WC-3564 Bug 1's mechanism — and the second is dead code, since after D2 no node ever holds `LOADING` to remember. Resolved by dropping the helper entirely; the restore collapses to a single expression on the node-creation branch: + +``` +treeNodeState: statesByIdRef.current.get(nodeId) ?? (config.startExpanded ? EXPANDED : COLLAPSED_WITH_JS) +``` + +The remembered state deliberately wins over `startExpanded` in both directions: a node the user collapsed under "Start expanded" = Yes must stay collapsed across a refresh, which is the whole point of remembering. + +Note that (b) and D2 are complementary rather than overlapping, and it is worth being precise about which bug each one owns, since both are "the tree looks wrong after a refresh": D2 stops a _spinner_ from being stuck on a node; (b) stops a node's _expansion_ from being lost. D2 alone still collapsed the tree on refresh; (b) alone still left spinners stuck. Together, the early return from (b) also makes D2's spinner correct during the load window — the tree stays mounted, so `status === Loading` has real nodes to render a spinner on instead of an empty message. + +### D5 (added — folding in `tmp/treenode-fix1`): the bootstrap effect's round gates must not `return`, or they swallow the late-arrival sweep + +The parallel branch also extended `useInfiniteTreeNode.ts` with a third preload mechanism: track the ids of nodes the user has expanded (`expandedIdsRef`, populated in `appendItems`), and on each subsequent update sweep `datasource.items` for any item whose `parentId` is an expanded id and which isn't tracked in either map yet, preloading it. This covers children that were still in flight when `appendItems` ran (so it received none) and children created later by a microflow — neither of which any existing mechanism reached. + +That sweep and D3's `round1DoneRef`/`round2DoneRef` gates collide, and the collision is invisible to inspection. D3 replaced the original `if (loadedParentsByIdRef.current.size === 0)` gate — which was skipped whenever `appendItems` had already populated the map — with a flag that fires on the _first_ post-init update regardless, and `return`s. The sweep sits after that return. Concretely, for `startExpanded = false`: + +``` +render init -> setFilter(root-only) +appendItems("parent") -> setFilter([root, "parent"]) ("parent" now a loaded parent) +items = [parent, child] arrives + round1Done? no + loadedParents.size === 0? no -> skip populate, size > 0, round1Done = true, setFilter, RETURN + sweep never runs -> filter still [root, "parent"], "child" never preloaded +``` + +**Fix**: no early `return` from round1 or round2. Each mechanism sets a single `shouldRefilter` flag, and one `setFilter` call happens at the end of the pass. Round1 and round2 stay mutually exclusive (`else if`) since round2's premise is that round1's fetch has landed; the sweep runs unconditionally after them. + +The three mechanisms stay distinct and none is redundant: round1 fetches roots' children (roots never pass through `appendItems`, so the sweep cannot see them), round2 preloads one level past that, and the sweep handles everything arriving after a real expand. D3's cascade for `startExpanded = true` is untouched and still returns early — it already treats every newly-arrived item as a loaded parent, which subsumes the sweep for that mode. + +**`appendItems` reconciliation.** Both branches rewrote it and removed the same outer gate; the end state is equivalent apart from two details. Kept D3's shape (it is what this change's `CONTEXT.md` documents) plus the parallel branch's `if (!loadedParentsByIdRef.current.has(childId))` guard on the child loop — without that guard, a child that was expanded earlier and then collapsed ends up in `loadedParentsByIdRef` _and_ `loadedChildsByIdRef`, producing a duplicate parent id in the preload filter. Harmless today, but it makes the filter's contents no longer a set, which is what one of the new tests asserts. + +**Known gap, deliberately not widened.** Root nodes are never added to `expandedIdsRef` under `startExpanded = false`, since they auto-expand via the bootstrap path rather than `appendItems`. So a child added to a _root_ after round2 has locked in is not swept. Pre-existing in the parallel branch too, out of scope here; fixing it means deciding whether the bootstrap path should register roots as "expanded", which changes what the sweep costs on wide trees. + ## Risks / Trade-offs - **[Trade-off]** If a real (non-microflow) datasource's `status` never meaningfully transitions to `Loading` for some fetch (e.g. resolves synchronously from cache), the spinner simply won't show for that fetch — same as the original pre-`LOADING`-commit behavior (brief icon pop-in instead of a spinner). Cosmetic only, not a functional regression. - **[Risk]** None identified that could reproduce either original bug — see D2's "structurally immune" reasoning above. Covered by unit tests asserting spinner-shows-while-loading, spinner-clears-on-settle (whether or not children arrived), and one node's resolution never affecting a sibling's spinner/children/state. - **[Risk]** D3's bootstrap round-2 preload fetches one extra level for every currently-known item, regardless of whether the user has looked at it — a bounded, one-time eager-fetch (proportional to tree _width_ at that level, not depth) → **Mitigation**: capped at exactly 2 rounds via content-based flags, verified live and by a unit test asserting a 3rd datasource change does not trigger a 3rd round. This matches the cost the widget already pays for round 1 (unconditionally preloading roots' children regardless of collapse state) — round 2 is the same category of cost, one level deeper, not a new category of risk. - **[Risk — realized and fixed, kept as a lesson]** A first attempt at this exact fix (a fire-count-based cap) passed every unit test against a mocked datasource but broke a real repro project live, because mocked datasources don't reproduce the transient "still loading, items temporarily empty" window that real ones do. Mitigation going forward: any change to `useInfiniteTreeNode.ts` that touches `setFilter` call timing/count must be live-verified against a real datasource before being trusted, not just unit-tested against a mock. +- **[Risk]** D4's `statesByIdRef` grows for the lifetime of the widget instance and is never pruned — an id whose item is deleted keeps its remembered state. Bounded by the number of distinct ids the datasource has ever delivered to this instance, holding one enum value each, and reset on unmount. → **Mitigation**: none taken deliberately; pruning on removal would break the legitimate case of an id disappearing and returning within the same session (a filter change, or an item re-delivered after a microflow round-trip), which is the case the map exists to serve. +- **[Risk]** D4's per-update sort runs over `rootsRef` plus every node's `children` array on every datasource delivery — O(n log n) across the tree rather than the previous O(1)-per-known-id. → **Mitigation**: the `children.length > 1` guard skips single-child and leaf nodes, which is most of a typical tree; the work is proportional to what the datasource just delivered, which the hook already iterates twice. +- **[Risk]** D5 changes `setFilter` call _timing and count_ in `useInfiniteTreeNode.ts` — precisely the category the lesson above says must not be trusted on mocked unit tests alone. The round1 early return is removed and the three mechanisms now share one call per pass, which is a behavioral change to the exact code path that broke live before. → **Mitigation**: mandatory live re-verification of all four scenarios (both WC-3564 bugs, the "Expanded bug" 4-tier cascade, and the "Collapsed bug" `startExpanded = false` path) against `~/Documents/_tickets/WC-3564-2` before this change is considered done — not just the new tests passing. Tracked as its own task group. +- **[Risk]** D4's early return on `items === undefined` means the widget renders stale nodes for the duration of a load, where it previously rendered an empty message. If a load never completes, the user sees old data with no indication rather than an empty tree. → **Mitigation**: accepted, and this is the intended behavior — D2's spinner is what indicates the in-flight load, and showing an empty tree mid-refresh was the reported bug. ## Migration Plan diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md index 8a8757f2de..fff03cc6f7 100644 --- a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/proposal.md @@ -4,6 +4,8 @@ Tree Node v2 stores `LOADING` as a per-node state and resolves it via a broken h Manual verification of that fix surfaced two further, separate, pre-existing bugs in the same "preload one level ahead of what's expanded" mechanism (`useInfiniteTreeNode.ts`) — unrelated to `LOADING`/`hasChildren`, but bundled into this same change since they were found, understood, and fixed during the same verification pass: a node's first-ever expand didn't preload its own children's children (needed a collapse+re-expand of that same node to reveal a deeper tier), and the automatic root-expansion path for "Start expanded" = Yes had the same gap, recurring at every level. A first attempt at the second fix (a fixed 2-round cap) broke a live repro project and was reverted before being corrected; a second attempt fixed that but, against a deeper (4-tier) dataset, turned out to still be too shallow — every level defaults to expanded under "Start expanded" = Yes, not just roots, so a fixed round count can't be right at all. The final fix replaces the round cap with an unbounded, self-terminating cascade, scoped specifically to "Start expanded" = Yes. See `design.md` D3 for the full story, including the lesson that a fix here needs live verification against a real datasource, not just mocked unit tests. +Separately, a parallel branch (`tmp/treenode-fix1`) fixed two more pre-existing v2 bugs in `useIncrementalTreeData.ts`, both rooted in the same design as the bugs above — the hook builds its node map incrementally and reuses nodes across datasource updates: (a) a node is appended to `rootsRef`/`parent.children` only the first time its id is seen, so a new datasource sort order is never applied until the widget remounts; (b) a refresh throws the tree away and rebuilds it, and rebuilt nodes start collapsed, so every node collapses on refresh (and the tree briefly showed "no data available" while loading, because an undefined `items` was read as "all items removed"). It also extended the `appendItems` preload to cover children that arrive _after_ an expand. That branch is folded into this change rather than merged separately, because it rewrites the exact two functions this change already rewrites — reconciling them is a design decision, not a textual merge. See `design.md` D4 and D5. + ## What Changes - `LOADING` is no longer stored on a node at all. Nodes are created already resolved (`EXPANDED`/`COLLAPSED_WITH_JS` per `startExpanded`); the click-to-expand handler goes back to unconditionally setting `EXPANDED`. @@ -11,13 +13,20 @@ Manual verification of that fix surfaced two further, separate, pre-existing bug - Expand-icon visibility for v2 stays derived from `node.children.length > 0` (unchanged from before this ticket) — **not** from the `hasChildren` widget property. `hasChildren` is hidden by Studio Pro and unset at runtime whenever `parentAssociation` is configured, which is every v2 instance; reading it crashes the widget (confirmed live during this change's implementation). - `useInfiniteTreeNode.ts`'s `appendItems`: removed a gate that skipped preloading a node's grandchildren-existence on its first-ever expand (only ran from the second expand onward). - `useInfiniteTreeNode.ts`'s bootstrap effect: when "Start expanded" is Yes, the preload now cascades level-by-level for as long as new descendants keep appearing (self-terminating once a level introduces nothing new) — matching every level defaulting to expanded in that mode. When "Start expanded" is No, the original capped round1+round2 behavior is unchanged (deeper tiers stay collapsed by default and already resolve correctly via a single real click). +- `useIncrementalTreeData.ts` re-applies the datasource order to `rootsRef` and to every node's `children` on every update, instead of only capturing order at first-sight of an id. +- `useIncrementalTreeData.ts` returns early when `items` is undefined, keeping the tree it already built while the datasource reloads — instead of reading undefined as an empty list, concluding every item was removed, and rebuilding from scratch behind the empty message. +- `useIncrementalTreeData.ts` remembers each node's expanded/collapsed state by id and restores it when a node is re-created during a rebuild, so the rebuilds this change cannot avoid (config-reference churn, item removal) no longer collapse the tree. The remembered state wins over the `startExpanded` default in both directions. +- `useInfiniteTreeNode.ts` tracks which nodes the user has expanded and preloads children that arrive after that expand — whether they were in flight at expand time or created later — so a node whose children arrive late still gets its expand affordance. +- `useInfiniteTreeNode.ts`'s bootstrap effect is restructured so the round1/round2 gates no longer return early: all three preload mechanisms (roots' children, one level past that, and late-arriving children of expanded nodes) run in the same pass and share a single `setFilter` call. See `design.md` D5 — without this, the round1 gate swallows the late-arrival sweep. - Update `TreeNodeV2.spec.tsx`, `useIncrementalTreeData.spec.ts`, and `useInfiniteTreeNode.spec.ts` to cover the corrected behavior. +- Add `@mendix/widget-plugin-test-utils` as an explicit devDependency (already imported by `useInfiniteTreeNode.spec.ts`, previously resolved only via hoisting). ## Capabilities ### New Capabilities -- `tree-node-expand-state`: governs how a v2 tree node decides (a) whether it shows an expand affordance at all, and (b) whether it shows a spinner in place of that affordance, independent of datasource re-delivery timing or datasource type (microflow vs. non-microflow). +- `tree-node-expand-state`: governs how a v2 tree node decides (a) whether it shows an expand affordance at all, (b) whether it shows a spinner in place of that affordance, and (c) whether it is expanded or collapsed — all independent of datasource re-delivery timing or datasource type (microflow vs. non-microflow). +- `tree-node-data-refresh`: governs how the v2 incremental tree map reacts to a datasource update — sibling and root ordering following the current datasource order, and the tree surviving a reload rather than being torn down and rebuilt. ### Modified Capabilities @@ -26,8 +35,9 @@ Manual verification of that fix surfaced two further, separate, pre-existing bug ## Impact - `src/components/v2/TreeNode.tsx` — icon-render condition now spinner-vs-chevron based on `datasource.status`; click handler simplified (no `LOADING` entry). -- `src/components/v2/hooks/useIncrementalTreeData.ts` — nodes created pre-resolved; no `LOADING` assignment anywhere in this file. -- `src/components/v2/hooks/useInfiniteTreeNode.ts` — `appendItems`'s preload gate removed; bootstrap effect's preload is now an unbounded, content-gated cascade when `startExpanded` is Yes, and unchanged (capped round1+round2) when it's No. +- `src/components/v2/hooks/useIncrementalTreeData.ts` — nodes created pre-resolved; no `LOADING` assignment anywhere in this file; new-node state falls back to a remembered per-id state before the `startExpanded` default; early return while `items` is undefined; datasource order re-applied to roots and to every node's children on every update. +- `src/components/v2/hooks/useInfiniteTreeNode.ts` — `appendItems`'s preload gate removed and its child loop deduplicated against already-known parents; expanded-node ids tracked; bootstrap effect's preload is an unbounded, content-gated cascade when `startExpanded` is Yes, and a restructured (no early return, single `setFilter`) round1 + round2 + late-arrival sweep when it's No. - `typings/TreeNodeProps.d.ts` — no change. +- `package.json` / `pnpm-lock.yaml` — `@mendix/widget-plugin-test-utils` added as an explicit devDependency. - `src/components/v2/__tests__/TreeNodeV2.spec.tsx`, `src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts`, `src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts` — updated/added regression tests. -- No XML property changes. `hasChildren` remains untouched for v1 (unaffected by this change). +- No XML property changes. `hasChildren` remains untouched for v1. The v1 code path is unaffected throughout: it rebuilds from `datasource.items` on every update, so it never exhibited the ordering or expansion-state bugs either. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md new file mode 100644 index 0000000000..8015d9ef1c --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: Sibling and root order always follows the current datasource order + +The v2 Tree Node widget SHALL order root nodes, and each node's children, according to the order in which the datasource currently delivers those items — re-applied on every datasource update, not captured once when a node's id is first seen. A node whose incremental tree map already contains an id MUST still be re-positioned among its siblings when the datasource's order for that id changes. + +#### Scenario: Root nodes are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same root items in a different order (for example after a microflow changed a sequence attribute that the datasource sorts on) +- **THEN** the widget renders the root nodes in the new datasource order, without requiring the page to be reopened or the widget to remount + +#### Scenario: A node's children are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same child items of an already-known parent in a different order +- **THEN** the widget renders that parent's children in the new datasource order + +#### Scenario: Re-ordering does not disturb node state + +- **WHEN** sibling order changes across a datasource update +- **THEN** each node keeps its own expanded/collapsed state, its title, and its already-placed children — only its position among its siblings changes + +#### Scenario: A node absent from the current delivery keeps a stable position + +- **WHEN** a node is present in the tree but its id is not in the current datasource delivery (so the datasource expresses no order for it) +- **THEN** the widget keeps that node ordered after every node the current delivery does order, preserving the absent nodes' relative order among themselves rather than reordering them arbitrarily + +### Requirement: The rendered tree is preserved while the datasource is reloading + +The v2 Tree Node widget SHALL treat an undefined `datasource.items` as "not yet delivered" and keep the tree it has already built, rather than interpreting it as an empty result. A reload MUST NOT clear the tree, discard the node map, or surface the empty-message state for the duration of the load. + +#### Scenario: A reload does not empty the tree + +- **WHEN** the datasource starts reloading and `items` becomes undefined +- **THEN** the widget keeps rendering the nodes it had before the reload, with their existing expanded/collapsed state, and does not show the "no data available" empty message + +#### Scenario: An undefined delivery is not mistaken for removed items + +- **WHEN** `items` is undefined +- **THEN** the widget does not conclude that every previously-known item was removed, and therefore does not trigger a rebuild of the node map for that reason + +#### Scenario: The tree updates once the reload settles + +- **WHEN** the reload completes and the datasource delivers items again +- **THEN** the widget applies the new items — including any additions, removals, and the current datasource order — to the tree it preserved diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md index 8fb0ba89a2..00ca6ec6b0 100644 --- a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md @@ -80,6 +80,45 @@ The v2 Tree Node widget SHALL NOT auto-cascade the preload beyond the existing c - **WHEN** "Start expanded" is No and a 3rd-tier item arrives as a result of the existing 2-round preload - **THEN** no further automatic preload round is triggered for it — expanding it further still requires a real click -### Requirement: Auto-expanded root nodes (`startExpanded = Yes`) — NOT YET IMPLEMENTED +### Requirement: A child that arrives after its parent was expanded still restores that parent's expand affordance -The equivalent one-level lookahead for automatically auto-expanded root nodes (so a root's children already show their correct expand affordance without needing the root collapsed and re-expanded) was attempted and reverted after it broke a live repro project (see `design.md` D3). No requirement is claimed here for this case. A collapse+re-expand of a root node is currently still needed to reveal a 3rd tier under `startExpanded = Yes`; this is a known, pre-existing, unfixed gap, tracked for a future change once root-caused. +The v2 Tree Node widget SHALL keep preloading one level ahead for nodes the user has already expanded, so a child that becomes known only after the expand — because it was still in flight at expand time, or because it was created later — is still preloaded, and the affected node's expand affordance is still correct. This applies whether or not the node's children were already known when `appendItems` ran for it. + +#### Scenario: Children still in flight at expand time are preloaded once they arrive + +- **WHEN** a user expands a node whose children have not been delivered yet, and those children arrive in a later datasource delivery +- **THEN** the widget preloads those children's own children, so each arriving child shows its correct expand affordance without the user collapsing and re-expanding the parent + +#### Scenario: A child created after the expand is preloaded + +- **WHEN** a node is already expanded with known children, and a microflow adds a further child to that node +- **THEN** the widget preloads the newly-added child's own children, so the new child shows its correct expand affordance as soon as it is rendered + +#### Scenario: Repeated deliveries do not re-request the same parent + +- **WHEN** a datasource delivery introduces no children that are not already tracked as a known parent or a known preloaded child +- **THEN** no further preload request is issued for that delivery, and no parent id appears more than once in the preload filter + +### Requirement: A node's expanded or collapsed state survives a tree rebuild + +The v2 Tree Node widget SHALL remember each node's expanded/collapsed state by item id and restore it when that node is re-created during a rebuild of the incremental node map, so a datasource refresh never silently collapses the tree the user had opened. A remembered state MUST take precedence over the `startExpanded` default, in both directions. + +#### Scenario: Expansion survives new prop instances on refresh + +- **WHEN** the Mendix client hands the widget new prop instances on a refresh (which the widget compares by reference and therefore treats as a configuration change, rebuilding the node map) +- **THEN** every re-created node comes back with the expanded/collapsed state it had before the rebuild, not with the `startExpanded` default + +#### Scenario: Expansion of remaining nodes survives an item removal + +- **WHEN** a single item is deleted from the datasource, triggering a rebuild of the node map +- **THEN** the remaining nodes come back with the expanded/collapsed state they had before the removal + +#### Scenario: A user-collapsed node stays collapsed even when "Start expanded" is Yes + +- **WHEN** "Start expanded" is Yes, the user collapses a node, and a later refresh rebuilds the node map +- **THEN** that node comes back collapsed — the remembered state wins over the `startExpanded` default + +#### Scenario: A node never seen before still follows the configured default + +- **WHEN** an item id appears that has no remembered state (a genuinely new node) +- **THEN** that node is created directly in the state `startExpanded` dictates (`EXPANDED` or `COLLAPSED_WITH_JS`) — never in a stored `LOADING` state diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md index c0e8656837..50797e5712 100644 --- a/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/fix-tree-node-loading-state/tasks.md @@ -48,3 +48,69 @@ Pre-existing on `main`, in `useInfiniteTreeNode.ts`'s second `useEffect` block - [x] 7.6 **Follow-up finding, same session:** the 2-round cap turned out insufficient — with a 4-tier dataset, the 3rd tier appeared as content but without its own expand icon (needed a real click on its own ancestor to reveal, one level deeper than the original repro). Root cause: under `startExpanded = Yes`, _every_ level defaults to `EXPANDED` (not just roots), so every level needs the same automatic preload treatment, not just a fixed 2 rounds. Fixed by replacing the 2-round cap with an unbounded, self-terminating cascade **gated specifically on `startExpanded === true`**: keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants keep appearing, stopping naturally once a round finds nothing new (bounded by the tree's real depth, not an arbitrary count). Explicitly scoped to `startExpanded = true` only, per user decision — `startExpanded = false` keeps the original capped round1+round2 behavior unchanged (deeper tiers there already resolve correctly via a single real click, per group 6's fix; auto-cascading for still-collapsed branches would just be wasted eager-fetching of content the user hasn't opened). - [x] 7.7 Updated the unit test from 7.4 to match: renamed/rewritten as an unbounded-cascade test asserting 3+ sequential levels each trigger exactly one more `setFilter` call as they arrive, and a repeated/empty delivery triggers none. Added a second test confirming `startExpanded = false` still caps at exactly 2 rounds (3rd-tier arrival triggers no further automatic call). - [x] 7.8 Verified live again: rebuilt against `~/Documents/_tickets/WC-3564-2`. "Expanded bug" tab (4 levels of data) now shows all 4 tiers automatically on load, with `Third level 1a1` already showing its own expand icon with zero manual interaction — matching the exact screenshot the user originally showed as the expected/desired result. Re-ran the full rigorous Bug 1 / Bug 2 / group-6 / "Collapsed bug" (startExpanded=false, 2-round-cap) regression suite live — all still pass, no regressions. + +## 8. Planning: fold in `tmp/treenode-fix1` (scope extension) + +The parallel branch `tmp/treenode-fix1` (commits `76bfb5b94`, `362bbd396`) fixed two further v2 bugs plus a preload gap, rewriting the same two functions groups 1-7 rewrote. Folded into this change rather than merged separately — see `design.md` D4/D5 for why, and for the four collision points. + +- [x] 8.1 Analysed the overlap between this change and `tmp/treenode-fix1`; identified four collisions (stored `LOADING` vs. restored state, the `items === undefined` guard, sibling ordering, and the `appendItems`/bootstrap-effect rewrites) and confirmed only the first and last need a design decision. +- [x] 8.2 Extended `proposal.md` (Why / What Changes / Capabilities / Impact) to cover the folded-in scope. +- [x] 8.3 Added `design.md` D4 (ordering + expansion-state survival, and why `resolveRestoredState` is dropped) and D5 (bootstrap effect restructure, `appendItems` reconciliation, known root-sweep gap), plus four new Risks entries. +- [x] 8.4 Added the `tree-node-data-refresh` capability spec (ordering, reload survival) and two requirements to `tree-node-expand-state` (late-arriving children restore the affordance; expansion state survives a rebuild). +- [x] 8.5 Removed the stale `Auto-expanded root nodes (startExpanded = Yes) — NOT YET IMPLEMENTED` requirement from `specs/tree-node-expand-state/spec.md`. It described the reverted first attempt from task 7.1 and directly contradicted the unbounded-cascade requirement three blocks above it, which task 7.6 delivered. +- [x] 8.6 Base the implementation branch on this change's branch (not on `tmp/treenode-fix1`) and re-apply the parallel branch's three behaviours as fresh commits — `git rebase` would conflict on both hooks, and collisions 1 and 4 are genuine rewrites rather than textual conflicts, so a rebase resolution would silently pick a shape neither design intended. Keep `tmp/treenode-fix1` intact as the reference. + +## 9. Fifth finding: datasource sort order never re-applied (D4a) + +- [x] 9.1 In `useIncrementalTreeData.ts`, build an `orderById` map from the current delivery (`getItemId(item) -> index`) alongside the existing `incomingIds` set. +- [x] 9.2 At the end of the update, sort `rootsRef.current` and every node's `children` by that order. Nodes absent from the current delivery sort to the end via `?? sourceItems.length`, keeping their relative order among themselves. +- [x] 9.3 Guard the per-node sort on `children.length > 1` so leaves and single-child nodes are skipped. +- [x] 9.4 Confirm sorting happens after all placement (including the out-of-order child-before-parent path) and before `setTreeData`, so a node placed late in the same pass is still ordered correctly. + +## 10. Sixth finding: every node collapses on a datasource refresh (D4b) + +- [x] 10.1 Return early from the effect when `items === undefined`, keeping the existing tree — instead of `items ?? []`, which made every known id look removed and tripped a rebuild behind the empty message. Verify the mid-load "no data available" flash is gone. +- [x] 10.2 Add a `statesByIdRef: Map` and snapshot every node's `treeNodeState` into it immediately before the `configChanged || removedIdsDetected` rebuild clears the maps. +- [x] 10.3 On the node-creation branch, prefer the remembered state over the `startExpanded` default: `statesByIdRef.current.get(nodeId) ?? (config.startExpanded ? EXPANDED : COLLAPSED_WITH_JS)`. +- [x] 10.4 **Do not** port the parallel branch's `resolveRestoredState` helper. Its "nothing remembered" arm returns `TreeNodeState.LOADING`, which reintroduces WC-3564 Bug 1's exact mechanism (see task 1.1/1.2), and its "remembered is `LOADING`" arm is dead code after group 1. See `design.md` D4. +- [x] 10.5 Confirm the remembered state wins over `startExpanded` in both directions — a node collapsed by the user under "Start expanded" = Yes must come back collapsed. +- [x] 10.6 Leave `isConfigChanged`'s reference comparison as-is; rebuilding on prop-instance churn is correct once the rebuild is non-destructive, and `ListExpressionValue`/`ListReferenceValue` have no meaningful structural equality to compare instead. + +## 11. Seventh finding: children arriving after an expand are never preloaded (D5) + +- [x] 11.1 Add an `expandedIdsRef: Set` to `useInfiniteTreeNode.ts`, populated in `appendItems` with the expanded node's id, and reset it in the `initializedRef` init block alongside `round1DoneRef`/`round2DoneRef`. +- [x] 11.2 Add the late-arrival sweep to the post-init effect: for each `datasource.items` entry not tracked in either map, if `getParentId(item, parentAssociation)` is in `expandedIdsRef`, add it to `loadedChildsByIdRef` and mark a refilter as needed. +- [x] 11.3 **Restructure the `startExpanded = false` path so round1 and round2 no longer `return`.** Replace their individual `setFilter` calls with a single `shouldRefilter` flag and one `setFilter` at the end of the pass; keep round1/round2 mutually exclusive (`else if`); run the sweep unconditionally after them. Without this, round1's early return swallows the sweep entirely — see `design.md` D5 for the concrete trace. +- [x] 11.4 Leave D3's `startExpanded = true` cascade untouched, including its early return — it already treats every newly-arrived item as a loaded parent, which subsumes the sweep for that mode. +- [x] 11.5 Add `parentAssociation` to the effect's dependency array (now read via `getParentId`) and import `getParentId` from `./helpers`. +- [x] 11.6 Keep group 6's `appendItems` shape, but add the parallel branch's `if (!loadedParentsByIdRef.current.has(childId))` guard to the child loop — without it a previously-expanded-then-collapsed child lands in both maps and duplicates a parent id in the preload filter. +- [x] 11.7 Note in review that roots are still never added to `expandedIdsRef` under `startExpanded = false`, so a child added to a _root_ after round2 locks in is not swept. Pre-existing in the parallel branch, deliberately out of scope — see `design.md` D5. + +## 12. Test reconciliation + +Both branches edited `useIncrementalTreeData.spec.ts` and `useInfiniteTreeNode.spec.ts`. The merged model changes what several tests can assert, so these are ported deliberately rather than concatenated. + +- [x] 12.1 Port the parallel branch's three ordering tests (roots reordered, children reordered, expansion state preserved across a reorder). The third asserted `EXPANDED` only after a second render because nodes used to start in `LOADING`; under group 1 they are `EXPANDED` from the first render, so the extra `rerender` is now redundant rather than required. +- [x] 12.2 Port the parallel branch's four expansion-survival tests (reload with `items: undefined`, config-instance churn, item removal, user-collapsed node under `startExpanded: true`). +- [x] 12.3 Keep group 3's rewritten `LOADING` tests as the authority on node-creation state — do **not** restore the parallel branch's copies, which still assert `LOADING` on first render. +- [x] 12.4 Port the parallel branch's `requestedParentIds(setFilter)` helper and its `appendItems` preload tests, including the "does not ask for the same parent twice" test that asserts the filter's parent ids are a set (this is what task 11.6's guard protects). +- [x] 12.5 Port the two late-arrival tests. Confirm the first one — "pre-loads children that were not known when the node was expanded" — passes only after task 11.3's restructure; it is the test that fails on a naive merge, and it is the regression test for that specific collision. +- [x] 12.6 Re-run group 7.7's two cascade tests unchanged and confirm both still pass: `startExpanded = true` unbounded cascade, and `startExpanded = false` capped at exactly 2 rounds. The latter is the one at risk from task 11.3 — verify the sweep is a genuine no-op there (no `appendItems`, so `expandedIdsRef` is empty) rather than merely appearing to pass. +- [x] 12.7 Add `@mendix/widget-plugin-test-utils` to `devDependencies` and refresh `pnpm-lock.yaml`. `useInfiniteTreeNode.spec.ts` already imports it (and now needs `dynamic` as well as `listReference`); it resolved via hoisting only. +- [x] 12.8 Run the full package suite (`pnpm run test`) and confirm it is green, then `pnpm run lint`. + +## 13. Live re-verification (mandatory — D5 changes `setFilter` timing) + +Task 11.3 changes `setFilter` call timing and count in `useInfiniteTreeNode.ts`, which is exactly the category task 7.1 proved cannot be trusted on mocked unit tests alone. All four scenarios must be re-confirmed live against `~/Documents/_tickets/WC-3564-2` before this change is done. + +- [ ] 13.1 Rebuild against the repro project and re-confirm WC-3564 Bug 1 (microflow datasource, "Start expanded" = Yes): fully expanded on load, zero `.widget-tree-node-loading-spinner` elements, no console errors. +- [ ] 13.2 Re-confirm WC-3564 Bug 2: expanding "Top level 2" leaves the sibling nodes' expand icons intact. +- [ ] 13.3 Re-confirm the "Expanded bug" tab's 4-tier cascade still resolves fully on load with no manual toggling. +- [ ] 13.4 Re-confirm the "Collapsed bug" tab (`startExpanded = false`): single-click expand still reveals a deeper tier's icon, and the restructured round1/round2/sweep pass does not over-fetch. +- [ ] 13.5 Verify the two newly-fixed bugs live: change a sequence attribute via a microflow and confirm the tree reorders without reopening the page; expand several nodes, trigger a refresh, and confirm they stay expanded and the tree does not flash the empty message. + +## 14. Changelog and domain notes + +- [x] 14.1 Merge the parallel branch's three `CHANGELOG.md` entries into this change's existing `[Unreleased] / Fixed` block — user-visible behaviour only, no implementation details. +- [x] 14.2 Update `CONTEXT.md`: the `LOADING` section should state that expansion state is now remembered by id across rebuilds, and the preload section should describe the third mechanism (the late-arrival sweep) alongside `appendItems` and the bootstrap rounds, including why the round gates must not return early. +- [x] 14.3 Add a decisions-log entry to `CONTEXT.md` recording that `tmp/treenode-fix1` was folded into this change rather than merged separately, and that `resolveRestoredState` was deliberately dropped. diff --git a/packages/pluggableWidgets/tree-node-web/package.json b/packages/pluggableWidgets/tree-node-web/package.json index 524b8e3816..1e401cf6e7 100644 --- a/packages/pluggableWidgets/tree-node-web/package.json +++ b/packages/pluggableWidgets/tree-node-web/package.json @@ -50,6 +50,7 @@ "@mendix/pluggable-widgets-tools": "*", "@mendix/prettier-config-web-widgets": "workspace:*", "@mendix/run-e2e": "workspace:*", - "@mendix/widget-plugin-platform": "workspace:*" + "@mendix/widget-plugin-platform": "workspace:*", + "@mendix/widget-plugin-test-utils": "workspace:*" } } diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts index 27ef1471fd..1a1eb33fcb 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts @@ -208,6 +208,141 @@ describe("useIncrementalTreeData", () => { }); }); + describe("reordering (datasource sort order changes)", () => { + it("reorders roots when the datasource returns them in a different order", () => { + const config = makeConfig(); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: [makeItem("a"), makeItem("b"), makeItem("c")] } } + ); + + expect(result.current.map(n => n.id)).toEqual(["a", "b", "c"]); + + // same items, new sort order (e.g. sequence attribute changed) + rerender({ items: [makeItem("c"), makeItem("a"), makeItem("b")] }); + + expect(result.current.map(n => n.id)).toEqual(["c", "a", "b"]); + }); + + it("reorders children when the datasource returns them in a different order", () => { + const config = makeConfigWithParentMap({ parent: undefined, c1: "parent", c2: "parent", c3: "parent" }); + const makeItems = (childIds: string[]): ObjectItem[] => [ + makeItem("parent"), + ...childIds.map(id => makeItem(id)) + ]; + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: makeItems(["c1", "c2", "c3"]) } } + ); + + expect(result.current[0].children.map(n => n.id)).toEqual(["c1", "c2", "c3"]); + + rerender({ items: makeItems(["c3", "c1", "c2"]) }); + + expect(result.current[0].children.map(n => n.id)).toEqual(["c3", "c1", "c2"]); + }); + + it("keeps expansion state while reordering", () => { + const config = makeConfig({ startExpanded: true }); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: [makeItem("a"), makeItem("b")] } } + ); + + expect(result.current.every(n => n.treeNodeState === TreeNodeState.EXPANDED)).toBe(true); + + rerender({ items: [makeItem("b"), makeItem("a")] }); + + expect(result.current.map(n => n.id)).toEqual(["b", "a"]); + expect(result.current.every(n => n.treeNodeState === TreeNodeState.EXPANDED)).toBe(true); + }); + }); + + describe("expansion state survives a data refresh", () => { + // Nodes are expanded by mutating treeNodeState, which is what TreeNodeV2 does on click. + function expand(node: { treeNodeState: TreeNodeState }): void { + node.treeNodeState = TreeNodeState.EXPANDED; + } + + it("keeps the tree while the datasource is reloading", () => { + const config = makeConfig(); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] | undefined }) => useIncrementalTreeData(items, config), + { initialProps: { items: [makeItem("a"), makeItem("b")] } as { items: ObjectItem[] | undefined } } + ); + + expand(result.current[0]); + + // datasource reloading: items is undefined until the new data arrives + rerender({ items: undefined }); + + expect(result.current.map(n => n.id)).toEqual(["a", "b"]); + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + + rerender({ items: [makeItem("a"), makeItem("b")] }); + + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + }); + + it("keeps expansion state when config objects are recreated with the same values", () => { + const items = [makeItem("a"), makeItem("b")]; + + const { result, rerender } = renderHook( + ({ items, config }: { items: ObjectItem[]; config: TreeConfigRef }) => + useIncrementalTreeData(items, config), + { initialProps: { items, config: makeConfig() } } + ); + + expand(result.current[0]); + + // Mendix hands over new prop instances on every refresh, which forces a rebuild + rerender({ items: [makeItem("a"), makeItem("b")], config: makeConfig() }); + + expect(result.current.map(n => n.id)).toEqual(["a", "b"]); + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + expect(result.current[1].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + }); + + it("keeps expansion state of remaining nodes when an item is removed", () => { + const config = makeConfig(); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: [makeItem("a"), makeItem("b"), makeItem("c")] } } + ); + + expand(result.current[0]); + + rerender({ items: [makeItem("a"), makeItem("c")] }); + + expect(result.current.map(n => n.id)).toEqual(["a", "c"]); + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + }); + + it("keeps a node collapsed after a refresh even when startExpanded is true", () => { + const config = makeConfig({ startExpanded: true }); + + const { result, rerender } = renderHook( + ({ items, config }: { items: ObjectItem[]; config: TreeConfigRef }) => + useIncrementalTreeData(items, config), + { initialProps: { items: [makeItem("a")], config } } + ); + + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + + result.current[0].treeNodeState = TreeNodeState.COLLAPSED_WITH_CSS; + + // full rebuild (new config instance, same values) + rerender({ items: [makeItem("a")], config: makeConfig({ startExpanded: true }) }); + + expect(result.current[0].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_CSS); + }); + }); + describe("deep nesting", () => { it("builds a three-level tree correctly", () => { const grandparent = makeItem("gp"); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts index df9b2539ba..f9cf0f090a 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts @@ -1,7 +1,7 @@ import { act, renderHook } from "@testing-library/react"; import { ObjectItem } from "mendix"; import * as FilterBuilders from "mendix/filters/builders"; -import { listReference } from "@mendix/widget-plugin-test-utils"; +import { dynamic, listReference } from "@mendix/widget-plugin-test-utils"; import { TreeNodeContainerProps } from "../../../../../typings/TreeNodeProps"; import { useInfiniteTreeNodes } from "../useInfiniteTreeNode"; @@ -20,6 +20,18 @@ function makeSetFilter(): jest.Mock { return jest.fn(); } +/** Ids of the parents the last setFilter call asks children for. undefined means root level. */ +function requestedParentIds(setFilter: unknown): Array { + const calls = (setFilter as jest.Mock).mock.calls; + const read = (expression: any): Array => { + if (expression?.type === "or") { + return expression.args.flatMap(read); + } + return [expression?.b?.v?.id]; + }; + return read(calls[calls.length - 1][0]); +} + function makeProps(overrides: Partial = {}): TreeNodeContainerProps { const setFilter = makeSetFilter(); return { @@ -142,6 +154,136 @@ describe("useInfiniteTreeNodes", () => { }); }); + describe("appendItems — pre-loading one level ahead", () => { + it("asks for the children of the expanded node and of its children", () => { + const props = makeProps(); + const { result } = renderHook(() => useInfiniteTreeNodes(props)); + + act(() => { + result.current.appendItems(makeItem("root"), [makeItem("child")]); + }); + + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "child"]); + }); + + it("keeps pre-loading when a node that was itself a pre-loaded child gets expanded", () => { + const props = makeProps(); + const { result } = renderHook(() => useInfiniteTreeNodes(props)); + + act(() => { + result.current.appendItems(makeItem("root"), [makeItem("child")]); + }); + act(() => { + result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); + }); + + // without the grandchild in the filter, "child"'s children can never report + // whether they have children of their own + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "child", "grandchild"]); + }); + + it("does not ask for the same parent twice", () => { + const props = makeProps(); + const { result } = renderHook(() => useInfiniteTreeNodes(props)); + + act(() => { + result.current.appendItems(makeItem("root"), [makeItem("child")]); + }); + act(() => { + result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); + }); + act(() => { + result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); + }); + + const requested = requestedParentIds(props.datasource.setFilter); + expect(requested).toEqual([...new Set(requested)]); + }); + + it("asks for the children of a node expanded while it has none yet", () => { + const props = makeProps(); + const { result } = renderHook(() => useInfiniteTreeNodes(props)); + + act(() => { + result.current.appendItems(makeItem("leaf"), []); + }); + + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "leaf"]); + }); + }); + + describe("children arriving after the expansion", () => { + function makePropsWithParents( + parentMap: Record, + items: ObjectItem[] + ): TreeNodeContainerProps { + const props = makeProps({ + parentAssociation: listReference(b => + b + .withId("assoc_1") + .withGet((item: ObjectItem) => { + const parentId = parentMap[String(item.id)]; + return parentId ? dynamic.available(makeItem(parentId)) : dynamic.unavailable(); + }) + .build() + ) + }); + return { ...props, datasource: { ...props.datasource, items } as any } as TreeNodeContainerProps; + } + + it("pre-loads children that were not known when the node was expanded", () => { + // node expanded while its children are still in flight, so appendItems gets none + let props = makePropsWithParents({ parent: undefined }, []); + const setFilter = props.datasource.setFilter; + + const { result, rerender } = renderHook(({ p }: { p: TreeNodeContainerProps }) => useInfiniteTreeNodes(p), { + initialProps: { p: props } + }); + + act(() => { + result.current.appendItems(makeItem("parent")); + }); + expect(requestedParentIds(setFilter)).toEqual([undefined, "parent"]); + + // the children arrive in a later datasource update + props = makePropsWithParents({ parent: undefined, child: "parent" }, [ + makeItem("parent"), + makeItem("child") + ]); + (props.datasource as any).setFilter = setFilter; + rerender({ p: props }); + + expect(requestedParentIds(setFilter)).toEqual([undefined, "parent", "child"]); + }); + + it("pre-loads a child added to an already expanded node", () => { + let props = makePropsWithParents({ parent: undefined, child: "parent" }, [ + makeItem("parent"), + makeItem("child") + ]); + const setFilter = props.datasource.setFilter; + + const { result, rerender } = renderHook(({ p }: { p: TreeNodeContainerProps }) => useInfiniteTreeNodes(p), { + initialProps: { p: props } + }); + + act(() => { + result.current.appendItems(makeItem("parent"), [makeItem("child")]); + }); + + // a microflow adds a second child later + props = makePropsWithParents({ parent: undefined, child: "parent", added: "parent" }, [ + makeItem("parent"), + makeItem("child"), + makeItem("added") + ]); + (props.datasource as any).setFilter = setFilter; + rerender({ p: props }); + + expect(requestedParentIds(setFilter)).toContain("added"); + }); + }); + describe("appendItems — re-expanding already-expanded node", () => { it("does not add duplicate entries when same parent expanded twice", () => { const parentItem = makeItem("parent"); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts index 27c42b4146..a5c46120a5 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts @@ -31,10 +31,24 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: const placementByIdRef = useRef>(new Map()); const previousIdsRef = useRef>(new Set()); const previousConfigRef = useRef(null); + // Expanded/collapsed state per item id, kept across rebuilds so a data refresh does not + // collapse the tree. A rebuild is unavoidable for a config-reference change or a removed + // item, but it does not have to be destructive. + const statesByIdRef = useRef>(new Map()); useEffect(() => { - const sourceItems = items ?? []; + if (items === undefined) { + // Datasource is (re)loading. Keep the tree we already built instead of reading + // undefined as an empty list, which would look like every item was removed. + return; + } + + const sourceItems = items; const incomingIds = new Set(sourceItems.map(getItemId)); + // The datasource order (e.g. a sort on a sequence attribute) is the source of truth for + // root and sibling order, and has to be re-applied on every update, not only when an id + // is first seen. + const orderById = new Map(sourceItems.map((item, index) => [getItemId(item), index])); const removedIdsDetected = incomingIds.size < previousIdsRef.current.size || @@ -44,6 +58,9 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: previousConfigRef.current = config; if (configChanged || removedIdsDetected) { + for (const node of nodesByIdRef.current.values()) { + statesByIdRef.current.set(node.id, node.treeNodeState); + } rootsRef.current = []; nodesByIdRef.current.clear(); placementByIdRef.current.clear(); @@ -119,7 +136,11 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: id: nodeId, item, parentId: nextParentId, - treeNodeState: config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS, + // A remembered state wins over the configured default in both directions: a node + // the user collapsed under "Start expanded" = Yes must come back collapsed. + treeNodeState: + statesByIdRef.current.get(nodeId) ?? + (config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS), title: nextTitle }; nodesByIdRef.current.set(nodeId, newNode); @@ -133,6 +154,21 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: } } + // Re-apply the datasource order once all placement is done — placement itself is + // order-independent by design (a child can arrive before its parent), so this is the only + // point where the full delivery order is known. A node the current delivery does not + // mention sorts after every node it does, keeping the unmentioned nodes' relative order. + const orderOf = (node: TreeNodeV2DataItem): number => orderById.get(node.id) ?? sourceItems.length; + const compareByDatasourceOrder = (a: TreeNodeV2DataItem, b: TreeNodeV2DataItem): number => + orderOf(a) - orderOf(b); + + rootsRef.current.sort(compareByDatasourceOrder); + for (const node of nodesByIdRef.current.values()) { + if (node.children.length > 1) { + node.children.sort(compareByDatasourceOrder); + } + } + previousIdsRef.current = incomingIds; setTreeData([...rootsRef.current]); }, [items, config]); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts index daaea000b9..0e8de65c68 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts @@ -1,7 +1,7 @@ import { ObjectItem, Option } from "mendix"; import { association, equals, literal, or } from "mendix/filters/builders"; import { useCallback, useEffect, useRef } from "react"; -import { getItemId } from "./helpers"; +import { getItemId, getParentId } from "./helpers"; import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps"; export type ItemType = Array>; @@ -15,6 +15,10 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { const loadedParentsByIdRef = useRef>(new Map()); // loadedChilds : track the pre-loaded nodes of expanded nodes. const loadedChildsByIdRef = useRef>(new Map()); + // expandedIds : nodes the user has opened. Their children have to be pre-loaded as they + // arrive, which is not always at expand time — they can still be in flight then, or be added + // later on by a microflow. + const expandedIdsRef = useRef>(new Set()); const initializedRef = useRef(false); // Used only when startExpanded is false (only roots auto-expand; deeper tiers resolve via a // real click through appendItems). Round 1 (pre-existing): preload roots' children, gated on @@ -46,6 +50,7 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { const appendItems = useCallback( (newItem: ObjectItem, children?: ObjectItem[]) => { const parentId = getItemId(newItem); + expandedIdsRef.current.add(parentId); if (children && children.length > 0) { children.forEach(child => { @@ -54,7 +59,12 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { // this is needed to be able to know if a node has further level children before expanding it. // Runs on every expand, including the first one — a node's own children being // preloaded as part of its parent's expand must not delay preloading its grandchildren too. - loadedChildsByIdRef.current.set(childId, child); + // Skip a child that is already a loaded parent (expanded earlier, then + // collapsed) — it would end up in both maps and duplicate a parent id in the + // filter, which is meant to be a set. + if (!loadedParentsByIdRef.current.has(childId)) { + loadedChildsByIdRef.current.set(childId, child); + } }); } @@ -95,6 +105,12 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { return; } + // The three mechanisms below all run in the same pass and share one setFilter call. + // None of them may return early: round 1 fires on the first post-init update whether + // or not appendItems already populated the map, so returning from it would swallow + // the late-arrival sweep for every node the user expanded before that update. + let shouldRefilter = false; + if (!round1DoneRef.current) { // after the first load of the datasource, // we want to pre-load the child nodes of roots @@ -107,11 +123,8 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { if (loadedParentsByIdRef.current.size > 0) { round1DoneRef.current = true; } - datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); - return; - } - - if (!round2DoneRef.current) { + shouldRefilter = true; + } else if (!round2DoneRef.current) { // Roots' children have arrived — preload one level further for them too, // exactly like appendItems does for a manually expanded node, so their own // expand affordance is known without an extra click. Only advances once real @@ -127,15 +140,37 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { }); if (addedAny) { round2DoneRef.current = true; - datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + shouldRefilter = true; } } + // Children of an expanded node can arrive after the expansion — still in flight when + // appendItems ran, or added later on. Pre-load them here too, so every visible node + // knows whether it has children of its own. + datasource.items?.forEach(item => { + const itemId = getItemId(item); + + if (loadedParentsByIdRef.current.has(itemId) || loadedChildsByIdRef.current.has(itemId)) { + return; + } + + const parentId = getParentId(item, parentAssociation); + if (parentId && expandedIdsRef.current.has(parentId)) { + loadedChildsByIdRef.current.set(itemId, item); + shouldRefilter = true; + } + }); + + if (shouldRefilter) { + datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + } + return; } initializedRef.current = true; loadedParentsByIdRef.current.clear(); + expandedIdsRef.current.clear(); round1DoneRef.current = false; round2DoneRef.current = false; @@ -144,7 +179,7 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { if (!startExpanded) { datasource.setFilter(getDatasourceFilter([undefined])); } - }, [datasource, getDatasourceFilter, getExpandedFilterItems, startExpanded]); + }, [datasource, getDatasourceFilter, getExpandedFilterItems, parentAssociation, startExpanded]); return { items: datasource.items, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 416c12a9b9..2de096b8c1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1913,7 +1913,7 @@ importers: version: link:../../shared/eslint-config-web-widgets '@mendix/pluggable-widgets-tools': specifier: 11.13.0 - version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(picomatch@4.0.5)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) + version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) '@mendix/prettier-config-web-widgets': specifier: workspace:* version: link:../../shared/prettier-config-web-widgets @@ -2686,7 +2686,7 @@ importers: version: link:../../shared/eslint-config-web-widgets '@mendix/pluggable-widgets-tools': specifier: 11.13.0 - version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) + version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(picomatch@4.0.5)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) '@mendix/prettier-config-web-widgets': specifier: workspace:* version: link:../../shared/prettier-config-web-widgets @@ -2696,6 +2696,9 @@ importers: '@mendix/widget-plugin-platform': specifier: workspace:* version: link:../../shared/widget-plugin-platform + '@mendix/widget-plugin-test-utils': + specifier: workspace:* + version: link:../../shared/widget-plugin-test-utils packages/pluggableWidgets/video-player-web: dependencies: