[WC-3163] Treenode v2

[gjulivan]

Pull request type

---

Description

re-review

[iobuhov]

Approve, but not all changes requested have been implemented.

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
src/TreeNode.tsx	New v1/v2 dispatch based on parentAssociation
src/TreeNode.xml	Added parentAssociation property
typings/TreeNodeProps.d.ts	Added parentAssociation: ListReferenceValue
src/TreeNode.editorConfig.ts	Updated property visibility and preview for parentAssociation
src/TreeNode.editorPreview.tsx	Updated import path to v1/TreeNode
src/components/common/HeaderIcon.tsx	Moved and refactored icon component
src/components/common/TreeNodeState.ts	Extracted shared enum
src/components/v1/Root.tsx	Extracted v1 container logic from TreeNode.tsx
src/components/v1/TreeNode.tsx	Renamed, updated imports
src/components/v1/TreeNodeBranch.tsx	Renamed
src/components/v2/TreeNode.tsx	New v2 widget component
src/components/v2/hooks/useIncrementalTreeData.ts	New incremental tree state hook
src/components/v2/hooks/useInfiniteTreeNode.ts	New lazy-loading datasource hook
src/components/v2/hooks/helpers.ts	Shared helpers for v2
src/components/v2/ui/TreeNodeV2.scss	New v2 CSS-collapse styles
e2e/TreeNode.spec.js	New E2E tests for v2
package.json	Pinned MENDIX_VERSION=11.9.1, test branch updated
CHANGELOG.md	Checked for entry

Skipped (out of scope): dist/, pnpm-lock.yaml, renamed-only files with no content changes

---

Findings

🔶 Medium — Missing CHANGELOG entry for new feature

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new parentAssociation XML property, a new V2 rendering path, and new XML schema changes are behaviour/API changes that require a changelog entry under [Unreleased]. The [Unreleased] section is currently empty.
Fix: Run pnpm -w changelog from repo root, or manually add to CHANGELOG.md:

## [Unreleased]

### Added

- We added a `Parent association` property that enables infinite-depth lazy-loading tree hierarchies (Tree Node V2).

---

🔶 Medium — Direct mutation of TreeNodeV2DataItem.treeNodeState outside React state

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState is mutated directly on the object that lives inside treeData (owned by useIncrementalTreeData). The forceRender call masks this, but React's concurrent mode and StrictMode can replay effects, making stale-closure reads observe inconsistent state. Mutation on shared ref-backed objects is also invisible to useMemo/useCallback consumers and breaks diffing assumptions.
Fix: Expose a setter from useIncrementalTreeData (or useInfiniteTreeNodes) that updates the node state through setTreeData, ensuring React sees a new reference:

// in useIncrementalTreeData, expose:
const setNodeState = useCallback((nodeId: string, state: TreeNodeState) => {
    const node = nodesByIdRef.current.get(nodeId);
    if (node) {
        node.treeNodeState = state;
        setTreeData([...rootsRef.current]);
    }
}, []);

Then remove the forceRender pattern from TreeNodeV2.

---

🔶 Medium — No unit tests for any v2 code

File: src/components/v2/ (no __tests__ directory)
Problem: Three new non-trivial hooks (useIncrementalTreeData, useInfiniteTreeNode) and a new component (TreeNodeV2) have no unit tests. The incremental tree data logic (parent re-placement, config reset, removed-items detection) is complex enough that regressions will be hard to catch without isolated unit tests.
Fix: Add src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts at minimum. Key cases to cover:

• Root nodes appear when parentId is undefined
• Child nodes attach to correct parent after incremental load
• Config change resets the tree
• Self-referencing node (parentId === id) is treated as root

---

🔶 Medium — aria-expanded always set on leaf nodes in v2

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded is unconditionally rendered on every <li role="treeitem">, including leaf nodes (hasChildren === false). Per WAI-ARIA 1.2, aria-expanded must not be present on leaf tree items — screen readers will incorrectly announce them as collapsible.
Fix:

