Skip to content

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Jan 30, 2024

This PR reduces the INP impact of the autofocus react attribute by deferring the focus event until next frame.

Calling .focus causes layout thrashing which causes UI jank. We are currently able to observe this using browser profiling where a lot of the call stacks point towards a .focus perf culprit.

CleanShot 2024-01-30 at 09 29 07@2x

Having looked at prod profiles and react source, the function responsible for calling .focus is commitMount, called during a finalizeInitialChildren. I did not verify when exactly this is called, but from the profiling data, this always appears to always be the last function in the call stacks when committing changes to the DOM.

The PR mitigates the sync nature of the .focus call by removing the autofocus attribute and scheduling the autofocus to be performed in the next browser event loop tick, yielding to the engine and splitting the cost over two multiple browser frames.

@JonasBa JonasBa requested a review from a team January 30, 2024 14:38
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 30, 2024
Copy link
Member

@k-fish k-fish left a comment

Choose a reason for hiding this comment

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

Let's try it 👍

@JonasBa JonasBa merged commit 5a40712 into master Jan 30, 2024
@JonasBa JonasBa deleted the jb/fix/assignee-selector-inp branch January 30, 2024 15:29
snigdhas pushed a commit that referenced this pull request Jan 30, 2024
…#64165)

This PR reduces the INP impact of the autofocus react attribute by
deferring the focus event until next frame.

Calling [.focus](https://gist.github.com/paulirish/5d52fb081b3570c81e3a)
causes layout thrashing which causes UI jank. We are currently able to
observe this using browser profiling where a lot of the call stacks
point towards a .focus perf culprit.

![CleanShot 2024-01-30 at 09 29
07@2x](https://github.com/getsentry/sentry/assets/9317857/98e5b8dc-565c-4b3f-9106-5dcf032a46ef)

Having looked at prod profiles and react source, the function
responsible for calling .focus is
[commitMount](https://github.com/facebook/react/blob/2477384650bd184d3ac4a881130118f2636f8551/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js#L676),
called during a finalizeInitialChildren. I did not verify when exactly
this is called, but from the profiling data, this always appears to
always be the last function in the call stacks when committing changes
to the DOM.
 
The PR mitigates the sync nature of the .focus call by removing the
autofocus attribute and scheduling the autofocus to be performed in the
next browser event loop tick, yielding to the engine and splitting the
cost over two multiple browser frames.
JonasBa added a commit that referenced this pull request Jan 31, 2024
Same optimization as #64165 -
defer focus event to next frame.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants