Preserve imperative tree.update() props across node toggles

[TrevorBurnham]

Picks up #229 from @iamyunsin (which targeted the old packages/ path and went stale), ports the fix to the current modules/ layout, and credits the original author via Co-Authored-By.

The bug

Closes #228 (partially — see Notes).

TreeProvider had a single useMemo that re-ran on either treeProps changes or state.nodes.open changes, always calling api.update(treeProps). That meant any prop a consumer set imperatively via tree.update({ ... }) would be reverted to the parent's React props the next time a node was toggled.

The fix

Split the effect:

• One useMemo syncs from treeProps (same as before, minus the open-state dep).
• A second useMemo rebuilds visible nodes on state.nodes.open changes using api.props — preserving any imperative changes.

Backward compatible: when no one calls api.update() imperatively, api.props === treeProps, so behavior is unchanged.

Notes

• Partial fix for There is a bug in using the tree.update method to set height. #228. The reporter also noted that tree.update() ''does not take effect immediately.'' That's a separate issue — api.update() mutates this.props but doesn't dispatch a Redux action, so useSyncExternalStore doesn't notify. That requires a different fix and isn't in scope here.
• No regression test. The repo currently has only a node-environment Jest setup with one TreeApi unit test. Reproducing this bug requires React component rendering (jsdom + @testing-library/react), which is a larger infra change. Happy to do that in a follow-up if you'd like.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Ports and updates the fix for #228/#229 to the current modules/ layout by separating “sync TreeApi props from React props” from “rebuild visible nodes on open/close changes”, so imperative tree.update() calls don’t get overwritten when nodes are toggled.

Changes:

• Removed state.nodes.open from the “sync from treeProps” update dependencies.
• Added a second memo keyed on state.nodes.open that rebuilds visible nodes using api.props to preserve imperative updates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[TrevorBurnham]

Good catch — you nailed it. CI surfaced the same regression and the amended commit (33e6de2) now bumps updateCount.current in the open-state memo as you suggested. Verified locally with the gmail-spec e2e (12/12 passing).

[TrevorBurnham]

Force-pushed an amended commit that fixes a regression caught by CI.

What was wrong: the original split-useMemo fix dropped updateCount.current += 1 from the open-state path. DefaultContainer (and useDataUpdates consumers generally) only re-render when DataUpdatesContext changes — so without the bump, state.nodes.open changes no longer triggered a re-render of the FixedSizeList, and itemCount stayed stale. That broke the Gmail demo's collapse/expand behavior (6/12 e2e tests).

Fix: bump updateCount.current inside the second useMemo too.

Verified locally: yarn cypress run --spec cypress/e2e/gmail-spec.cy.js → 12/12 passing with the corrected commit, vs. 6/12 failing before.