<li
    key={node.id}
    className="widget-tree-node-branch"
    role="treeitem"
    tabIndex={0}
    {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

---

⚠️ Low — Non-interactive <span> used as click target for tree header

File: src/components/v2/TreeNode.tsx line 23
Problem: The header is a <span onClick={...}>. Only nodes with children are visually "clickable" (via the widget-tree-node-branch-header-clickable class), but the onClick fires on all nodes. Keyboard users can focus the <li> but cannot activate the toggle via Enter/Space on the span. The v1 implementation already has a dedicated keyboard handler — v2 is missing this.
Note: This is consistent with v1's span-based header, but v2 should add keyboard handling before shipping.

---

⚠️ Low — getExpandedFilterItems dependency array is empty but reads mutable refs

File: src/components/v2/hooks/useInfiniteTreeNode.ts line 32–34
Problem: getExpandedFilterItems is memoised with [] deps but reads loadedParentsByIdRef.current and loadedChildsByIdRef.current inside. Since these are refs this is technically safe (ref.current is not a dep), but the empty array is misleading — linters may flag it in future ESLint configs. A comment clarifying the intent would help.

---

⚠️ Low — helpers.ts uses non-relative import path

File: src/components/v2/hooks/helpers.ts line 3
Problem: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a bare non-relative specifier. The other v2 files use "../../../../typings/TreeNodeProps". This may work because of a paths alias in tsconfig, but it's inconsistent and could break if the alias is removed.
Fix: Use the relative path: import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps";

---

Positives

• Excellent structural separation: existing v1 code was reorganised into components/v1/ without any behaviour change, making the diff easy to audit.
• The incremental tree data hook handles out-of-order delivery correctly (children arriving before parents are temporarily placed as roots and re-parented when the parent arrives).
• Self-referencing cycle guard (node.parentId !== node.id) prevents infinite loops on malformed data.
• CSS-only collapse via grid-template-rows: 0fr is a clean, animation-friendly approach that avoids DOM removal and preserves scroll position.
• useIncrementalTreeData correctly detects removed items and resets tree state, preventing stale nodes after datasource refreshes.
• E2E tests use aria roles and .mx-name-* selectors (preferred order), include proper afterEach session logout, and test the full lazy-load flow including grandchildren.

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
src/TreeNode.tsx	Entry-point split into v1/v2 dispatch
src/TreeNode.xml	New parentAssociation property added
typings/TreeNodeProps.d.ts	ListReferenceValue + parentAssociation added
src/TreeNode.editorConfig.ts	Updated getProperties and getPreview for v2
src/TreeNode.editorPreview.tsx	Import path update
src/components/common/HeaderIcon.tsx	Moved + refactored to shared
src/components/common/TreeNodeState.ts	Extracted TreeNodeState enum
src/components/v1/Root.tsx	Extracted v1 root container
src/components/v1/TreeNode.tsx	Renamed (path change only + re-export)
src/components/v1/TreeNodeBranch.tsx	Renamed + minor import updates
src/components/v2/TreeNode.tsx	New v2 recursive renderer
src/components/v2/hooks/helpers.ts	Tree data utilities
src/components/v2/hooks/useIncrementalTreeData.ts	Incremental tree state hook
src/components/v2/hooks/useInfiniteTreeNode.ts	Datasource filter / lazy-load hook
src/components/v2/ui/TreeNodeV2.scss	CSS grid collapse animation
e2e/TreeNode.spec.js	New v2 E2E tests
package.json	Test-project branch + Mendix version pinned

Skipped (out of scope): dist/, pnpm-lock.yaml, pure rename/no-change files

---

Findings

🔶 Medium — CHANGELOG entry missing for new feature

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: The [Unreleased] section is empty. This PR introduces a new XML property (parentAssociation), a new v2 rendering path, and lazy-loading behavior — all of which are user-visible runtime changes that require a changelog entry.
Fix: Run pnpm -w changelog from the repo root, or add manually under [Unreleased]:

### Added
- We added a `parentAssociation` property that enables a v2 lazy-loading mode for infinite-depth hierarchies using a self-referencing association.

---

🔶 Medium — aria-expanded applied to leaf nodes; clicking a leaf sets it to true

File: src/components/v2/TreeNode.tsx lines 22, 91-103
Problem: aria-expanded={isExpanded} is unconditionally set on every <li role="treeitem">, including leaf nodes (hasChildren === false). The ARIA spec says aria-expanded must only appear on items that can be expanded — its presence on a leaf implies falsely that the item is expandable. Worse, because onClick is also attached unconditionally (line 29), clicking a leaf node mutates node.treeNodeState to EXPANDED and triggers forceRender, causing aria-expanded to flip to "true" on a non-expandable item.
Fix:

// In renderRecursiveNode — only set aria-expanded when the node has children
<li
  key={node.id}
  className="widget-tree-node-branch"
  role="treeitem"
  tabIndex={0}
  {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

And guard onNodeClick to no-op on leaf nodes:

const onNodeClick = useCallback((node: TreeNodeV2DataItem) => {
    if (node.children.length === 0) return;
    // ... rest of handler
}, [appendItems]);

---

🔶 Medium — Interactive header uses <span onClick> instead of <button>

File: src/components/v2/TreeNode.tsx lines 23-30
Problem: The expand/collapse trigger is a <span> with onClick. Keyboard users (Enter/Space) cannot activate it, and screen readers won't announce it as interactive. The frontend guidelines require replacing <div>/<span> click handlers with semantic elements.
Fix: Replace the header <span> with a <button>:

<button
  type="button"
  className={classNames("widget-tree-node-branch-header", {
      "widget-tree-node-branch-header-reversed": iconPlacement === "left",
      "widget-tree-node-branch-header-clickable": hasChildren
  })}
  id={`${node.id}TreeNodeBranchHeader`}
  onClick={() => onNodeClick(node)}
  aria-label={typeof node.title === "string" ? node.title : undefined}
>

If the existing v1 behaviour for non-expandable nodes must be preserved (no cursor pointer), apply the hasChildren check to conditionally render a <button> vs a presentational <span>.

---

🔶 Medium — No unit tests for v2 hooks

File: src/components/v2/hooks/ (no __tests__ directory)
Problem: useIncrementalTreeData contains non-trivial tree-building logic (parent re-placement, config-change reset, removed-ID detection, orphan adoption). useInfiniteTreeNode manages Mendix datasource filter composition. Neither has unit tests. The E2E tests verify the golden path but won't catch edge cases like circular self-references, nodes arriving out of order, or config resets.
Fix: Add src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts covering at minimum:

• Root-only render on first pass
• Child adopted when parent arrives later
• Tree reset when configChanged
• Tree reset when removedIdsDetected

---

⚠️ Low — Non-relative import path in helpers.ts

File: src/components/v2/hooks/helpers.ts line 3
Problem: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a bare (root-alias) path. Every other file in v2 uses a relative path ("../../../../typings/TreeNodeProps"). This will likely resolve correctly via the TypeScript paths config but fails if the file is compiled by a tool that does not load tsconfig.json path aliases (e.g. Jest without moduleNameMapper).
Fix:

import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps";

---

⚠️ Low — Direct mutation of node.treeNodeState with forceRender pattern

File: src/components/v2/TreeNode.tsx lines 92-103
Problem: node.treeNodeState is mutated directly on the shared TreeNodeV2DataItem object (which lives inside useIncrementalTreeData's refs) and forceRender is called to trigger re-render. This works today but breaks React Strict Mode double-invocation guarantees, makes state changes invisible to any future MobX/context consumer, and creates a hidden coupling between the render component and the data hook's mutable internals.
Fix: Consider exposing a setNodeState(id, state) updater from useIncrementalTreeData, or at minimum document the mutation contract with a comment.

---

Positives

• The v1/v2 dispatch in the entry-point (if (props.parentAssociation)) is clean and keeps backward compatibility without touching any existing v1 code paths.
• Extracting TreeNodeState to common/TreeNodeState.ts and re-exporting from v1/TreeNode.tsx is a clean way to share the enum without breaking existing imports.
• The CSS grid 0fr/1fr collapse animation with will-change and min-height: 0 on grid children is a well-known, performant technique — no layout thrashing.
• E2E tests cover lazy loading, multi-root independent expansion, startExpanded, and collapse — good breadth for the main user journeys.
• useIncrementalTreeData correctly handles out-of-order arrivals (children before parents) and re-places orphan nodes when their parent is later added.