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

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.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 119
  • Comments: 64 (34 by maintainers)

Commits related to this issue

Most upvoted comments

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!

You can disable currently this by using comment in your styled:

css`
  :first-child /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this- 
 the-warning-exists-for-a-reason */ {
    color: hotpink;
  }  
`

Its really funny that after all this years there is no real solution to this.

Do we have a solution for this really really annoying log? We are not concerned at all about SSR today and we are annoyed all the time with this “Warning” that is even an error 🤦‍

Please can I mute this at the source? Thanks 🙏

Hey everyone, I just figured out a dirty way of getting rid of this irritating warning.

export const disableEmotionWarnings = () => {
  if (!process.env.NODE_ENV === "development") {
    return;
  }
  /* eslint-disable no-console */
  const log = console.error.bind(console);
  console.error = (...args) => {
    /* eslint-enable no-console */
    if (
      args.indexOf("The pseudo class") &&
      args.indexOf(
        "is potentially unsafe when doing server-side rendering. Try changing it to"
      )
    ) {
      return;
    }
    log(...args);
  };
};

I appreciate the emotion library and the developers who put so much work into it. However, it is time all library authors learn that it is up to developers to decide their project’s coding practices. It is arrogant for a ‘library’ to presume it knows best for every use-cases. This is a trend that needs to stop. The many complaints in this thread, over a 5 year period, illustrate how highly annoying this is.

Such ‘hints’ are useful, but there should be easy-to-use options. The ideal scenario is granular control, such as eslint-style rules, so that developers get to decide which warnings are appropriate and which are not. Ideally the severity (warning vs error) could also be configured. The message in this thread notes that it is only “potentially unsafe”, yet it still shouts at devs as a console ‘warning’. At a minimum this should only be a ‘warning’.

This non-applicable error caused some of my devs to change the accurate and superior use of :first-child with :first-of-type. This made the CSS less clear, and in at one case created a bug. Teaching the developers in my team how to write CSS is my job, not this library’s.

Again, thanks to all the maintainers of this library. However as the years of feedback above indicates, a more refined approach to such messaging would be a great improvement. I’m glad I found the compat option here, but this seems like a sledgehammer solution to a nuanced problem. It also seems this is still too hard to find for many users.

For anyone still experiencing this problem, I was able to work around this issue by essentially recreating :first-child with :first-of-type :

& > :first-of-type:not(style):not(:first-of-type ~ *),
& > style + * {
    margin-top: 0;
}

Hope this helps.

Fyi @caedney that style rule is going to be quite taxing on the CPU compared to :first-child.

I think that a simpler way to disable this should be a goal for Emotion 12.

This is closed, but what is the solution for that. We render content. And we do not know, what content will be displayed. But we need the first and the last element to have no margins. Regardless of its type. What’s the solution?

For anyone else who’s like me and has been saddled with this opinionated log spam just because you decided to use a dependent like Storybook (e.g. Did not choose Emotion directly and have no current desires to combine Emotion with anything server-side), here’s a (very hacky) workaround via a Webpack loader:

{
  test: /node_modules\/@emotion\/cache\/(src|dist)/,
  loader: "string-replace-loader",
  options: {
    search: "if (unsafePseudoClasses",
    replace: "if (false && unsafePseudoClasses",
  },
}

It’s rather irritating though that I have to resort to monkey patching something that could have easily been a config flag…

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.

This is a pretty serious limitation. I understand it has to do with <style> elements being added inline in the DOM when doing SSR; is it at all possible for emotion to… not do that?

I’ve been using emotion for a whole of a week and have ran into this issue multiple times, where using :first-of-type isn’t a valid substitute. To get around this issue I’ve had to add superfluous <div> elements, which clutter the DOM unnecessarily, and complicate the use of my components.

You can also disable those warnings by setting .compat = true on a cache.

Is there not a simple way to disable this in jest tests? I have no interest in server-side rendering, first-child is an extremely useful feature (and first-of-type is no substitute, the advice in the error message is bad).

I’ve tried some of the recipes mentioned in the thread but they don’t work since the package structure of emotion is different now (e.g. there’s no require(‘emotion’) any more.) Other engineers on my team have been complaining about this for 6 months now, and I keep coming back to this thread. 😦

That’s what we did in the end. Thanks.

I suggest to change that comment flag name (it’s too verbose and annoying to use if you have multiple instances of first-child). On 23 Nov 2019, 17:14 +0800, Mateusz Burzyński notifications@github.com, wrote:

Then I would go with option 1 as it allows for a low-level control over this for library authors. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Option 1

css`
  :first-child /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ {
    color: hotpink;
  }
`

Option 2

import { CacheProvider } from '@emotion/core'
import createCache from '@emotion/cache'

const myCache = createCache()
myCache.compat = true

<CacheProvider value={myCache}>
  <App/>
</CacheProvider>

Option 3

import { cache } from 'emotion'

<CacheProvider value={cache}>
  <App/>
</CacheProvider>

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.

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

Any update on this?

@sebastianfast is right, createCache function in v11 requires some cache-key configuration that must be lower case alphabetical.

image

image

And using the cache from @emotion/css kinda fixes that. Looking at the docs, this config should have been optional.

However, because mixing @emotion/css with v11’s @emotion/react doesn’t just feel right to me, I decided to simply pass to createCache function a random key config value like so:

const emotionCache = createCache({ key: 'my-lower-case-alphabetical-cache-key' });
emotionCache.compat = true;

And that worked for me.

@amcsi Rather than wrapping the div in an anchor, have you tried inverting that logic, so that all children are divs, e.g.:

<div />
<div><a /></div>
<div>

On the styling side, if you apply the same styling to each, you can do the following instead:

& + & {
  margin-left: 1rem;
}

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.

If I understand correctly I need to create an EmotionCacheProvider and wrap my app with it. I am afraid the app will end up with 2 caches. This one and some internal one by MUI. Since I have no idea what the Emotion Cache is and I don’t want to dive into it, I add the following to my app which silences the error also.

const consoleError = console.error;

console.error = function filterErrors(msg, ...args) {
    if (/server-side rendering/.test(msg)) {
        return;
    }
    consoleError(msg, ...args);
};

Copied from here https://github.com/oliviertassinari/react-swipeable-views/issues/534#issuecomment-1396138110 and adjusted…

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

hi @seafoam6, the string check should be args[0]

Hey everyone, I just figured out a dirty way of getting rid of this irritating warning.

Thank you! Works like a charm. But i think the first if should look like this, otherwise the result will always be false.

if (process.env.NODE_ENV !== "development") {

@Andarist OK that worked - or rather, I wrote a wrapper around react-testing-library’s ‘render’ function that added the CacheProvider. Thanks!

Option 2 fixed it for me, I was having these warnings when running Jest+RTL tests and adding a test wrapper with a CacheProvider fixed it:

import { CacheProvider } from '@emotion/core'
import createCache from '@emotion/cache'

const emotionCache = createCache()
emotionCache.compat = true

<CacheProvider value={emotionCache}>
  /* run tests here */
</CacheProvider>

@Andarist could you provide more insight on what emotionCache.compat = true does? I haven’t found docs for this in the Emotion cache docs.

@bradleyayers this is a bug in the parser we are using, I’ve reported it there https://github.com/thysultan/stylis.js/issues/152

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

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