Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with styles injected into select element #56

Open
hnrchrdl opened this issue Dec 13, 2023 · 7 comments
Open

Issue with styles injected into select element #56

hnrchrdl opened this issue Dec 13, 2023 · 7 comments
Assignees

Comments

@hnrchrdl
Copy link
Contributor

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
Copy link
Owner

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 theKashey self-assigned this Dec 13, 2023
@hnrchrdl
Copy link
Contributor Author

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
Copy link
Contributor Author

hnrchrdl commented Jan 6, 2024

<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
Copy link
Owner

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?

@hnrchrdl
Copy link
Contributor Author

hnrchrdl commented Jan 8, 2024

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
Copy link
Owner

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

@hnrchrdl
Copy link
Contributor Author

as a starting point, here is a failing test #57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants