withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
45.82k stars 2.41k forks source link

Unexpected slow initial render when using "visibility: hidden" and "<label>" #10327

Closed fasiha closed 6 months ago

fasiha commented 6 months ago

Astro Info

Astro                    v4.4.9
Node                     v21.6.2
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/solid-js

If this issue only occurs in one browser, which browser is a problem?

Checked Safari and Firefox and Chrome

Describe the Bug

I ran into a weird performance bug that hits when I have two conditions:

  1. a lot of <label> DOM nodes in a Solid component with client:visible, which
  2. renders inside a hover box using visibility: hiddenvisibility: visible in CSS.

In the minimal reproducible example https://github.com/fasiha/hover-label-astro-solid-demo, if you load the page, there's a few seconds of lag as the tab pegs the CPU to 100% (Firefox profiler output shows that "Layout: query selector" is fully occupying the CPU for 2.7 seconds).

While I understand that visibility: hidden is probably making Astro+Solid render all the <label> nodes on page load because these are still sort-of-visible due to visibility: hidden CSS, the really surprising thing is that if I replace <label> with <span> or <div> or <>, the problem goes away. This is why I thought to open an issue—in case there's a bug with how Astro is handling <label>? Specifically, if I replace the <label>s in Word.tsx with <>, Firefox profiler shows that refresh takes 177 ms and doesn't peg the CPU at all.

The other way I can fix this problem is to, instead of using visibility CSS, use display: none and display: block, i.e., to remove the non-hovering elements from page flow. That makes <label> behave the same as <span> etc.

What's the expected result?

When using client:visible Astro directive and visibility CSS, I expect a Solid component that generates a lot of <label> elements to load just as fast if it generated a lot of <span> or <>. That is, <label> should not be abnormally slow compared to these alternatives.

Link to Minimal Reproducible Example

https://github.com/fasiha/hover-label-astro-solid-demo

Participation

fasiha commented 6 months ago

Here's a screenshot of the Firefox profiler showing the issue with <label>: 2.7 seconds:

firefox-profiler

In contrast, here's the profile after replacing <label> with <> in Word.tsx in the minimal reproducible example: 177 ms:

firefox-profiler-fast
bluwy commented 6 months ago

From the stack trace, looks like it's coming from the @medv/finder dependency that the dev toolbar uses.

fasiha commented 6 months ago

@bluwy well-spotted! That's the culprit: if I disable the dev toolbar in astro.config.mjs (devToolbar: { enabled: false },), all is well with <label>.

Will dig more in the morning but happy to close this if it's not an Astro problem but rather a Dev Toolbar/finder issue.

bluwy commented 6 months ago

I don't think it's necessarily an issue with @medv/finder. It's only used to generate a query selector string, from a given HTMLElement. The reason why div or span worked is because it doesn't have a11y reports compared to labels. Using labels, the audit shows `label` element should have an associated control and a text content.

I'm not sure what the correct solution is here, but perhaps we can make auditing lazy to workaround this issue.

Princesseuh commented 6 months ago

Hmm, auditing is (supposed to be) lazy. Dev toolbar plugins are initted in a requestAnimationFrame that's in a DOMContentLoaded.

There's definitely performance improvements available, though. I'll take a look.