react-spectrum
react-spectrum copied to clipboard
adobe
TableView reduce renders and onLoadMore
Closes
In collaboration with @devongovett This addresses some extra renders and extra layout effect calls that were noticed in the resizing PR
useTableColumnResizeState now has internal state, where, if no prop "onResize", is provided, it'll use its own state to cause a re-render. However, if onResize is provided, then it's assumed that the component calling the hook will trigger a re-render in some fashion. This could be from a layout or their own state management.
We make use of the onResize in our TableView because we have a layout and virtualizer, so we invoke the re-render with a relayoutNow.
Previously useTableColumnResizeState called a state update, and then in an effect later, relayoutNow was called. Now we just go directly to the relayoutNow, skipping the first render that the setState triggered.
This brought up a case where we still had two renders though, as setTableWidth causes a relayoutNow and setVisibleRect causes a relayout. The order we had them in meant that relayoutNow was called and then another relayout was scheduled after a raf. Instead, we can coalesce those into one render by scheduling the relayout and then immediately doing the relayoutNow which will handle everything in the scheduled update immediately as well.
This brought up yet another issue where we had some tests that accounted for way too many calls to onLoadMore, which we've always felt was off anyways. Our number of renders was now correct, but our layout effect was being called with a stale value, resulting in onLoadMore being called when it shouldn't be. We considered a few different ways of running the layout effect, however, we found our common methods of choosing a dependency array to be insufficient. I've outlined what we considered in a comment in the code.
Performance testing
Initial Load: no difference, the build on main spent 459ms in initial TableView, the build on my branch spent 450ms by comparison
Resize: build on main each resize took on average 95ms build on my branch each resize took on average 48ms
in the same span of about 1second, main managed 9 frames vs my branch did 14 frames main also averaged 2-3 recalculate style calls to the browser in between each resize, while my branch averaged 1
one interesting thing, that i don’t quite know how it fits in is that main ran each resize under onGlobalMessage and barely ran anything under onPointerMove meanwhile, my branch ran most of everything under onPointerMove and had a much much smaller onGlobalMessage call
✅ 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:
🧢 Your Project:
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