-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Virtualizer: Optimize IO usage and export useMeasureList #32375
base: master
Are you sure you want to change the base?
Virtualizer: Optimize IO usage and export useMeasureList #32375
Conversation
📊 Bundle size report✅ No changes found |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
FluentProviderWithTheme | virtual-rerender | 40 | 32 | 10 | Possible regression |
All results
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 610 | 661 | 5000 | |
Button | mount | 328 | 302 | 5000 | |
Field | mount | 1135 | 1116 | 5000 | |
FluentProvider | mount | 731 | 702 | 5000 | |
FluentProviderWithTheme | mount | 84 | 85 | 10 | |
FluentProviderWithTheme | virtual-rerender | 40 | 32 | 10 | Possible regression |
FluentProviderWithTheme | virtual-rerender-with-unmount | 82 | 75 | 10 | |
MakeStyles | mount | 867 | 888 | 50000 | |
Persona | mount | 1760 | 1689 | 5000 | |
SpinButton | mount | 1382 | 1381 | 5000 | |
SwatchPicker | mount | 1661 | 1676 | 5000 |
change/@fluentui-react-virtualizer-bab73d05-7372-4052-8d22-ca75b5fadfe2.json
Show resolved
Hide resolved
3894a66
to
429d65a
Compare
0c84cd4
to
edd7073
Compare
packages/react-components/react-virtualizer/library/src/hooks/useMeasureList.ts
Outdated
Show resolved
Hide resolved
...ges/react-components/react-carousel-preview/stories/src/Carousel/CarouselDefault.stories.tsx
Outdated
Show resolved
Hide resolved
...s/react-components/react-virtualizer/library/src/components/Virtualizer/Virtualizer.types.ts
Show resolved
Hide resolved
...ages/react-components/react-virtualizer/library/src/components/Virtualizer/useVirtualizer.ts
Show resolved
Hide resolved
...ages/react-components/react-virtualizer/library/src/components/Virtualizer/useVirtualizer.ts
Show resolved
Hide resolved
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.
There's a breaking change in this PR by introducing the containerSizeRef
as a required property to the Virtualizer
component.
Also, I'd like to understand better what is happening here, so I'll take a bit of time to review properly, at the moment I'm blocking it as it introduces a breaking change and there's no warnings about it in the PR.
Breaking change: Additional info was required to be shared between hooks, this added new properties to our hook inputs which are required for accurate calculation of IO depth (Virtualizer is still in old preview format)
Previous Behavior
useMeasureList was not exported
IO would iterate based on it's current position
New Behavior
useMeasureList is now exported
IO now iterates based on how far into the IO the user scrolls, cutting down re-renders significantly