Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow optional @ inside /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ #1705

Closed
xxx opened this issue Dec 31, 2019 · 3 comments
Milestone

Comments

@xxx
Copy link

xxx commented Dec 31, 2019

The problem

We're have a library, and want to turn off the first-child, etc. warnings, since SSR is not a concern for us. I'm aware of adding /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ as a comment, but because we also use babel-plugin-emotion, the comment is simply stripped away during minification.

babel-plugin-emotion skips removal of comments that have @ embedded within them, and it would be nice to be able to add that to the magic comment that, at this time, must be character-for-character identical to what I pasted above.

Proposed solution

Loosen the requirements to use the magic comment such that a @ can be added, to prevent babel-plugin-emotion from stripping it away during minification.

Alternative solutions

Only emit the warning if actually used in an SSR context.

Additional context

N/A

@Andarist
Copy link
Member

Andarist commented Jan 1, 2020

This indeed sounds useful, I think we should squeeze this into upcoming v11. Would you maybe be willing to work on this? I believe we should just make this comment kept for non-production code as-is, no @ required.

@Andarist Andarist added this to the v11 milestone Jan 1, 2020
@xxx
Copy link
Author

xxx commented Jan 2, 2020

I will see what I can do, but my time is squeezed at the moment. I don't know what the schedule for v11 is.

@Andarist
Copy link
Member

I've implemented this change as part of the #1817 , any comment that includes this ignore flag will work when this PR lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants