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._
: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".* + [anything], is unreliable, but not listed.:only-child is unreliable, but not listed.:nth-last-child is still reliable when prepending the style element and should not be listed.Suggested solution:
Let's tackle each problem, in turn:
"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].
"of-type" workaround
The warning's suggestion should be reworded to make clear that it may not work in all scenarios.
Incorrect unsafe list of selectors
:only-child needs to be added (changing this to :only-of-type is a good workaround, though!style element remains prepended* + [anything] selector pattern needs to be flagged:nth-last-child needs to be removedstyle element is appendedMute 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.
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
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.
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.
Another possible solution would be to wrap the components into an additional
divwithdisplay: contents;and use it to contain thestyletag, but that wouldn't work on unsupported browsers.
Do you have time to create a simple example of how this would look like?
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...
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.
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.
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.
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.
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:
https://github.com/emotion-js/emotion/blob/2971dba3d4a30be10adea2a7afae047ed83eea0e/packages/cache/src/index.js#L69-L84
into inline script tag 🤔
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.
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.
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)
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.
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
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.
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.
Maybe one of the options to avoid FOUC currently would be also to put this:
emotion/packages/cache/src/index.jsLines 69 to 84 in 2971dba
if (isBrowser) {
container = options.container || document.headconst 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.
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...
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.
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
@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.
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)
@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.
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
Just to echo what @jesstelford has said and suggest a path forward.
1) Document the two SSR approaches, the magic one liner which has this limitation OR use extractCritical. Each are supported and have their pros/cons, ie no streaming support when using extract critical vs css limitations
2) A change is made to emotion so if you are using extractCritical the warning is muted (because it doesn't apply)
In my mind it solves this issue?
We gave 2 a try, but couldn't find a nice place to put a flag when extractCritical is called which could be checked when the warning is about to be output.
Any thoughts on the above @mitchellhamilton? I don't mind helping, I just do not know the codebase well enough to implement suggestion 2.
I think this makes sense to me, I think I'm okay with adding a check, it should be as simple as adding a check around the block at https://github.com/emotion-js/emotion/blob/master/packages/cache/src/index.js#L204-L227 if cache.compat === undefined
Created a PR to progress the conversation
Going back to https://github.com/emotion-js/emotion/issues/1178#issuecomment-461524602 and https://github.com/emotion-js/emotion/issues/1178#issuecomment-469010046 on "lobotomized owl" selectors (* + *), the following worked for me:
export const Example = styled.div({
'& > * + *': {
marginTop: 16,
},
'& > style:first-child + *': {
marginTop: 0,
},
});
This is far from ideal though. I really appreciate the hard-work achieved by the Emotion team to get us all this far but I agree with @brentertz, the trade-off of “breaking” commonly used CSS pseudo selectors isn't worth. it. We should be encouraging developers to make use of the cascade, not shy away from it
I couldn't get the extractCritical approach to work with my next js app, but this works for me...
import Document from "next/document";
const filterEmotion = htmlIn => {
const styles = [];
const unique = {};
const html = htmlIn.replace(
/<style data-emotion-css="([^"]+)">(.+?)<\/style>/g,
(all, id, content) => {
if (!unique[id]) {
styles.push(
<style
key={id}
data-emotion-css={id}
dangerouslySetInnerHTML={{ __html: content }}
/>
);
}
unique[id] = true;
return "";
}
);
return [styles, html];
};
export default class MyDocument extends Document {
static async getInitialProps(ctx) {
const initialProps = await Document.getInitialProps(ctx);
const page = ctx.renderPage();
const [styles, html] = filterEmotion(page.html);
return {
...initialProps,
html,
styles
};
}
}
Here's a potential workaround for folks looking for a style-only alternative (i.e. no walking through children, no requiring the extractCritical SSR API) to the first-child or nth-child selectors.
These styles work by stacking up the adjacent sibling selector to reach the desired index. This means that a) you can only target a specific index (e.g. 0, 3) and b) they'll need to be programmatically generated for index you need (but hey, this is CSS-in-JS).
These styles make me kind of sad, but I'm hoping to just wrap them up in a helper and forget about this whole thing.
/*
* This will select the 3rd child when no style
* tag is present (client-side rendering, or SSR
* with the critical-styles API). This style will
* not apply if a style tag is present.
*/
.container > *:first-child:not(style) + * + * { /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */
background-color: red;
}
/*
* This will select the 3rd non-style child when a style
* tag is present (normal SSR). This will not apply
* if no style tag is present.
*/
.container > style + * + * + * {
background-color: red;
}
Demo in JS Bin: https://jsbin.com/xejekumuyu/1/edit?html,css,output
Considering this one is merged: https://github.com/emotion-js/emotion/pull/1483 Can this be closed? Or is it waiting for docs?
I've noticed a few warnings we are still getting, also I haven't done the docs. I'll try sort it out after I get my types PR merged
@jooj123 That PR looks like it's more about fixing the warning right? I think this issue is more about the fact that
Most helpful comment
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...