dgreene1 / react-accessible-treeview

A react component that implements the treeview pattern as described by the WAI-ARIA Authoring Practices.
https://dgreene1.github.io/react-accessible-treeview
MIT License
261 stars 37 forks source link

Tree crashes, when wrong Id is in the selectedIds or expandedIds #191

Open urrri-redis opened 1 month ago

urrri-redis commented 1 month ago

Describe the bug Tree is used in controlled mode for selectedIds. User selects some Ids. Then user searches for some text (filtering scenario) and some of selected items are disappeared from the list. that means some of selected Ids dont exist in the data. Tree crashes.

To Reproduce Steps to reproduce the behavior:

  1. Go to Checkbox with controlled selectedIds or to Basic with controlled expandable node
  2. Add non-existing id (e.g. 55) into the ids input and click on "Set" button
  3. See crash

Expected behavior Tree should ignore wrong Ids. In best case in dev mode it should log wrong ids (better in one line).

Screenshots image

Desktop (please complete the following information):

urrri-redis commented 1 month ago

I tried to filter ids according to existing ones in data. It works, when you changing only selectedIds, but if you change also data (e.g filter by search) and together filter selectionIds, then it crashes, if previous selectionIds contained id, that not exists in new data. Hell

hangbt commented 1 month ago

I am also facing a similar issue when passing the filtered expandedIds array that contains IDs that do not exist in the tree data. Is this the same cause as the issue that is being discussed here?

dgreene1 commented 1 month ago

We’re open to PRs but since our team hasn’t experienced this issue, it’s not at the top of our prioritized list.

I’ll add the help wanted label.

kavorka-cat commented 1 month ago

I am also facing the same issue quite often, particularly when new nodes are added or some nodes are removed. Although I clean all selected and expanded nodes, the issue persists

hangbt commented 1 month ago

I guess that this issue only occurs when using controlled selectedIds or expandedIds. When passing only data, it works properly

urrri-redis commented 1 month ago

Of course when controlled. How other way you pass "wrong" id

пт, 26 июл. 2024 г., 10:01 hangbt @.***>:

I guess that this issue only occurs when using controlled selectedIds or expandedIds. When passing only data, it works properly

— Reply to this email directly, view it on GitHub https://github.com/dgreene1/react-accessible-treeview/issues/191#issuecomment-2252102857, or unsubscribe https://github.com/notifications/unsubscribe-auth/BFBHKIM2U3FKVHTNF2RN4O3ZOHX2PAVCNFSM6AAAAABLFWQM76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJSGEYDEOBVG4 . You are receiving this because you authored the thread.Message ID: @.***>

Disclaimer

The information contained in this communication from the sender is confidential. It is intended solely for use by the recipient and others authorized to receive it. If you are not the recipient, you are hereby notified that any disclosure, copying, distribution or taking action in relation of the contents of this information is strictly prohibited and may be unlawful.

This email has been scanned for viruses and malware, and may have been automatically archived by Mimecast Ltd, an innovator in Software as a Service (SaaS) for business. Providing a safer and more useful place for your human generated data. Specializing in; Security, archiving and compliance. To find out more visit the Mimecast website.

yhy-1 commented 1 month ago

I tried to filter ids according to existing ones in data. It works, when you changing only selectedIds, but if you change also data (e.g filter by search) and together filter selectionIds, then it crashes, if previous selectionIds contained id, that not exists in new data. Hell

Hello @urrri-redis , will you be able to provide a stripped down example of how you are using it. As Dan stated we are not having any issue. Are you managing both the data and selectedIds when it been removed/filtered?

ckpearson commented 1 week ago

We're also running into this for the following scenario:

  1. Construct node structure using props
  2. Flatten using flattenTree
  3. Derive ids for expansion from flattened.Map((node) => node.id)

Provide both data and expandedIds to the component, set up a prop that results in nodes being filtered out dynamically. User toggles, data alone works just fine, expandedIds causes trouble if nodes that previously existed no longer do.

My best guess is somewhere in the internals the component is holding onto the previously passed expanded ids.

ckpearson commented 1 week ago

Just a note for others running into this, one way we've found to semi-overcome this is to:

  1. Compute our data and expanded IDs together (only gets you so far as the tree view does hold onto previous values)
  2. Derive a random key using crypto.randomUUID() and memoize it based on our combined computed data changing
  3. Pass the key to the tree view

This does admittedly mean triggering a re-render of the tree view, so if you're controlling the expanded IDs, you're going to want to listen to the onExpand changes and make your local state updates or you'll lose the in-component expanded states on the next render.