[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.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
src/TreeNode.tsx	Root component — routes to v1 or v2 based on parentAssociation
src/TreeNode.xml	Adds parentAssociation association property
typings/TreeNodeProps.d.ts	Adds parentAssociation?: ListReferenceValue + preview type
src/TreeNode.editorConfig.ts	Hides/shows properties based on new property; preview updated
src/TreeNode.editorPreview.tsx	Import path update
src/components/common/HeaderIcon.tsx	Renamed + decoupled IconOptions from v1 TreeNodeProps
src/components/common/TreeNodeState.ts	Extracted shared TreeNodeState enum
src/components/v1/Root.tsx	Extracted v1 container logic (was in TreeNode.tsx)
src/components/v1/TreeNode.tsx	Renamed; re-exports TreeNodeState
src/components/v1/TreeNodeBranch.tsx	Renamed; import paths updated
src/components/v1/HeaderIcon.tsx	Thin re-export shim
src/components/v2/TreeNode.tsx	New v2 component — renders recursive tree using lazy data
src/components/v2/hooks/useIncrementalTreeData.ts	Incremental tree build from flat ObjectItem[]
src/components/v2/hooks/useInfiniteTreeNode.ts	Drives datasource filter for lazy loading
src/components/v2/hooks/helpers.ts	Utility functions for id/parent/title extraction
src/components/v2/ui/TreeNodeV2.scss	CSS-grid collapse animation for v2
e2e/TreeNode.spec.js	New E2E tests for v2 lazy-loading and startExpanded
package.json	Test branch + Mendix version bump for E2E

Skipped (out of scope): dist/, pnpm-lock.yaml, renamed files with zero diff

---

Findings

🔶 Medium — No CHANGELOG.md entry for runtime behavior change

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new XML property (parentAssociation) and an entirely new v2 rendering path are added. This is a visible behavior and API change — a CHANGELOG entry is required per repo conventions.
Fix: Run pnpm -w changelog inside packages/pluggableWidgets/tree-node-web and add an entry under [Unreleased] describing the new parentAssociation lazy-loading mode.

---

🔶 Medium — Direct state mutation on TreeNodeV2DataItem triggers a forceRender hack

File: src/components/v2/TreeNode.tsx lines 659–666
Problem: node.treeNodeState is mutated directly on the data item (a plain object from the ref-backed tree), then forceRender(v => v + 1) forces a re-render. This bypasses React's state model — the mutable tree lives outside React state, so the [, forceRender] sentinel is the only thing signalling React that anything changed. If useIncrementalTreeData ever produces a new array reference (e.g. after a config change) while a node is expanded, the mutation is lost because treeData is re-derived from setTreeData([...rootsRef.current]). The expanded/collapsed state should either be tracked in a Map held in a separate useState/useReducer, or the data items should be treated as immutable and replaced on each toggle.
Fix:

// Option: maintain a separate expanded-state map in useState
const [expandedIds, setExpandedIds] = useState<Set<string>>(new Set());

const onNodeClick = useCallback((node: TreeNodeV2DataItem) => {
    setExpandedIds(prev => {
        const next = new Set(prev);
        if (next.has(node.id)) {
            next.delete(node.id);
        } else {
            next.add(node.id);
            appendItems(node.item, node.children.map(c => c.item));
        }
        return next;
    });
}, [appendItems]);

Then pass expandedIds into renderRecursiveNode and derive isExpanded from the set rather than from node.treeNodeState.

---

🔶 Medium — useInfiniteTreeNode useEffect missing dependency: startExpanded

File: src/components/v2/hooks/useInfiniteTreeNode.ts line 975
Problem: startExpanded is read in the useEffect body but is listed as a dependency — that part is fine. However initializedRef.current is used as a one-shot guard that never resets when startExpanded changes after mount. If the host page changes startExpanded dynamically, the guard prevents the filter from being re-applied. The startExpanded value used in the effect is the value captured when initializedRef was first set; changes are silently ignored.
Fix: Reset initializedRef.current when startExpanded changes, or move the initialization logic into a separate useMemo/effect keyed on startExpanded so toggling startExpanded re-runs the root filter.

---

🔶 Medium — Header span uses onClick without keyboard support

File: src/components/v2/TreeNode.tsx lines 591–597
Problem: The <span className="widget-tree-node-branch-header" … onClick={…}> is the interactive toggle for expand/collapse. A <span> does not fire click events on Enter/Space for keyboard users, and the tabIndex={0} is set on the <li role="treeitem"> rather than on the clickable element. Per the guidelines and WCAG 2.1, interactive controls should be <button> elements (or have role="button" + onKeyDown).
Fix:

<button
    type="button"
    className={classNames("widget-tree-node-branch-header", { ... })}
    onClick={() => onNodeClick(node)}
    aria-expanded={isExpanded}
>

Move aria-expanded from the <li> to the button (ARIA authoring practice for treegrid/tree recommends the expanding control carry aria-expanded).

---

⚠️ Low — appendItems early-return when parent already loaded skips pre-loading children

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 922–947
Problem: When loadedParentsByIdRef.current.has(parentId) is already true but children is empty or undefined, the function returns without calling datasource.setFilter. If a previously-expanded node is re-clicked and the component re-calls appendItems (e.g. after a datasource reload), the filter is never refreshed and children may disappear. This is a subtle edge case but could surface in startExpanded scenarios where all nodes are pre-loaded.

---

⚠️ Low — renderRecursiveNode is a plain function inside the module, not a component

File: src/components/v2/TreeNode.tsx lines 578–622
Problem: renderRecursiveNode is called as a function (renderRecursiveNode(node, …)) rather than rendered as a JSX component (<RecursiveNode node={node} … />). This means React cannot reconcile individual nodes — every call to forceRender unmounts and re-mounts the entire subtree. Define it as a proper component (function TreeNodeItem(…)) and render it with JSX so React can diff individual nodes.

---

⚠️ Low — No unit tests for v2 components

File: src/components/v2/ (no __tests__/ directory)
Problem: useIncrementalTreeData, useInfiniteTreeNode, and the TreeNodeV2 component have no unit tests. The incremental placement logic in useIncrementalTreeData (config-change reset, parent-not-yet-loaded case, cycle guard candidate.id !== nodeId) has enough branching to benefit from targeted unit tests with EditableValueBuilder/ListValueBuilder.

---

⚠️ Low — TreeNodePreviewProps.parentAssociation typed as string (should be string | null)

File: typings/TreeNodeProps.d.ts line 1037
Problem: The preview type for parentAssociation is string while the container type is ListReferenceValue | undefined. The editor config uses values.parentAssociation !== null as the guard, but string can never be null in TypeScript — the guard is always truthy if the field is "" (empty string). Check how Mendix Studio Pro supplies unset association previews (usually null) and align the type and guard accordingly.
Fix: Change to parentAssociation: string | null in TreeNodePreviewProps, or follow what other association preview props use in this codebase.

---

Positives

• Clean v1/v2 separation — all existing v1 files moved intact to components/v1/ with only import-path corrections; no v1 behaviour changed.
• TreeNodeState correctly extracted to components/common/ so both v1 and v2 share the same enum without circular imports.
• CSS-grid 0fr / 1fr collapse animation is a clean, accessible technique that avoids display: none (content stays in the DOM for screen readers).
• E2E tests use .mx-name-* selectors and ARIA roles consistently; afterEach session logout is present; no hardcoded waitForTimeout calls; toBeAttached used instead of toBeVisible for lazy-loaded content.
• useIncrementalTreeData correctly guards against cycles (candidate.id !== nodeId) and handles the "child arrives before parent" case by re-placing orphaned roots.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
src/TreeNode.tsx	V1/V2 routing based on parentAssociation
src/TreeNode.xml	New parentAssociation XML property
src/TreeNode.editorConfig.ts	Studio Pro config updated for new property
src/TreeNode.editorPreview.tsx	Import updated to v1 path
src/components/common/HeaderIcon.tsx	Moved to shared, IconOptions interface extracted
src/components/common/TreeNodeState.ts	New shared TreeNodeState enum
src/components/v1/Root.tsx	Logic extracted from old TreeNode.tsx
src/components/v1/TreeNode.tsx	Renamed (path only)
src/components/v2/TreeNode.tsx	New V2 render component
src/components/v2/hooks/useIncrementalTreeData.ts	New incremental tree-state hook
src/components/v2/hooks/useInfiniteTreeNode.ts	New lazy-loading/filter hook
src/components/v2/hooks/helpers.ts	Utility functions for item ID, title, parent
src/components/v2/ui/TreeNodeV2.scss	New CSS-grid collapse animation
typings/TreeNodeProps.d.ts	parentAssociation?: ListReferenceValue added
e2e/TreeNode.spec.js	New V2 E2E test suite
package.json	Test branch + Mendix version pin

Skipped (out of scope): dist/, pnpm-lock.yaml, snapshot PNGs

---

Findings

🔶 Medium — Missing CHANGELOG entry for new feature

File: CHANGELOG.md
Problem: [Unreleased] is empty. Adding parentAssociation is a new public XML property and behaviour — it requires a changelog entry per repo conventions.
Fix: Run pnpm -w changelog or manually add under [Unreleased]:

### Added
- We added the `parentAssociation` property to enable infinite-depth lazy-loaded tree hierarchies (V2 mode).

---

🔶 Medium — <span> used as interactive element — keyboard inaccessible

File: src/components/v2/TreeNode.tsx line 23
Problem: The node header is a <span onClick>. Keyboard users cannot activate it with Enter or Space. Screen readers also won't announce it as interactive. V1 uses a dedicated button/header element with keyboard handlers.
Fix: Replace with <button> (or keep <span> but add role="button", tabIndex={0}, and onKeyDown). The cleaner fix is 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)}
>

