Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensureVisible inside useEffect and "two children with the same key" #9

Open
fintara opened this issue Dec 17, 2020 · 9 comments
Open

Comments

@fintara
Copy link

fintara commented Dec 17, 2020

Reproduction: https://codesandbox.io/s/sweet-mestorf-jcqoo?file=/src/App.js

In short, I have to track the path of the file that is currently selected. It can be changed externally (i.e. not from the tree), as in the example via next/prev buttons. I pass this state to the tree so that the folders expand when necessary via ensureVisible.

If you click "NEXT" several times, React will complain that there are "two children with the same key", and indeed, you can see how the labels mix up.

Any ideas?

@zikaari
Copy link
Owner

zikaari commented Jan 12, 2021

Apologies for the late response, been caught with a whole bunch of stuff.

It seems that the error consistently happens with key 13. Before I investigate if there's anything wrong with the harness, are you experiencing any error like this in your production/real app too?

Btw I'm curious to know what are you using react-aspen for? I'd love to feature it on the readme.

@fintara
Copy link
Author

fintara commented Jan 12, 2021

No problem!

It seems to depend on the height of AspenFileTree, e.g. with 700 in the example above it errors at key 24.
And it does happen in my app consistently too (given the same data and height). However, it's not always "just below the visible part", check this: https://codesandbox.io/s/inspiring-payne-giqwu?file=/src/App.js - depending on eachLength (how many files are in each directory), you'll get the error at different places (smaller = sooner, larger = might not get it at all).

And another strange case (see https://codesandbox.io/s/thirsty-dust-qq0xd?file=/src/App.js):

  • there is a folder with 10 files
  • followed by a folder with 1 file
  • followed by many other entries

The error will happen when you reach the folder with 1 file, as the next entries' keys will mix up. Note: It will not happen if the whole tree is expanded beforehand (so perhaps ensureVisible will only do the scrolling). It will also not happen if after the 1-file folder there are less than 4 entries (perhaps this is connected with how many entries are pre-rendered?).


As for the use case, I have a flat list of paths (similar to the examples) out of which I build an fp-ts/Tree and then I use react-aspen to visualize it.

The Tree allows me to count how many files each directory has (up to the root, which will show the count of all files), and also track other metadata for the directory (which depends on its files' content). I also use it in createHost as a kind of a file system (given the tree and the path, I "cd" into it and return the contents).

In each renderRow, if it's a file I get the object behind the path, and I put some information below the file name (like a preview). For directories I show how many files are inside.

So this family of libraries (aspen) is fantastic - not only is if fast (2000+ entries are not a problem - scrolling, expanding/collapsing), but it also gives enough flexibility.

@graup
Copy link

graup commented Jan 19, 2021

We also ran into a similar issue when trying to 'expand all' directories on initial load. Sometimes we get overlapping keys. I think this might be a timing issue somewhere? It seems like the same row is rendered once collapsed and once expanded.

@zikaari
Copy link
Owner

zikaari commented Jan 19, 2021

Will allocate some time this week to put some breakpoints and see what's going on.

@fintara
Copy link
Author

fintara commented Oct 2, 2021

After some experiments I have pinpointed it to FileTree.tsx - getItemAtIndex.

If I bypass the idxTorendererPropsCache map, I see that at some point different items are returned for the same index (I've used my first example). Therefore when root.getFileEntryAtIndex(index) is always called, the problem with the same key disappears.

Turning the cache back on, when I add this.idxTorendererPropsCache.clear() before this line https://github.com/NeekSandhu/react-aspen/blob/630f7c0f0ae3923c27c0af02f589a841678e5c00/src/components/FileTree.tsx#L236 the issue disappears as well. It could be better to put it before if (this.scrollIntoView(fileEntry, align)), but I don't fully understand the semantics. :)

@zikaari
Copy link
Owner

zikaari commented Oct 4, 2021

Am I understanding correctly that if caching mechanism is active, then different indexes can sometimes return same prop data?

@fintara
Copy link
Author

fintara commented Oct 4, 2021

I think so. My interpretation is that when a directory is unfolded (due to ensureVisible), the elements in the list change their index.

@nikhil-mohite
Copy link
Contributor

Hi @zikaari,
We are also facing the same issue of overlapping keys while loading larger data. Is there any way we can disable the cache?
(If I follow the fix provided by @fintara issue is getting resolved. )

@zikaari
Copy link
Owner

zikaari commented Dec 3, 2021

@nikhil-mohite can you please create a very minimal codesandbox, just bare enough to reproduce your specific same key issue? This is to ensure we have knowledge of all the ways this issue can be triggered.

Also, note that there will be no further development towards the current version of Aspen, all the resources are now dedicated towards Aspen 2. You can follow the progress here. Click Subscribe to be notified of the beta and final release. I'm asking for repro codesandbox just so I can make sure this issue does not also show up in Aspen 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants