[Violation] Forced reflow while executing JavaScript took <N>ms

dan5py commented 2 years ago

Some elements like (Dialog, Dropdown menu, Select, Popover, ...) cause a Forced reflow and this can lower the performance of the component and the entire page.

andy-hook commented 2 years ago

@Dan5py These types of performance warnings depend on many variables including complexity of page, performance of client and other factors, there might be more we can do to optimise but without a better understanding of your project and usage it would be hard to assess in a meaningful way.

Are you able to provide a "reduced" example showing these violations?

christiankaindl commented 2 years ago

I am having a similar issue, in my case with Dialog. Clicking the Dialog trigger takes up to 1 second, depending on the size of the page. The larger the page the bigger the lag. E.g. this page seems to have no delay, while this page suffers quite a bit. The issue seems a bit more severe on Firefox compared to Chrome (at least on my Intel MacBook Air), but it is very noticeable nonetheless. The following recording was taken on this page: https://dev.gesetzefinden.at/bundesrecht/bundesgesetze/abgb

Here's a video showing the issue in action:

https://user-images.githubusercontent.com/15364860/193408962-9b3d177b-7388-4306-8e82-91e5dc3fcdda.mov

However, the issue only persists when modal is set to true, which leads me to believe that the issue is caused by the "aria-hidden" package on this line, as that is the only noticeable difference I could find between modal and non-modal implementations (but that's just a guess):

https://github.com/radix-ui/primitives/blob/1f66c88538da13f5ec6b4e97f215268df1a3baa2/packages/react/dialog/src/Dialog.tsx#L262

ahmadaf94 commented 2 years ago

Same here with select component

benoitgrelard commented 2 years ago

However, the issue only persists when modal is set to true, which leads me to believe that the issue is caused by the "aria-hidden" package on this line, as that is the only noticeable difference I could find between modal and non-modal implementations (but that's just a guess):

It's more likely to be an issue with re-layout because of react-remove-scroll which restyles the body.

I am not sure there is much to be done about this here, you could maybe experiment with CSS containment in your specific case to lower down the amount of reflow happening. Unfortunately as @andy-hook pointed out, these types of issues are rather specific to use-cases/complexity of specific pages.

christiankaindl commented 2 years ago

It's more likely to be an issue with re-layout because of react-remove-scroll which restyles the body.

You're right, that seems to be a big part of the problem, but I think it's not the only bottleneck. We have tried a workaround using modal={false} and handling body-scroll-locking manually. This approach still has some lag, but it is less severe. It's consistently slower without the workaround (around 15-30%). Compare (opened/closed the dialog trigger 6 times each):

With modal={true} (measured on this page):

With modal={false} and manually setting "overflow: hidden" on the body (measured on this page):

I've done some quick profiling: the style re-calculation stayed roughly the same (which is expected), but the click handler is much faster on the non-modal version:

modal:

non-modal:

As you can see the click-handler in the took much longer. The messages in the console also changes from "click handler took " to "message handler too ". But maybe I'm misreading something

I'll try adding more containment and maybe replicate with other libraries, and will report back 👍

benoitgrelard commented 2 years ago

Have you tried with non-modal and not implementing any scroll locking?

andy-hook commented 2 years ago

and handling body-scroll-locking manually

I'm also curious whether the upcoming native scrollbar-gutter would show a big improvement on these large pages.

christiankaindl commented 2 years ago

Have you tried with non-modal and not implementing any scroll locking?

Non-modal without scroll locking is butter smooth (see below).

I'm also curious whether the upcoming native scrollbar-gutter would show a big improvement on these large pages.

I didn't know about that property, thank you! I played around with scrollbar-gutter as well as body locking and have some interesting findings:

TL;DR The problem is not overflow: hidden, but rather touch-action: none/pointer-events: none and CSS custom properties set on the body

First, my scroll locking implementation was setting the following properties on the <html> element: overflow: hidden, touch-action: none and a custom CSS variable --scrollbar-offset that is used to prevent the body or navbar from jumping when overflow: hidden is applied.

Just setting overflow: hidden is actually very fast. With Chrome there's around 50-100ms lag caused by text-reflow at some widths. scrollbar-gutter: stable helps with preventing the layout shifts, but is not significantly faster compared to manually preventing the layout shift with margin-right: 15px. Also, scrollbar-gutter doesn't seems to prevent layout shifts for elements with position: fixed like navbars or the modal itself, so I still need to work around that manually.

If I add either --scrollbar-offset CSS var or touch-action: none (or both) to the <html> element I start to see a 200-350ms lag with a message appearing in the console telling me about a forced reflow (pointer-events: none had the same effect). CSS vars are inheritable so that might simply be bad performance (see bug 1056209). Why touch-action is so slow I don't know, but maybe it's not needed anymore in modern browsers anyways?

I will try implementing the following and then report back:

Use scrollbar-gutter if available and fall back to setting margin-right: <scrollbar-width>px to prevent layout shift on the body
Remove touch-action: none
Remove --scrollbar-offset and replace with a JS solution where components that need it can subscribe to it (e.g. via Zustand)

andy-hook commented 2 years ago

Thanks for the additional research @christiankaindl , did your findings take you anywhere?

Here's another example:

https://user-images.githubusercontent.com/11708259/198611169-cbf18d22-0525-4f4b-8ac8-834ea287f707.mp4

https://workos.com/docs/reference

christiankaindl commented 2 years ago

@andy-hook yes, sorry for not following up sooner. I did land on a solution that removes most if not all of the lag. This is what I currently do in my custom solution:

Only set minimal styles on the :root element (overflow, and if scrollbar-gutter is not supported also margin-right)
Save the scrollbar width to a store, so that elements with position: fixed/absolute can subscribe to it (no CSS variable propagation, which is too slow)

function useScrollLock (enabled: boolean = true): void {
  useIsomorphicLayoutEffect(() => {
    if (enabled) {
      const supportsScrollbarGutter = CSS.supports('scrollbar-gutter', 'stable')
      const scrollbarOffset = scrollbarWidth() ?? 0 // In my implementation, `scrollbarWidth()` comes from the package "@xobotyi/scrollbar-width" and returns the number of pixels the scrollbar is wide 

      document.documentElement.style.setProperty('overflow', 'hidden')
      if (!supportsScrollbarGutter) {
        // Fallback when scrollbar-gutter is not supported in the browser
        document.documentElement.style.setProperty('margin-right', `${scrollbarOffset}px`)
      }
      // Even when scrollbar-gutter is supported set this value, so that elements with `position: fixed` can subscribe to it to prevent layout shifts. Could be any store mechanism that supports subscribing to a value, like Zustand, Svelte Stores, Valtio, etc...
      scrollbarOffsetStore.set(scrollbarOffset)

      // Cleanup
      return () => {
        document.documentElement.style.removeProperty('overflow')
        if (!supportsScrollbarGutter) {
          document.documentElement.style.removeProperty('margin-right')
        }
        scrollbarOffsetStore.set(0)
      }
    }
  }, [enabled])
}

You can see the before/after yourself, by clicking on one of the "Anmerkung" buttons. -> Radix with modal={true}: https://gesetze-finden-kpcq86cbs-christiankaindl.vercel.app/bundesrecht/bundesgesetze/abgb -> Radix with modal={false} and custom scroll locking: https://gesetze-finden-omlw801cy-christiankaindl.vercel.app/bundesrecht/bundesgesetze/abgb

Note: having a lot of content in the modal (or something that is expensive to render) also contributes to the forced reflows in my testing. The reason for that is probably, because Radix removes the dialog content DOM when the dialog is not open, so it has to be re-created on every time it is opened--unlike Ariakit for example.

kachkaev commented 1 year ago

@christiankaindl interesting, thanks for sharing your workaround! Do you know if it is applicable to Select? I can’t seem to find modal={false} in this component.

UPD potential fix: https://github.com/radix-ui/primitives/pull/2253

lakbychance commented 1 year ago

@benoitgrelard Not totally sure how much our issue ties into this but we are using Modal (created using Dialog internally with open as true on Root) and facing a weird experience in Firefox. Our use case is like this :-

A user scrolls down the page and we use custom logic to set a boolean state to true
Based on that boolean state, we show the modal like so :-
```
{showModal && <Modal/>}
```
In Safari and Chrome, the Modal appears at the bottom of page only but in Firefox, the user gets scrolled to top of the page and then Modal appears there. If we use modal=false, the jumping to top stops but defeats the purpose of using Dialog for a Modal for our use case.

We do not find this issue occuring in firefox if a button is clicked to open the sameModal at bottom of the page.

Any pointers as to what might be happening here ?

jacobsoftwares commented 1 year ago

Have a look at the events hooked up on the body tag of your website... Reason I think is repetitive duplicate event hooks

awfulminded commented 9 months ago

I had simillar issue with chakra-ui

https://github.com/chakra-ui/chakra-ui/issues/7909

using --removed-body-scroll-bar-size: 0px !important; on root or body seems to works fine with chakra. However the same approach is not working with radix.

modal={false} works fine for sheet, select and dropdown menu, but not working for dialog, the dialog is completely gone if applied. The worst lags are in the Safari on iOS, event with around ~1500 nodes on the page it took around 2-3 seconds to just open a dialog

vitorsilvalima commented 8 months ago

My customer just sent a video reporting that our new version which uses Dialog from shadcn is extremely slow compared to the previous version which used antd modal

She is currently using Chrome on Windows

https://github.com/radix-ui/primitives/assets/9752658/c2bd86f4-09fb-4561-8001-25a961e7a6d5

ashot commented 6 months ago

How can an issue like this persist for years. This should be hair on fire stuff no?

raisiqueira commented 6 months ago

As an alternative to the radix-dialog component, I'm using the ark-ui dialog component. Maybe this gist can help someone here: https://gist.github.com/raisiqueira/4e29b734c49533822189a4c0dfa77931

jonathandion commented 6 months ago

On Safari, the modal takes a noticeable amount of time to open after being triggered. The delay seems to be related to the number of items present in the DOM. It's not related to the height of the page. I have built a small demo https://p3c695.csb.app/item/5

taigrr commented 5 months ago

This seems to have a compounding effect with pdf.js, likely because of all the extra DOM elements.

AayushKarki714 commented 4 months ago

I was facing the same Issue and looked after radix documentations website. I found out, It also has the same issue.

Is there any ways to fix it??

christiankaindl commented 4 months ago

I have since switched to Ariakit, so other than my workarounds mentioned here https://github.com/radix-ui/primitives/issues/1634#issuecomment-1296698480 I don't have any other solutions.

taigrr commented 4 months ago

Oh, I added a few useMemo calls on the dom-heavy components and the issues disappeared. might want to look into giving that a try!

suitux commented 4 months ago

Same here

altyntsevlexus commented 3 months ago

Problem still exists. If we throttle CPU x4/x6 it is eye-visible that Dialog or Select opens with a delay (0.5-1s). I've tried doing it on the Radix website and problem exists there too. I hope this gonna be fixed soon.

AlfredMadere commented 1 month ago

This seems to have a compounding effect with pdf.js, likely because of all the extra DOM elements.

Noticing this as well. BIG issue.

taigrr commented 1 month ago

@AlfredMadere wrap it in useMemo.

AlfredMadere commented 1 month ago

@AlfredMadere wrap it in useMemo.

@taigrr Wrap what exactly? I tried wrapping my pdf viewer in React.memo and confirmed that it does not rerender when the dialog is opened, however the reflow is still taking 2000ms.

 const DebouncedResizingPDFViewer = React.memo(
  ({ pdfUrl }: { pdfUrl: string }) => {
  console.log('i rerendered') //This is not logged when the dialog is opened or closed
  //rending the pdf in here but removed for simplicity
  }

Attempting to contain it using css containment also had no effect

 <div
        className="pdf-viewer-wrapper absolute inset-0"
        style={{
          contain: 'strict',
          transform: 'translateZ(0)',
          willChange: 'transform'
        }}
      >
           <DebouncedResizingPDFViewer pdfUrl={pdfUrl} />
</div>

jhawson commented 1 month ago

I love Radix, but two years is a long time to go without a fix for this issue. As a short-term solution, I temporarily gutted my wrapper for the Radix Dialog component and used React Aria Dialog to fulfill the interface contract.

AlfredMadere commented 1 month ago

I love Radix, but two years is a long time to go without a fix for this issue. As a short-term solution, I temporarily gutted my wrapper for the Radix Dialog component and used React Aria Dialog to fulfill the interface contract.

@jhawson would you be willing to share a snippet?

jhawson commented 1 month ago

@jhawson would you be willing to share a snippet?

@AlfredMadere Unfortunately, it's not an open source project. And the code involves some nuances that are specific to our use case that would clutter up an example. But I found the React Aria documentation quite easy to follow; I would recommend just giving it a try as a short-term solution, while we wait for a fix.

pnd280 commented 1 month ago

I love Radix, but two years is a long time to go without a fix for this issue. As a short-term solution, I temporarily gutted my wrapper for the Radix Dialog component and used React Aria Dialog to fulfill the interface contract.

Not just Dialog, any component with a floating mechanism also suffers from the same issue: Dialog, Popover, Tooltip, Select, Dropdown, etc. I'm wondering if your project will also replace them. If so, what's the point of using Radix primitives anyway?

jhawson commented 1 month ago

Not just Dialog, any component with a floating mechanism also suffers from the same issue: Dialog, Popover, Tooltip, Select, Dropdown, etc. I'm wondering if your project will also replace them.

With Popover and DropdownMenu, there is at least an option to disable modal behavior. For me, that's sufficient because I don't typically want modal behavior with those components. But unfortunately, there's no option to disable modal behavior for Select (#1927). So yes, I'm now contemplating whether to also temporarily replace my Select implementation using another headless library.

If so, what's the point of using Radix primitives anyway?

Overall, I think Radix has a great API, clear documentation, and is generally solid in most other respects. But this issue makes it very difficult to use Radix on pages with complex content. And it's not clear that the Radix team intends to develop a solution (either because they don't think it's important, or they lack the resources to make the necessary improvements; the silence here speaks volumes). So you might have a point...

pytnik89 commented 2 weeks ago

The simplest way to reproduce the issue is by visiting the official Radix site and using the dialog example. Clicking “Edit Profile” in the example triggers the modal, which results in a “performance warning.”

taigrr commented 2 weeks ago

My issue cropped back up even with useMemo. I ended up throwing tanstack/virtual at the problem and the issues seem gone for real now.

lime517 commented 2 weeks ago

Kinda blown away that this issue exists after 2 years+, and is present on the Radix site itself :(

radix-ui / primitives

[Violation] Forced reflow while executing JavaScript took <N>ms #1634