---

🔶 Medium — aria-expanded set on leaf nodes

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded={isExpanded} is placed on every <li role="treeitem">, including leaf nodes (hasChildren === false). Per WAI-ARIA, aria-expanded must only be present on nodes that can be expanded; setting it on leaves is a spec violation and confuses screen readers.
Fix: Only render the attribute when the node has children:

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

---

🔶 Medium — V2 missing all keyboard navigation

File: src/components/v2/TreeNode.tsx
Problem: V1 has useTreeNodeBranchKeyboardHandler and useTreeNodeFocusChangeHandler for full arrow-key / Enter / Escape navigation, and uses a roving tabIndex pattern (active node: 0, others: -1). V2 sets tabIndex={0} on every node and has no key handlers at all. This is a complete accessibility regression versus V1.
Fix: Port the accessibility hooks from v1/hooks/ (or move them to common/) and wire them into the V2 tree. At minimum, add onKeyDown to the header that maps Enter/Space to onNodeClick.

---

🔶 Medium — No unit tests for V2 code

File: src/components/v2/
Problem: Three substantial new modules (TreeNode.tsx, useIncrementalTreeData.ts, useInfiniteTreeNode.ts) have no unit tests. useIncrementalTreeData especially — which handles incremental insertion, re-parenting, config-change resets, and cycle guards — has complex logic that is hard to verify through E2E alone.
Fix: Add src/components/v2/__tests__/useIncrementalTreeData.spec.ts and TreeNode.spec.tsx covering at minimum: root-only initial render, child insertion, parent change, startExpanded path, and config-change reset.

