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 - #1059 (comment) 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 [cache] Add ignore flag for the unreliable selectors warning #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 [cache] Add ignore flag for the unreliable selectors warning #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 <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

elements, which clutter the DOM unnecessarily, and complicate the use of my components.

[antoinerousseau]

Anyone knows if this happens using styled-components too? Or is it emotion-specific?

[Mario-Eis]

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?

[coolassassin]

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

Hello, I have tried this code and it breaks all console.error calls.
And I can explain you why.
The argument args is an array, but you work with it like with string.
Also using indexOf as is, it is a potential bug, which we can see here.
If something not matching the case we have -1 in both cases and !!(-1 && -1) === true

So I see comments bellow about your mistakes, but you didn't fix them.
So, that is the right one to avoid mistakes for guys like me:

export const disableEmotionWarnings = () => {
  if (!process.env.NODE_ENV !== 'development') {
    return;
  }
  const log = console.error.bind(console);
  console.error = (...args) => {
  if (
    typeof args[0] === 'string' &&
    args[0].includes('The pseudo class') &&
    args[0].includes('is potentially unsafe when doing server-side rendering. Try changing it to')
  ) {
    return;
  }
  log(...args);
  };
};

[Andarist]

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

[caedney]

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.

[Nantris]

@Andarist is there no way to disable this without the need for a custom cache? It's rather onerous.

[Nantris]

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.

[Schnueggel]

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

[kinoli]

You can check if you are in Jest and only include the pseudo code if you are not in jest. This works for me.

${!process.env.JEST_WORKER_ID
      ? css`
          *:first-child {
            margin-top: 0;
          }

          *:last-child {
            margin-bottom: 0;
          }
        `
      : null}

[Hideman85]

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 🙏

[allpro]

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.

[christiaanwesterbeek]

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 oliviertassinari/react-swipeable-views#534 (comment) and adjusted...

[dvegap95]

About the :first-of-type approach, I have a button that can have an Icon beside text, I use [role='figure']:first-child so if it appears at the begining gets a different margin on the left, I cannot do this with :first-of-type, cuz I would still have the appended icon with a margin left applied, since it would be the first image within the button. :first-child is the way to go here, I don't use SSR, why should it even mention SSR without any context?

[phiter]

I don't understand why this is still a thing. We don't use SSR and yet this error is still present.

[yaindrop]

Continuing the discussion from the previous comments, I figured out that this modification of console.error can be more considerate. By using useEffect clean-up function it will prevent unnecessary multiple wraps around the original console.error, which can happen if the function is triggered multiple times, such as during live-reloading of the dev server.

function filterConsoleError(): (() => void) | undefined {
  if (process.env.NODE_ENV !== 'development') {
    return
  }
  const consoleError = console.error.bind(console)
  const emotionUnsafeSsrRegex = /The pseudo class "[^"]+" is potentially unsafe when doing server-side rendering/g
  console.error = (...args: string[]) => {
    if (emotionUnsafeSsrRegex.test(args[0])) {
      return
    }
    consoleError(...args)
  }
  return () => {
    console.error = consoleError
  }
}

function App() {
  useEffect(() => {
    return filterConsoleError()
  }, [])
  ...
}

That said, while I appreciate the hard work of the library's authors, it's unfortunate that we end up discussing "hacky" modifications to console.error for what seems like a trivial issue. The recommended options are clearly unsatisfactory when it's not in a SSR use case, as frankdilo, RoystonS and allpro have persuasively pointed out.