[cache] Add ignore flag for the unreliable selectors warning

[kylegach]

What:

1. Add the ability to ignore a selector's "potentially unsafe when doing server-side rendering" warning message by adding /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ after the pseudo class
   • The flag is intentionally verbose (see comments below)
2. Add tests for above

Why:

The inability to disable or ignore these warnings is an issue for some (#1105). Until a decision is made on #1178, it is likely worthwhile to provide a way to ignore them. This seemed like a lightweight way of doing so, that is nicely co-located with the existing warning logic.

How:

Modified the implementation of the stylis plugin used for the warning to check for the presence of the flag.

Checklist:

• Documentation — Maintainers wish for this flag to be hard to discover, so docs were intentionally not updated
• Tests
• Code complete

[codecov]

Codecov Report

Merging #1209 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
packages/cache/src/index.js	100% <100%> (ø)	⬆️

[codecov]

Codecov Report

Merging #1209 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
packages/cache/src/index.js	100% <100%> (ø)	⬆️

[emmatown]

I'm not completely opposed to doing something like this but the way this works specifically would require us to also remove the property in production and I'm really against adding code in prod that isn't necessary in prod.

[brentertz]

@mitchellhamilton @kylegach I'd suggest that we simply change it to use a CSS custom property, e.g. --ignore, so that it it valid syntax and does not need removed for production.

[netlify]

Preview Docs

Built with commit 2920fc4

https://deploy-preview-1209--emotion.netlify.com

[kylegach]

@mitchellhamilton — Glad to hear you're open to the idea. I updated the code (force-pushed) and the PR description to reflect @brentertz's suggestion above.

[emmatown]

Changing it to a custom property doesn't really solve the problem. There's nothing that will break with shipping an invalid css property to production and it's better than a custom property in a way since browsers will ignore the invalid property. The bigger problem is that an extra css property is there at all.

I think it's possible to do with comments like this and since comments are already removed, it won't have any ramifications on production.

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

Yes, I know it's long, that's intentional, it should be difficult for people to disable the warning. It's there to help people and fix a problem they could have so it should be be difficult to stop the warning because in most cases, people should fix their code.

[kylegach]

@mitchellhamilton —

Oh! Thanks for the suggestion to place the comment in the selector. I had tried comments first, but couldn't figure out how to make it work for style objects.

Agreed that it's a good idea to have an obnoxiously verbose flag.

Just updated the PR description and force-pushed again. Beyond changing the flag to a comment, the major changes from last review are no longer printing the compiled selector in the error (decided it wasn't that helpful) and a bunch more test cases for compound selectors, which necessitated re-organizing that section of the spec a bit.

[Coolranch]

Did this flag make it in? I'm using version 10.0.7 with the disable flag in my styles, but I'm still seeing the warning.

[joshuaegclark]

Any updates on this getting merged?

[leftmostgeek]

I'd like to see this change too.

[kylegach]

Force-pushed one more time with the changes requested and updated the description accordingly. Should be good to go! Thanks for the guidance, @mitchellhamilton.