---

🔶 Medium — Mendix association value read without status check

File: src/components/v2/hooks/helpers.ts lines 14, 20
Problem: parentAssociation?.get(item).value and config.headerCaption?.get(item).value are accessed directly without checking .status first. When the value is loading, .value is undefined, which is silently swallowed here — but it means nodes render with a fallback ID instead of their real title while loading, and parent associations are resolved as undefined (treated as roots) until the data arrives.
Fix:

export function getParentId(item: ObjectItem, parentAssociation: TreeNodeContainerProps["parentAssociation"]): string | undefined {
    const ref = parentAssociation?.get(item);
    if (ref?.status !== ValueStatus.Available) return undefined;
    return ref.value?.id ? getItemId(ref.value) : undefined;
}

export function getItemTitle(item: ObjectItem, config: TreeConfigRef): ReactNode {
    if (config.headerType === "text") {
        const caption = config.headerCaption?.get(item);
        return caption?.status === ValueStatus.Available ? caption.value : getItemId(item);
    }
    const content = config.headerContent?.get(item);
    return content ?? getItemId(item);
}

---

🔶 Medium — Direct mutation of shared state object

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState = TreeNodeState.COLLAPSED_WITH_CSS mutates an object that is part of the ref-backed tree structure shared across renders. This works today because of the forceRender call, but it bypasses React's state model — devtools won't show state changes, and this pattern will break under React Strict Mode or concurrent features.
Fix: Store treeNodeState separately (e.g. a Map<string, TreeNodeState> in a useRef updated via setTreeData) or call setTreeData(prev => [...prev]) after mutating — at minimum, don't rely on mutation + imperative re-render for correctness.

