-
Notifications
You must be signed in to change notification settings - Fork 435
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
fix(deps): upgrade @tanstack/react-virtual
to v3.11.2
#8062
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Dec 16, 2024 10:38 PM (UTC) ❌ Failed Tests (2) -- expand for details
|
⚡️ Editor Performance ReportUpdated Mon, 16 Dec 2024 22:41:02 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
Nice! We have a few uses of this in the content releases branch, so just commenting myself here to remember to rebase our branch once this is in :) |
Edit: tests updated to pass on the new version |
3c4e600
to
1ebac8a
Compare
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
1ebac8a
to
a1f8a99
Compare
@tanstack/react-virtual
to v3.11.1@tanstack/react-virtual
to v3.11.2
a1f8a99
to
3688db4
Compare
3688db4
to
60e83ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the array inputs in the debug scenarios and other things, everything looks good to me, thanks for taking care of this!
Well this is unfortunate, I'd mistaken React 19 support for react compiler support... but alas this issue still exists |
Description
Moves the dependency out of the beta range, and brings React 19 support which is why this is happening now.
@binoy14 There were some minor crashes in the array input after upgrading - the
observeElementOffset
seems to need a second argument,isScrolling
. I assumed that since we were calling this from a scroll handler it would be safe to set this to always betrue
, but we're also calling it once on "mount", at which point I am giving itfalse
. Additionally, the virtual items seems to start at an empty array now, which crashed when attempting to use the first element in the styles. Can you see that things works as expected? I did some testing and to my eyes it appears to work fine.@robinpyon I am requesting a review from you since you have been involved in multiple components that utilize this library: scheduled publishing and CommandList being the two I can find. CommandList is being used quite a number of places, and from what I can tell things seems to work fine - but would really appreciate a quick run through to double check.
What to review
That things still work as expected. Surfaces affected are (at least):
Testing
No new tests were added - relying on existing tests and manual review.
Notes for release
None