used-styles icon indicating copy to clipboard operation
used-styles copied to clipboard

Issue with styles injected into select element

Open hnrchrdl opened this issue 1 year ago • 7 comments

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? :)

hnrchrdl avatar Dec 13 '23 14:12 hnrchrdl

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.

theKashey avatar Dec 13 '23 20:12 theKashey

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 avatar Dec 14 '23 09:12 hnrchrdl

<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.

hnrchrdl avatar Jan 06 '24 22:01 hnrchrdl

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:

  • list "good tags", ie allow style flush after body, div, span, younameit, both open and close. There is no "counting", script will just adjust a location of insertion from "prefix" to "in the middle of". If such location is not found - it will accumulate value till the next chunk
    • ease to do, requires testing
    • can include after tags like </select> or </script>.
  • list "bad tags" - script, select, will will find more. In this case option will be covered by parent select
    • just a few tags to track, not a big deal

In both cases there is question "what to delay"

  • it might delay styles until sees a good point to inject them. This might lead to some locations being temporary unstyled
  • it might delay content, until it is "closed", so it can dump all styles + all content. This version is "safe"

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

Thougths?

theKashey avatar Jan 08 '24 01:01 theKashey

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.

hnrchrdl avatar Jan 08 '24 09:01 hnrchrdl

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

theKashey avatar Jan 09 '24 07:01 theKashey

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

hnrchrdl avatar Jan 15 '24 00:01 hnrchrdl