---

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

File: src/components/v2/hooks/helpers.ts line 3
Note: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a non-relative path. All other files in the package use explicit relative paths (../../../../typings/TreeNodeProps). This only works if a paths alias is configured in tsconfig.json, which is uncommon in this repo. Verify it resolves in CI builds and consider aligning with the relative-path convention.

---

⚠️ Low — will-change in static CSS rule

File: src/components/v2/ui/TreeNodeV2.scss lines 6–8
Note: will-change: grid-template-rows, opacity is declared statically on .widget-tree-node-v2-body. This permanently promotes all tree-node body elements to compositor layers. For trees with many nodes, this increases GPU memory usage even when no animation is running. Consider applying will-change only during animation via a transitioning class, or removing it entirely (the transition still works without it).

---

⚠️ Low — E2E: toBeAttached used where toBeVisible is appropriate

File: e2e/TreeNode.spec.js lines 76–77
Note: toBeAttached only verifies the element is in the DOM, not that it is visible to the user. After lazy-loading grandchildren, the intent is to confirm the user can see them. Use toBeVisible() instead.

---

Positives

• Clean V1/V2 split — routing at the widget entry point with a simple props.parentAssociation guard is easy to follow and preserves backward compatibility entirely.
• useIncrementalTreeData correctly handles out-of-order arrival (children before parents are temporarily placed as roots and re-parented when the parent arrives) and self-reference cycles (node.parentId !== node.id guard).
• isConfigChanged uses reference equality on Mendix props, which is correct — Mendix gives the same reference when the config hasn't changed, so this avoids spurious full-tree resets.
• Moving TreeNodeState to components/common/ and re-exporting from v1/TreeNode is a clean refactor that makes it available to V2 without a circular dependency.
• E2E tests use aria-expanded assertions for collapse state rather than visibility — appropriate since V2 uses CSS-grid for animation (children remain in the DOM).
• Snapshot baselines are committed alongside the new tests.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
src/TreeNode.tsx	Route to v1/v2 based on parentAssociation
src/TreeNode.xml	New parentAssociation association property
typings/TreeNodeProps.d.ts	Added ListReferenceValue parentAssociation
src/TreeNode.editorConfig.ts	Updated getProperties/getPreview for v2
src/TreeNode.editorPreview.tsx	Import updated to v1/TreeNode
src/components/common/HeaderIcon.tsx	Moved + refactored IconOptions
src/components/common/TreeNodeState.ts	Extracted shared enum
src/components/v1/Root.tsx	Extracted root adapter (was TreeNode.tsx)
src/components/v1/TreeNode.tsx	Renamed
src/components/v1/HeaderIcon.tsx	Re-export shim
src/components/v2/TreeNode.tsx	New v2 component
src/components/v2/hooks/helpers.ts	Tree data helpers
src/components/v2/hooks/useIncrementalTreeData.ts	Incremental tree state hook
src/components/v2/hooks/useInfiniteTreeNode.ts	Filter/lazy-load hook
src/components/v2/ui/TreeNodeV2.scss	CSS-only collapse animation
e2e/TreeNode.spec.js	New v2 E2E test scenarios
package.json	Pinned MENDIX_VERSION=11.9.1, updated test branch

