Problems surrounding SSR injection of <style> and unreliability of :first-child selectors

[kylegach]

Problem description:

In emotion v10+, a new warning is thrown if you use :first-child, :nth-child, or :nth-last-child selectors in a styled component or css prop value. This is because, when using SSR, the style element is injected immediately above (prepended) the associated component. (More details.)

There are a number of problems with this approach:

Note: I made a test case demo to illustrate which selectors are unreliable.

1. In our codebase and matching years of experience for myself and peers, the "first" selectors are used much more often than the "last". Making these selectors unreliable is a big blow to those expecting standard CSS features to work.
2. The suggested workaround of changing, e.g. :first-child to :first-of-type, only works if all sibling elements are of the same type. This is probably fairly common for components like lists, grids, etc... But it's far from a guaranteed solution. It could also discourage the use of semantic markup by driving devs to "just use divs" for everything, since it's a simpler "fix".
3. The list of unsafe selectors is incorrect.
   • The adjacent sibling pattern, * + [anything], is unreliable, but not listed.
     • This type of selector may seem somewhat obscure, but it can be rather useful.
   • :only-child is unreliable, but not listed.
   • :nth-last-child is still reliable when prepending the style element and should not be listed.
4. Consumers not interested in SSR should be able to turn off the warning (Cannot turn off "potentially unsafe when doing server-side rendering" noise #1105)

Suggested solution:

Let's tackle each problem, in turn:

1. "First" vs. "last" selectors (or prepending vs. appending the style element)

   If emotion injected the style element after (appended) the associated component, then the following selectors would be unreliable: :last-child, :nth-last-child, :only-child.

   Compare that to the currently unreliable selectors: * + [anything], :first-child, :nth-child, :only-child.

   By changing to appending the style element, emotion would no longer block use of the common adjacent sibling selector and the "last" (rather than "first") varieties of the selectors would be affected, which I would argue is a better tradeoff more in line with actual usage of each [citation needed].

2. "of-type" workaround

   The warning's suggestion should be reworded to make clear that it may not work in all scenarios.

3. Incorrect unsafe list of selectors

   • :only-child needs to be added (changing this to :only-of-type is a good workaround, though!
   • If the style element remains prepended
     • The * + [anything] selector pattern needs to be flagged
     • :nth-last-child needs to be removed
   • If the style element is appended
     • Update list to include selectors in (1), above.

4. Mute the unsafe selectors warning

   At the very least, emotion should allow consumers to do so.

Alternative solutions:

Another suggested workaround is to somehow replace :first-child selectors with :first-child:not(style), style:first-child + * in the styles output. While a clever bit of CSS selecting, this suggestion has two main problems I can think of: 1) it won't work for non-:first-child selectors (though maybe we could come up with other rewrites for the others) and 2) replacing :first-child in a selector is not trivial, as it is affected by whether or not its used on the immediate element being styled or a descendent element.

The maintainers of emotion and its community could also simply decide that making these selectors unreliable is not worth the tradeoff of SSR "just working" and abandon the approach of injecting colocated style elements entirely. I have no idea how palatable such a change would be, but wanted to enumerate all the options.

---

This issue is intended to open discussion around these problems. If we can decide on forward actions, I'm happy to help implement wherever I can.

[FezVrasta]

Considering that Emotion knows if it's being server-rendered, it could even replace :first-child with :nth-child(N + 1) when server-side rendered, and adjust any :nth-child(N) accordingly

[OriginalEXE]

I don't think we can simply move the styles to after the elements as that would trigger the flash of unstyled content until the styles are parsed. I agree this is a pain in the rear, but I can't think of immediately obvious solutions. Does anyone have experience with other css-in-js libraries, how are they solving this problem, and is it actually solvable.

Rewriting of selectors sounds like a possible solution but it also sounds dangerous.

[FezVrasta]

Another possible solution would be to wrap the components into an additional div with display: contents; and use it to contain the style tag, but that wouldn't work on unsupported browsers.

[OriginalEXE]

Another possible solution would be to wrap the components into an additional div with display: contents; and use it to contain the style tag, but that wouldn't work on unsupported browsers.

Do you have time to create a simple example of how this would look like?

[FezVrasta]

I mean:

<div style="display: contents;">
  <style></style>
  <div class="css-xyz">stuff</div>
</div>

the display: contents will make sure the additional DIV doesn't break the layout, but it may break descendant selectors and adjacent ones so I don't think it's that feasible actually...

[OriginalEXE]

Hmm I don't think that would help, the .css-xyz component is still not a first child, I think you would have to do it this way:

<style></style>
<div style="display: contents;">
  <div class="css-xyz">stuff</div>
</div>

This solves the problem of the :first-child selector, but it breaks some other selectors, like immediate sibling X + .css-xyz. As you mentioned, it also only works in very modern browsers.

[kylegach]

Glad this spurred some discussion, and thanks for chiming in.

@FezVrasta

Replacing with :nth-child(N + 1) is an interesting idea. I thought selectors needed to work in both SSR and client "modes", but if emotion knows, there could be something here.

Using display: contents is a clever idea, but (currently) has serious accessibility flaws.

---

@OriginalEXE

don't think we can simply move the styles to after the elements as that would trigger the flash of unstyled content until the styles are parsed.

This came up in outside conversation, so I started looking for a reference to confirm. Do you know of one?

If you're right (I suspect you are), I suppose that makes the first problem in this issue moot.

[Andarist]

Replacing with :nth-child(N + 1) is an interesting idea. I thought selectors needed to work in both SSR and client "modes", but if emotion knows, there could be something here.

It needs to work in both modes, the problem at hand here is FOUC. The SSR styles are being moved at runtime to head - as emotion in browser works the same, it doesnt need to know much if the html was SSRed or not. Because of this the rules would have to rewritten both on server and on the client so they produce the same hash for caching purposes.

I think those rules CAN be rewritten reliably - it's just not super easy and need some effort into writing the smart-ish parser and writing comprehensive tests to ensure we doesnt break anything.

[OriginalEXE]

don't think we can simply move the styles to after the elements as that would trigger the flash of unstyled content until the styles are parsed.

This came up in outside conversation, so I started looking for a reference to confirm. Do you know of one?

If you're right (I suspect you are), I suppose that makes the first problem in this issue moot.

I was actually wrong, apologies for providing false information. It looks like there is no FOUC with the inline style tags, at least not in Chrome and Firefox. I was not able to find any conclusive info on it, though.

[Andarist]

Have you tried slower connections? :first-child will match "inline" style tags until your script gets downloaded & executed.

Maybe one of the options to avoid FOUC currently would be also to put this:

emotion/packages/cache/src/index.js

Lines 69 to 84 in 2971dba

	if (isBrowser) {
	container = options.container || document.head
	const nodes = document.querySelectorAll(`style[data-emotion-${key}]`)
	Array.prototype.forEach.call(nodes, (node: HTMLStyleElement) => {
	const attrib = node.getAttribute(`data-emotion-${key}`)
	// $FlowFixMe
	attrib.split(' ').forEach(id => {
	inserted[id] = true
	})
	if (node.parentNode !== container) {
	container.appendChild(node)
	}
	})
	}

into inline script tag 🤔

[OriginalEXE]

Sorry if I was not clear, I meant that I was not able to produce a FOUC with the style tags placed after the HTML they are targetting.

[OriginalEXE]

Example reproduction: https://s.codepen.io/OriginalEXE/debug/vvqbrb/vPAKKzQzPgVA

I have checked the rendering in chrome and the very first frame already includes the styled paragraph.

[Andarist]

Oh, the issue here is different though - FOUC in this case is caused purely by the fact that we render style tags together with elements and this messes up mentioned pseudo classes (:first-child etc)

[OriginalEXE]

I understand that. But the thread author initially proposed a solution of moving the style tags after the elements, hence solving the issue of the :first-child selector as it's more often used than the :last-child, and my immediate reaction was that this would cause FOUC, but it seems like that is not the case.

[Andarist]

Oh, I've misunderstood you then. If we can't confirm FOUC for this - I would be in favour of switching the order of those, technically this would be a breaking change though ;s

[kylegach]

If we can find a solution that doesn't make any selectors unreliable, that'd be great!

Thank you for the reproduction, @OriginalEXE! I'll investigate other browsers to confirm behavior there, too.

[OriginalEXE]

Yup, I think it would be a better solution, but it's definitely a breaking change. I am trying to dig up whether it would have any performance implications.

[OriginalEXE]

Maybe one of the options to avoid FOUC currently would be also to put this:
emotion/packages/cache/src/index.js

Lines 69 to 84 in 2971dba

if (isBrowser) {
container = options.container || document.head

const nodes = document.querySelectorAll(style[data-emotion-${key}])

Array.prototype.forEach.call(nodes, (node: HTMLStyleElement) => {
const attrib = node.getAttribute(data-emotion-${key})
// $FlowFixMe
attrib.split(' ').forEach(id => {
inserted[id] = true
})
if (node.parentNode !== container) {
container.appendChild(node)
}
})
}

into inline script tag

Here is a simplified version: https://s.codepen.io/OriginalEXE/debug/yGdrrL/mVkbGpLpGPvM

I was not able to reproduce FOUC this way either. I wonder however whether anything changes with a more complex DOM. Also, I believe not reproducing the FOUC should not be the only thing we consider before making any changes, performance implications should be tested as well.

[brentertz]

Hello everyone. Just weighing in with some comments and questions...

I appreciate the simplicity and relative elegance of Emotion’s SSR strategy, but I believe that the trade-off of “breaking” many commonly used CSS pseudo selectors to not be worth it.

Some pseudo selector usages can be changed to an alternate, but oftentimes there are others that get unwieldy fast or simply cannot be accommodated. In any of the cases, the updated implementations are unlikely to be the best choice or most easily understood. Further, the console error that suggests to try using nth-of-type is also not very helpful, particularly for those not even doing SSR. #1105.

May I ask what was the impetus to change from the previous SSR implementation? Performance, reliability, usability, difficulty, maintenance? I did not use it and therefore only have so much perspective on it. It seems that if it did not suffer from these issues, that perhaps it was a superior solution, albeit perhaps less elegant. Personally speaking, I would much rather have to deal with a bit of extra SSR configuration code as it is often a one time cost and limited in scope.

Is it possible to opt-out of the new SSR implementation for those cannot accommodate the concessions? I see that it is documented that the old SSR API’s can be used with Emotion 10, however with the caveat that is should only be done for compatibility and migration purposes. I can’t imagine that you would want to support multiple API’s long term though. It also would not address #1105.

A few comments on some of the other proposed ideas...

• Browser support and accessibility are paramount concerns.
• I don’t believe rewriting selectors to be a good idea. It seems potentially error prone, slow, heavy-handed, unexpected, and likely confusing to debug.
• Regarding an inline script insertion to move styles, I am not sure that I fully grok the suggestion. Is it one script tag per page or per component? If per component, it should ideally remove itself to avoid breaking any pseudo selectors. If once per page, would the script or function call need manually inserted by users in their server layout code? I suppose either of these sounds potentially viable, though I am unaware of what the performance costs might be and what rendering oddities might arise on slower connections.

[maximilianschmitt]

I don't think rewriting selectors is a good idea because it's confusing when debugging and also complex and error-prone.

The server-rendering of Emotion 10 unfortunately also messes up the following selector:

> * + * { margin-top: 5px; }

I agree with @brentertz that sacrificing common CSS selectors for easier setup is not worth it. At least not for us.

[bradwestfall]

Yep. @maximilianschmitt Just launched a new site and we're using Emotion 10 with a utility class for "lobotomized owl" (the * + * shown above). Just to catch anyone up on that selector, it's importance is that it selects all first level children except for the very first one to add margin top to. Without getting into the details of why this is so cool (read https://css-tricks.com/lobotomized-owls), the point is the mechanics of this selector are to only add margin to non-first child elements. So with Emotion we're experiencing a janky shifting around of things on initial load because <style> is the first child for a moment before it's relocated to the <head>. So what should have been my first div tag is actually second (getting margin top) and then becomes first so it loses margin top. Here is the fix we might be implementing:

.owl > * + * {
  margin-top: 1em !important;
}

.owl > style + * {
  margin-top: 0 !important;
}

The second selector is basically saying "Take the second child of .owl which is next to a style and lets just remove top margin now because when emotion js kicks in and removes style, the second element will become the first". This fixes the shifting.

Just wanted to put this solution here for others that stumble upon this

[jesstelford]

@bradwestfall

To stop the FOSC (Flash-Of-SSR'd-Content) when using the labotomized owl, I found this selector to be useful:

*:not(style[data-emotion-css]) + * {
  margin-top: 1em;
}

Edit, but there's also a case where the data- attribute is data-emotion, not data-emotion-css, which makes that selector not work, and the combination selector just boggles my mind.

[jesstelford]

It looks like most of the issues can be avoided by doing the following for a React app:

import createEmotionServer from "create-emotion-server";
import { CacheProvider } from "@emotion/core";
import createCache from "@emotion/cache";

let cache = createCache();

let { extractCritical } = createEmotionServer(cache);

let { html, css, ids } = extractCritical(
  renderToString(
    <CacheProvider value={cache}>
      <App />
    </CacheProvider>
  )
);

let styleTag = `<style data-emotion-css="${ids.join(" ")}">${css}</style>`;

(Thanks @mitchellhamilton for the suggestion)

[gersomvg]

@jesstelford That's what I will do also. Tried to rewrite a few selectors, but it's a pain. So a combination of extractCritical and muting the warning in the console will do for now.

I can't imagine basic CSS being sacrificed for the sake of magic SSR. People doing serious SSR are usually not the least skilled and people using NextJS for SSR are being pushed to styled-jsx by default anyway.

[joe-bell]

Cheers @jesstelford!

Probably a dumb question as I'm fairly new to tinkering with SSR (I'm currently stuck on this issue too), but am I right in thinking that styleTag you've used above means we don't have to worry about calling hydrate() as per the docs?

If this is the case, would it be worth my time raising a PR to update the documentation? I've been quite stuck trying to resolve this with the current docs

[Andarist]

A great podcast outlining some problems with those selectors (among other things) - http://www.fullstackradio.com/134

[jooj123]

I assume that the advanced approach (https://emotion.sh/docs/ssr#advanced-approach) wont work with setups for nextjs ?

as they dont expose the renderToString for it to be modified with CacheProvider and extractCritical etc

[azizhk]

@jooj123 Check if these help:
https://github.com/zeit/next.js/tree/canary/examples/with-emotion-10
https://github.com/zeit/next.js/tree/canary/examples/with-emotion-11

[jooj123]

awesome thanks @azizhk

[Vadorequest]

I experienced this warning today for the first time The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type". and after 30mn of reading through various issues, I kind of understand the issue.

But the warning is very misleading, and it's not explicit that it comes from Emotion either. Using the term unsafe made me believe it might be a security issue.

Could it be possible to:

• Make it clear it's an Emotion warning
  => Would help understand where this is coming from faster (DX)
• Add a link in the warning to learn more about the issue
  => Would help understand the implications of what we're doing faster (DX), and how disable those warnings (I haven't figure it out yet)
• Replace unsafe by unreliable in the warning

Couldn't Emotion rewrite those CSS rules, e.g: :first-child by something like :first-child:not(style) or similar?

If not possible to rewrite, could a solution be given within the warning (or its link)?
I'm thinking about something along the lines of Replace :first-child by :first-child:not(style).

[Andarist]

Yes, those are possible. I would be open to merging PRs improving any of those points.

As to rewriting CSS rules - it's hard to cover all possible use cases and we probably wouldn't like dive into this unless someone provides a quality PR for this (then we could start considering it). What we try to avoid here is potential FOUC and thus a simple replacement like you have proposed is not enough. This could work on the assumption that style is always a first child for Emotion SSRed output (which should always hold true): :first-child:not(style), > style + *:
https://jsfiddle.net/z4nskr61/

[Vadorequest]

Is there an existing webpage where this issue is explained thoroughly with developer experience in mind that could be printed alongside the warning? Or is it something that needs to be created?

Does using :first-child:not(style) triggers the warning at this time? We need to be careful about pointing out a solution that would still yield a false-positive warning.

[Andarist]

Or is it something that needs to be created?

It's vaguely mentioned here but a dedicated page or a new section should be created that would go into more details.

Does using :first-child:not(style) triggers the warning at this time?

It probably triggers it right now as it's currently just a very simple check.

[deansimcox]

Hey everyone, I just wanted to share a way to recreate the :first-child selector using :is() and :not().

  > *:not(:is(*:not(style) ~ *)) {
    float: absolute;
  }

This will select the first of any element that is not style and has adjacent siblings.
The only time this selector will break, is if there is only one child, because then the * ~ * will not match.

This selector can also have an element or class like this

  > article.this-article-has-a-class:not(:is(*:not(style) ~ *)) {
    float: relative;
  }