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

[urrri-redis]

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

Desktop (please complete the following information):

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

[urrri-redis]

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]

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]

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]

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]

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

[urrri-redis]

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]

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]

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]

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.

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

[jej2003]

We have this exact issue when modifying the data the tree displays and controlling the expanded nodes. If I should open a new issue please let me know, but for now I created a gist that demonstrates the issue based off of the filtering example. Can we reopen this? To reproduce the issue expand the "Fruits" and "Vegetables" nodes, then apply a filter for "fr". You will see a stack trace similar to

chunk-KDCVS43I.js?v=da12dc13:9176 Uncaught Error: Node with id=3 doesn't exist in the tree.
    at getTreeNode (utils.js:336:15)
    at isBranchNode (utils.js:41:18)
    at index.js:197:21

The attached patch to src/TreeView/index.tsx with extremely limited knowledge of the codebase and minimal testing has mitigated the issue I was seeing but again more testing is needed and any feedback would be great.

Thanks for the great component!

react-accessible-treeview with controlled expanded ids causing the tree to crash during data modifications

[dgreene1]

@jej2003, @ckpearson would you be willing to try submitting a fix PR?

Thank you for the reproduction gist (cc @yhy-1 )

I’ve reopen this issue. Help wanted.

[jej2003]

absolutely! Per your request https://github.com/dgreene1/react-accessible-treeview/pull/216

[jej2003]

@dgreene1, sorry to poke this again but was just looking for feedback on https://github.com/dgreene1/react-accessible-treeview/pull/217? We are using the code in this PR and addresses our use cases and is less impactful to the existing code base ~1/2 the changes as compared to the original PR (216).

[dgreene1]

@jej2003 our turnaround time is not very fast, so please be patient. I’ve asked a member of our time to review each of the PRs.

[jej2003]

No worries! My thought is that #216 is probably more than is required and #217 will be easier to get approved given the smaller impact to code base.

[alexander-ignatow]

@jej2003 it is just what I was looking for - thanks! I hope it will get to the codebase soon

[shaurya-sharma064]

Been waiting on this to get merged for a while now. Can someone please look into it? Will help me out a ton

[dgreene1]

@shaurya-sharma064 I’ve asked the team to look at it again. To repeat what I shared above, we’re not experiencing this bug ourselves so it’s lower priority, but we understand that you and others are.

@jej2003 our turnaround time is not very fast, so please be patient. I’ve asked a member of our time to review each of the PRs.

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