Skipped (out of scope): dist/, pnpm-lock.yaml, pure renames with no content change

---

Findings

🔶 Medium — CHANGELOG entry missing for new XML property and v2 behavior

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new parentAssociation XML property was added and an entirely new rendering path (v2) introduced. These are runtime/XML/behavior changes and require a CHANGELOG entry under [Unreleased].
Fix: Run pnpm -w changelog or manually add to CHANGELOG.md:

## [Unreleased]

### Added

- We added a new `parentAssociation` property that enables infinite-depth hierarchies using a self-referencing association, powered by a new lazy-loading tree renderer (v2).

---

🔶 Medium — Direct mutation of node.treeNodeState then forceRender — MobX-style state mutation anti-pattern in plain React

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState is a property of a plain object held inside React state (treeData). Mutating it directly and then calling forceRender bypasses React's immutable state model. React's reconciler compares object references — the same reference is mutated, so React.memo or any memoization downstream cannot detect the change. This will also silently break if useIncrementalTreeData ever uses Object.is comparison.
Fix: Either produce a new node object (immutable update) or call setTreeData in useIncrementalTreeData with a shallow copy. A minimal immutable approach:

const onNodeClick = useCallback(
    (node: TreeNodeV2DataItem) => {
        node.treeNodeState =
            node.treeNodeState === TreeNodeState.EXPANDED
                ? TreeNodeState.COLLAPSED_WITH_CSS
                : TreeNodeState.EXPANDED;

        if (node.treeNodeState === TreeNodeState.EXPANDED) {
            appendItems(node.item, node.children.map(child => child.item));
        }
        // trigger a new array reference so React sees the change
        setTreeData(prev => [...prev]);
    },
    [appendItems, setTreeData]
);

This requires setTreeData to be returned from useIncrementalTreeData.

---

🔶 Medium — aria-expanded rendered on all <li role="treeitem"> nodes, including leaf nodes

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded={isExpanded} is always set on every <li>. According to WAI-ARIA, aria-expanded must only be present on items that can be expanded (i.e., items with children). Leaf nodes must have no aria-expanded attribute. Setting it to false on a leaf node incorrectly signals to screen readers that the node is collapsed but could be expanded.
Fix:

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

---

🔶 Medium — useEffect in useInfiniteTreeNode has stale-closure risk on startExpanded

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 69–93
Problem: startExpanded is captured at initialization time via initializedRef. However, if the parent's startExpanded prop changes after the initial render (which Mendix props can), the effect will not re-run the initialization path because initializedRef.current is already true. The initializedRef guard bypasses the deps array intent.
Fix: Accept that startExpanded is an init-only value by capturing it in its own ref, making the intent explicit and preventing confusion:

const startExpandedRef = useRef(startExpanded); // snapshot on mount
// then use startExpandedRef.current inside the effect

Remove startExpanded from the deps array since the ref value doesn't change. This self-documents that it is intentionally read once.

---

⚠️ Low — <span onClick> on the header instead of <button>

