Skip to content

Conversation

snowystinger
Copy link
Member

@snowystinger snowystinger commented Aug 26, 2025

Closes #8725

Note, this does not address initial load/render of Tree, this is just for expanded keys

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

visitNode in Tree for the expand all action takes
Before: ~20772ms
After: ~190ms

🧢 Your Project:

@rspbot
Copy link

rspbot commented Aug 26, 2025

@snowystinger snowystinger changed the title fix: Speed up tree collection builder fix: Speed up tree collection builder flattening when expanding keys Aug 26, 2025
}

const treeData = generateTreeData();
console.log(`Total items: ${totalItems}`);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best way to test this is to the root level visitNode in timing, I didn't include it so that the tests would all pass and we'd get a build, the delay/difference should be noticeable in the build, but just if you want numbers to put to it.

  let startTime = Date.now();
  for (let node of collection) {
    visitNode(node);
  }
  let endTime = Date.now();
  console.log(`Time taken: ${endTime - startTime}ms`);

@rspbot
Copy link

rspbot commented Aug 26, 2025

@devongovett
Copy link
Member

btw I was thinking a while ago that it might be possible to do the flattening incrementally instead of copying the whole collection up front. Basically you'd do it on demand in getKeyAfter / getKeyBefore etc. by traversing down the tree. BaseCollection already kinda does this for sections. TreeCollection could be implemented in a similar way but for items. We'd need to handle the special cases that flattenTree has currently for loaders/content nodes though.

@snowystinger
Copy link
Member Author

@devongovett

Sure, but this is still a huge savings in the meantime and wouldn't preclude us from making those changes

@devongovett
Copy link
Member

Oh I didn't mean now!

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@snowystinger snowystinger added this pull request to the merge queue Aug 30, 2025
Merged via the queue into main with commit e717ee9 Aug 30, 2025
32 checks passed
@snowystinger snowystinger deleted the tree-performance branch August 30, 2025 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tree from react-aria crashes when expanding nodes muntiple in big trees
4 participants