Controlled mode is broken

[urrri-redis]

Describe the bug when I want to control selection, I add external state for selectedIds and pass them to the tree. this is working, when I apply ids for the first time, like in uncontrolled mode. When I'm trying to apply ids, based on onSelect event, then it behaves incorrect and weird

Code That Causes The Issue

function App(){    
  const [sel, setSel] = useState<NodeId[]>([]);
  return (
    <TreeView
          data={nodes} // any hierarchical nodes (at least 1 level with branches and leaves)
          multiSelect
          propagateSelect
          propagateSelectUpwards
          togglableSelect
          selectedIds={sel}
          onSelect={({ element, treeState, ...state }) => {
            // following only to show the problem
            if (!props.isBranch) setSel([element.id]);
          }}
   />
  )
}

To Reproduce Steps to reproduce the behavior:

1. Add the code to any tree example
2. Click on any leaf node
3. Then click on any other leaf node
4. ! See endless switching between that nodes
5. to make problem even more visual, wrap setSel call with setTimeout
6. select some leafs/branches one by one
7. ! see flickering among all the selections

Note: I also tried to use following setSel([...treeState.selectedIds]), but it also makes flickering, when I select last leaf in branch, when other leaves already selected

Expected behavior if selectedIds prop is not undefined (controlled mode), then component must show passed selection, instead of internal state, even if user doesn't changes selection. Only after change events (onSelect, onNodeSelect) are treated and selectedIds updated, tree should represent new state, based only on values in the prop

Screen capture https://github.com/user-attachments/assets/5f4d0651-7844-493a-bfa7-4933793a041e

Desktop (please complete the following information):

• OS: [Windows 10]
• Browser [chrome]
• Version [126]

[urrri-redis]

@dgreene1 onSelect is called on all 4 cases

1. user selected node
2. related nodes (parent/children) are selected as result of user action (# 1)
3. node is selected, because selectedIds prop is changed
4. related nodes (parent/children) are selected as result of selectedIds change (# 3)

only first 2 events should cause onSelect, otherwise you have endless loop. to be backward compatible, you can add some flag, that tells, if this event caused by user action (## 1,2) or controlled state update (## 3,4)

[urrri-redis]

same issue is for the expandedIds/onExpand

[yhy-1]

Hi, @urrri-redis is there a reason why your not just using the defaultSelectId with the uncontrolled version?

OnSelect and OnSelectNode are applied only after the selection. Would it make more sense to have an onToggle callback for what you're looking for? This would be called before the actual selection had happened. You will need to manage the selectId, included when user deselected.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue was closed automatically since it was marked as stale because it has not had recent activity. Thank you for your contributions.

[cmd-azalea]

Could this be reopened? It's pretty confusing that the controlled selectedIds can't be updated in the parent. Typically when I think of a controlled component, I think of the parent holding the single source of truth for the child. This doesn't seem to be possible with the selectedIds prop.

[danciudev]

Yes, it looks like a total desynchronization. I think a redesign of the API should be done.

In my case I am making a search engine that when searching filters the tree and expands all the elements, in the internal state it maintains a previous state and gives error because it does not find certain Ids of the elements of the tree have been filtered.

[dgreene1]

We took over this library since it was abandoned. We wanted to make sure that security patches got released. But we don’t have capacity for major rewrites. If you’d like to attempt a PR, we can review it @danciudev or @cmd-azalea.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cmd-azalea]

Bumping to stop this from going stale

[stale[bot]]

This issue was closed automatically since it was marked as stale because it has not had recent activity. Thank you for your contributions.