File: src/components/v2/TreeNode.tsx lines 23–36
Problem: The node header uses a <span> with onClick. This is not keyboard-accessible out of the box — it will not fire on Enter/Space unless keyboard handlers are added, and it lacks implicit button semantics for screen readers. The v1 implementation uses proper button semantics via keyboard hooks.
Note: This is a medium concern from an accessibility standpoint but given the tree already uses tabIndex={0} on the <li>, it may partially work. Still, a <button> or at minimum role="button" with onKeyDown should be used for the header span to match v1 behaviour and WCAG 2.2 AA.

---

⚠️ Low — parentAssociation is string in TreeNodePreviewProps but nullable check uses !== null

File: src/TreeNode.editorConfig.ts line 66 / typings/TreeNodeProps.d.ts line 50
Problem: In TreeNodePreviewProps, parentAssociation is typed as string (not string | null). Studio Pro passes an empty string "" for unset association properties. The check values.parentAssociation !== null will always be true (an empty string is not null), meaning showChildren is always true when parentAssociation is used. The correct check is values.parentAssociation !== "" (or just truthy !!values.parentAssociation).
Fix:

const showChildren = values.hasChildren || !!values.parentAssociation;

---

⚠️ Low — No unit tests for new v2 components

File: src/components/v2/
Note: useIncrementalTreeData, useInfiniteTreeNode, and TreeNode.tsx (v2) are new, non-trivial logic files with no corresponding spec files. Consider adding at minimum unit tests for useIncrementalTreeData covering: initial render with root nodes, incremental load of children, config reset on change, and the re-parenting logic when parent arrives after child.

---

Positives

• Clean v1/v2 split with backward-compatible routing in TreeNode.tsx — existing users are unaffected.
• useIncrementalTreeData correctly handles the out-of-order arrival case (children arriving before parent) by re-placing nodes when the parent is later inserted.
• CSS-only collapse animation using grid-template-rows: 0fr/1fr avoids JS height measurement and is a solid pattern.
• E2E tests use getByRole("treeitem") and .mx-name-* selectors — follows the preferred selector hierarchy from the review guidelines.
• TreeNodeState enum correctly extracted to common/ so both v1 and v2 share the same constants without duplication.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
src/TreeNode.tsx	Top-level router: delegates to V1 or V2 based on parentAssociation
src/TreeNode.xml	Added parentAssociation XML property
typings/TreeNodeProps.d.ts	Added ListReferenceValue parentAssociation prop
src/TreeNode.editorConfig.ts	Property visibility / preview logic updated for v2
src/TreeNode.editorPreview.tsx	Import path updated for renamed v1 component
src/components/common/HeaderIcon.tsx	Shared header icon (renamed + refactored)
src/components/common/TreeNodeState.ts	New shared state enum
src/components/v1/Root.tsx	V1 adapter (extracted from old TreeNode.tsx)
src/components/v1/TreeNode.tsx	Renamed; re-exports TreeNodeState
src/components/v1/HeaderIcon.tsx	Re-export shim
src/components/v2/TreeNode.tsx	New infinite-depth tree renderer
src/components/v2/hooks/useInfiniteTreeNode.ts	Datasource filter management
src/components/v2/hooks/useIncrementalTreeData.ts	Incremental tree-node data builder
src/components/v2/hooks/helpers.ts	Pure helper utilities
src/components/v2/ui/TreeNodeV2.scss	CSS-only collapse animation
e2e/TreeNode.spec.js	New v2 E2E test scenarios
CHANGELOG.md	Added [Unreleased] ### Added entry
package.json	MENDIX_VERSION pinned; test-project branch switched to v2

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

---

Findings

🔶 Medium — Direct mutation of treeNodeState on ref-stored objects bypasses React's rendering model

File: src/components/v2/TreeNode.tsx lines 116–121
Problem: node.treeNodeState is mutated in place on objects that live inside refs managed by useIncrementalTreeData. React never sees a new object reference, so the tree is only re-rendered because of the manual forceRender counter hack. This pattern is fragile: concurrent rendering, StrictMode double-invocations, or any future refactor can silently break it. React state (or at minimum a MobX action) should own the expand/collapse state.
Fix: Move treeNodeState into useState / a Map<string, TreeNodeState> stored in React state, keyed by node ID, and derive isExpanded from that map during render. useIncrementalTreeData should deal only with structural tree data (parent/child relationships), not UI state.

