[@mantine/hooks] use-window-scroll: optimisations to reduce re-rendering on resize and scroll #8574
+30
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's no point letting JavaScript/React update more often than the browser repaints.
requestAnimationFrameautomatically syncs our updates to the display's refresh rate. This is less opinionated than using throttling or debouncing.The API stays identical, consumers of the hook don't need to change anything but we should see some performance improvements on low-end devices.
How this relates to other places where Mantine uses
requestAnimationFrameI looked at hooks like
useScrollIntoViewanduseResizeObserverthat also userequestAnimationFrameand named my ref the same as theirs (frameID).There were a few areas where I was deliberately inconsistent:
useResizeObserveranduseMovereset the value offrameID.currenteach time by cancelling the existing frame ID then rescheduling a new one. My approach is to skip if a frame is already scheduled. This prevents unnecessarycancelAnimationFrame()calls on every event and means that events are handled in the very next frame rather than being delayed by the cancel existing frame -> schedule a new one -> handle event chain.useResizeObserveruses0as the default value, I usenull. This is based on this MDN comment:as well as the semantics of
nullmore clearly meaning "no frame is scheduled"How is this different to #8287?
Given that #8287 caused #8320 we want to make sure the same thing doesn't happen again.
Use of the Mantine
useWindowEventhook#8287 used the native browser API to set its event listeners, not
useWindowEvent.Memory leak/stale closure bug
#8287 seems to have a stale closure value bug so the RAFs are never cleaned up:
My approach uses
useRef, andrafRef.currentwill always be the latest value.Unnecessary renders
IN
#8287we always callgetScrollPositionand always update the value of the position state:In my approach we don't update the state if the scroll position has not changed:
This prevent re-renders, for example it does not change any state during a resize event.
My approach adds
passive: trueto the scroll and resize listenersWithout the passive option browsers can't optimise scrolling and must run the scroll handler before the scroll can finish. This blocks the main thread and can lead to jankiness.
Single value for deduplication and cleanup
In my approach the RAF ID serves two purposes (tracking + deduplication). #8287 needs two variables (
ticking+rafID), one for deuplication and one for cleanup.Summary of the changes
The main change is to the function that is run on each scroll and resize event:
handleScrolluseCallbackuseCallbackso that it becomes a stable reference to pass touseWindowEvent(this is necessary becauseuseWindowEventhaslistenerin its dependencies and functions are not stable references)requestAnimationFramerafRefguardsetPosition(getScrollPosition()){ passive: true }{ passive: true }, the browser can start scrolling immediately in parallel with running handlersWe use a ref to store the ID of any scheduled
requestAnimationFrame. It serves two purposes:The RAF throttling flow: