Cannot turn off "potentially unsafe when doing server-side rendering" noise

[RoystonS]

Emotion 10 is emitting a series of console warnings along the lines of:

The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to "first-of-type"
The pseudo class ":nth-child" is potentially unsafe when doing server-side rendering. Try changing it to "nth-of-type"

• emotion version: 10.0.4
• react version: 16.7.0-alpha2

Problem description:

1. I'm not using server-side rendering so these warnings aren't useful for me and are just console noise that means real problems will be masked. Apart from running in a production build, I can't see any way to turn these warnings off right now.
2. The error message doesn't help me understand why these pseudo classes are a problem; I can't find them mentioned in the Emotion docs, or references on the Internet as to why these might be a problem.

Suggested solution:

1. Provide a way to turn off these warnings if users don't want them.
2. Provide a link, or some documentation, as to why these pseudos are problematic.

[Andarist]

Leaving this here - https://github.com/emotion-js/emotion/issues/1059#issuecomment-444566635 as this issue is dedicated to this problem.

[vajkri]

We are getting the warnings too using SSR. The styles targeting with :nth-child(), :first-child or :last-child don't even get applied server-side, so this causes elements to "jump" on page load when these styles get applied client-side.

[brentertz]

Hi Folks,

The inability to disable these console messages is a blocker for us being able to ship the Emotion 10 update to our component library. As a library, these messages are also visible to all of our downstream consumers.

We do not wish to update the selector usage in our library as we do not believe Emotion's current SSR strategy to be a valid trade-off for breaking numerous commonly used CSS selectors, hence #1178. We are hopeful that an alternate approach can be found there as well, but in the mean time, simply would like to disable these console messages.

Is there anything that can be done here to move things forward? We are willing to do the work, or collaborate, if you can provide some insight into the direction that you would like to take.

Possible Solutions

• Simply remove logging; instead improve and rely on SSR documentation
• Conditionally remove logging, e.g. ENV var or other opt out mechanism
• Only log when server side rendering
• Only log message once per page load
  • Unsure if this would avoid excess logging during unit tests though
• Remove the Stylis “linting” plugins from the default cache implementation and make them opt-in
• Use linter/stylelint instead
  • Rules then become user configurable as ignored, warning, or error
  • Create and document a stylelint-config-emotion package
• Adjust the Stylis "linting" plugin to support a special style property that allows the message to be suppressed. See PR #1209

Additional Considerations

• Our product is an open-source component library, as opposed to an application, so preferably any solution will not place an additional configuration burden on downstream consumers.

Lastly, thank you for all of your efforts in creating and maintaining this project, which solves a lot of our problems!

[brentertz]

@Andarist Do you have any thoughts on my comments above?

[Andarist]

Well, personally I think the story around this has to be improved somehow. There is somewhat more fresh discussion around this here. Could you take a look there and comment what do you think about stuff mentioned there?

[brentertz]

Hi @Andarist Thank you for your attention.

I am aware of that issue and even link to it in my comments above. I'd be happy to comment on it further following this message. For full transparency, @kylegach and I work together.

Whilst Emotion's SSR implementation is certainly at the heart of this issue, I would imagine that is a bigger issue to resolve and I did not want to presume that you would be open to making changes in that area.

In the mean time, I believe that finding a way to resolve this issue would provide immediate value to many Emotion users, particularly those affected that aren't even doing SSR. I understand not wanting to put a lot of work into it if changing the underlying implementation could just make this issue go away, but what if it could be resolved with minimal effort and minimal impact?

I recently added another possible solution to the list in my comment above, which feels pretty lightweight. I'll reiterate it here.

• Adjust the Stylis "linting" plugin to support a special style property that allows the message to be suppressed. See PR #1209

[Andarist]

Unfortunately this is tightly coupled to SSR concerns - especially for your use case. From what I understand you develop a UI library which uses emotion and ideally your consumers shouldn't be aware of emotion and they should be able to do SSR without suffering the FOUC.

[brentertz]

I agree that this issue should ideally be solved by altering Emotion's SSR implementation to not “break” standard CSS, but I don’t believe that issue will be resolved in as timely a manner as this one may be mitigated. Again, I think #1209 resolves the issue quite minimally.

[RoystonS]

Whilst #1209 is a 'minimal' change to emotion, it's not a 'minimal' change for a user: it requires each use of a :first-child selector to be flagged with a really long "emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason" string. (This would bloat my output bundles in a rather unpleasant way.)

Isn't it the environment (SSR or not) that determines whether those selector usages are safe or not, rather than the individual uses? e.g. I'm using this in an environment that goes nowhere near SSR, so I'd much rather just tell emotion/stylis/cache/whatever once (not once per use) that it's safe to use these selectors.

[brentertz]

