theKashey / used-styles

📝All the critical styles you've used to render a page.
MIT License
137 stars 9 forks source link

Issue with styles injected into select element #56

Open hnrchrdl opened 8 months ago

hnrchrdl commented 8 months ago

Another issue popped up, where styles are injected into a <select> element, but are not wrapped into a <style> tag, which means moveStyles won't recognize them, so they are not moved into head before hydration, and therefore breaking hydration.

image

Anything we can do about this to fix it? :)

theKashey commented 8 months ago

wondering why they are not wrapped in a style block. Probably they are in "raw HTML", but invalid tag is removed lated in the browser. Please check how two sources match eachother.

as an example <div> tags will be hoisted from <p> tags as they cannot be nested.

And yeah, there is a solution. We just need to make tracking a little bit smart and account not only for style tags, but for script(which can be very long) or ➡️ select, which is expected to contain some options, but not styles.

hnrchrdl commented 8 months ago

yes, you are right, the server renders the page with <style> tags inside the <select>, and browsers seem to strip / sanitize by throwing them out but leave the childNode as string type.

image

thanks for looking into it!

hnrchrdl commented 7 months ago

<option> tags are also affected. these must also not contain any <style> tags.

anything we can do to fix this? it is quite annoying because i get a lot of hydration errors recently due to this.

theKashey commented 7 months ago

With not all tags being "self closed" it's a little hard to "count" them in order to understand is it a good moment to dump styles. I see two options:

In both cases there is question "what to delay"

I think the second one is better, and it's implementation gravitates towards counters from the first section.

Thougths?

hnrchrdl commented 7 months ago

You would know better how to fix it properly.

When it comes to allow list vs. block list, a block list (the second option) sounds more reasonable to me intuitively. It also builds on what we already implemented for avoiding nested <style> inside <style>.

about the delaying of content, i am not so sure if we should go that route, because it messes with the original stream (at least ro my limited understanding, correct me if I am wrong).

so, i think i would go with the second option and (as a first step) just delay the styles.

theKashey commented 7 months ago

Some experimentation is needed here. Cannot promise any quick resolutions. If you need to unblock yourself you may temporary switch to bulk rendering

hnrchrdl commented 7 months ago

as a starting point, here is a failing test https://github.com/theKashey/used-styles/pull/57