---

🔶 Medium — useEffect in useInfiniteTreeNode.ts runs on every render when datasource reference changes

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 69–93
Problem: datasource is in the dependency array. Mendix ListValue objects are typically recreated on every parent render, so datasource is a new reference each render cycle. This means the effect runs repeatedly, clearing initializedRef on the first run and then immediately re-triggering the pre-load logic on subsequent runs — causing infinite filter updates.
Fix: Depend on datasource.status and datasource.items (stable primitives Mendix actually changes when data changes) rather than the whole datasource object, matching the existing V1 pattern at v1/Root.tsx:45.

---

🔶 Medium — No unit tests for the new v2 hooks and component

File: src/components/v2/
Problem: useIncrementalTreeData, useInfiniteTreeNode, and TreeNodeV2 have no corresponding __tests__/*.spec.tsx files. The incremental tree-building logic in useIncrementalTreeData has several tricky branches (re-parenting nodes, cycle prevention, partial-load ordering) that are exactly the kind of code that breaks silently and requires unit test coverage. E2E tests cover the golden path, but not edge cases.
Fix: Add src/components/v2/__tests__/useIncrementalTreeData.spec.ts covering at minimum: root-only load, child loaded before parent (out-of-order), parent change (re-parenting), and config change causing full reset.

---

⚠️ Low — key prop placed on <li> inside a plain function (not a component)

File: src/components/v2/TreeNode.tsx line 28
Note: renderRecursiveNode is a plain function, not a React component, so the key prop on the returned <li> has no effect — React uses key only on elements returned directly from a component's render or a .map() call at the call site. The caller at line 138–147 passes these elements directly into treeData.map(...), so React does see the key there. But if renderRecursiveNode is ever called outside of a .map(), or if a linter/reader tries to reason about it, the misplaced key is confusing. Move key to the .map() call sites, or convert to a proper React component.

---

⚠️ Low — Icon-only expand/collapse button uses <span onClick> instead of <button>

File: src/components/v2/TreeNode.tsx lines 44–52
Note: The icon container is a <span> with an onClick. For openNodeOn === "iconClick" this is the sole interactive target, making it keyboard-inaccessible (no focus, no Enter/Space handler). Use <button type="button"> with an aria-label (e.g. "Expand Electronics") for the icon-only click target. The same pattern exists in the parent header <span> at line 34 for headerClick mode.

---

⚠️ Low — getExpandedFilterItems useCallback has an empty dependency array despite reading mutable refs

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 32–34
Note: getExpandedFilterItems reads loadedParentsByIdRef.current and loadedChildsByIdRef.current. Refs are stable so the empty [] dep array is technically safe — but the lint rule react-hooks/exhaustive-deps will flag this. Consider inlining the function body into the callers or adding a // eslint-disable-next-line with a short explanation to prevent future confusion.

---

Positives

• Clean v1/v2 split: all existing V1 code was preserved exactly through renames, with zero behaviour changes, making the diff easy to audit.
• useIncrementalTreeData correctly guards against self-referencing cycles (node.parentId !== node.id) and handles out-of-order arrival (re-placing orphan nodes once their parent arrives).
• CSS-only collapse animation using grid-template-rows: 0fr is a modern, paint-efficient technique that avoids height: auto transition hacks.
• datasource.setFilter using Mendix filter builders (association, equals, literal, or) is the correct way to drive lazy loading — no direct DOM/fetch calls.
• E2E tests include afterEach session logout and waitForLoadState("networkidle") in beforeEach, matching the required structure exactly.
• CHANGELOG entry is present and follows Keep a Changelog format.