@RoystonS I agree with you. We had to work within the constraints of what the maintainers would allow. IMHO, #1209 is a workaround and the real way to solve this issue is to not break standard CSS.

FWIW, you can always assign the string to a variable. Also, the obnoxiously long comment is stripped from the build, so it should not affect your bundle size at all.

[RoystonS]

Yeah, I started putting it into a var, but then realised it was simpler just to adjust our build process to patch @emotion/cache (before we run webpack) to remove the warning. Not really an option for library authors, of course...

You mentioned the comment being stripped from the build - what would do that? It's a comment inside a string, so a minifier wouldn't know it can be removed... (And there's another copy inside @emotion/cache itself. :-( so my hack-the-package before build wins on both counts. :-) )

[brentertz]

@RoystonS My mistake, it feels like ages since I worked on this issue. The strings do remain in the bundle, though the effect could likely be minimized using process.env.NODE_ENV. It sounds like you are using a better solution for non-library authors anyway. :)

[ahutchings]

You mentioned the comment being stripped from the build - what would do that? It's a comment inside a string, so a minifier wouldn't know it can be removed...

My team is running into this issue. babel-plugin-emotion auto-minifies emotion CSS, stripping comments in the process. There doesn't appear to be an option to disable stripping comments, so unfortunately the comments to suppress the "unsafe selector" noise only work when not using the babel plugin (likely only in development). My team is using the babel plugin in tests for autolabeling and source maps, so we're seeing the warnings and there is no apparent way to suppress them. Until the larger issue is resolved (#1178), there are a few ways to mitigate this:

1. Only check for and warn about unsafe selectors in development (some sort of flag/option for @emotion/cache, environment variable, etc). Maybe less code could be shipped to prod this way?
2. Add an option to babel-plugin-emotion to disable stripping all comments.
3. Update babel-plugin-emotion to be aware of the "unsafe selector" comment and leave it in place automatically.

@mitchellhamilton @kylegach Do either of you have any thoughts around this? We may be able to contribute something for this via PR, but would like to have consensus on the direction before opening one.

[kylegach]

@ahutchings — The code, both to check and to warn, is wrapped in a not-prod check (line 174), so (1) is already the case.

I'll let Mitchell weigh in on the other points.

For clarification, I slightly misinterpreted Royston's comment earlier. I meant to say that the suppress comment is stripped in the output CSS; he is correct that it is still in the bundle itself (unless running babel-plugin-emotion, as you've discovered).

[ahutchings]

@kylegach Thanks, I missed that check at the top. Do you have a suggestion for how we can use the suppression comment when running unit tests in Jest (along with the babel plugin), where NODE_ENV is set to "test" automatically by the runner?

We're also developing a component library, and would like for our users to be able to take advantage of emotion source maps (provided by the babel plugin) in development without seeing the unsafe selector errors, but I suppose that is more the larger issue that needs to be resolved (#1178).

[kylegach]

@ahutchings — I don't, and I'm fairly sure a fix would require a code change in either the babel plugin or (somehow) the cache package. We don't run the plugin in our tests, so we missed this issue. Sorry about that.

Agreed that addressing the larger issue of having unreliable selectors at all would be a better long-term solution.

[kylegach]

@ahutchings — I think this may be more nuanced than we've discussed so far. Here's a codesandbox running our components, which have multiple unreliable selectors (e.g. in Button) and are using the plugin (via babel-preset-css-prop, which includes babel-plugin-emotion). If you open the console, you can see that it's not running in production mode, but there are no selector warnings logged.

I think this means it's at least possible to get the desired behavior in a test environment, but I'm not sure what about your setup is causing the issue to be sure.

[ahutchings]

@kylegach Thanks for your prompt responses and for looking into it further. The "ignore" comments are still in place in the version of mineral-ui pulled down in the sandbox, so the warning is not logged. The babel plugin may be leaving the "ignore" comments in place due to the way the comments are defined in mineral-ui source (object-based styles with a computed property + reference to the ignore string).

js _objectSpread2['& [role="img"]:first-child' + _emotion.ignoreSsrWarning]

We are using the tagged-template-literal-based method of defining styles in our components library, and the comments are stripped out whether we define them inline or using a reference to the "ignoreSsrWarning" variable like mineral-ui does.

[kylegach]

@ahutchings — Ahh, the meaningful difference is the object styles vs. tagged template literal styles. I'm unsure how to proceed from here, but I suggest starting with a new, dedicated issue to keep the conversation focused going forward. It still seems to me like the plugin code would need changed for the fix. I know stylis, which emotion uses to parse styles, already strips comments before outputting CSS, so it's redundant (and problematic) for the plugin to do the same.

[nelsonlaquet]

This is a pretty serious limitation. I understand it has to do with