Fixed an issue with styles containing a trailing css variable not being inserted correctly where the styled were optimized by our Babel plugin to a static opaque object #2283
Conversation
🦋 Changeset detectedLatest commit: b1df9ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -164,7 +164,9 @@ let createCache = (options: Options): EmotionCache => { | |||
| } | |||
| } | |||
|
|
|||
| stylis(selector ? `${selector}{${serialized.styles}}` : serialized.styles) | |||
| stylis( | |||
| selector ? `${selector}{${serialized.styles};}` : serialized.styles | |||
There was a problem hiding this comment.
this is the core of the fix - } is allowed in css variables so it was parsed as part of it and the whole thing resulted in incorrect styles as the opaque object does not contain a trailing ;
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b1df9ff:
|
Andarist
force-pushed
the
fix/2219
branch
2 times, most recently
from
dc8e335
to
40a8344
Compare
…ng inserted correctly where the styled were optimized by our Babel plugin to a static opaque object
| const EMOTION_BABEL_PLUGIN_ANNOTATION_REGEX = /\*?\s*@emotion\/babel-plugin\s+(.+$)/ | ||
|
|
||
| // this is really intended to only be used internally | ||
| const getEmotionBabelPluginPragmaOptions = state => { | ||
| if (!state.file.ast.comments) { | ||
| return {} | ||
| } | ||
| return state.file.ast.comments | ||
| .map(comment => comment.value.match(EMOTION_BABEL_PLUGIN_ANNOTATION_REGEX)) | ||
| .filter(Boolean) | ||
| .map(match => JSON.parse(`{${match[1]}}`)) | ||
| .reduce((a, b) => Object.assign(a, b), {}) | ||
| } |
There was a problem hiding this comment.
I'd rather we didn't have this because people could start relying on it. Maybe we could do the same parsing in the babel test infra?
There was a problem hiding this comment.
We could - that was my initial intention but I didn't have time to figure out how this should be done properly. Do you have any idea how this could be done without sharing the parsed options through the state (which would basically also allow people to abuse it)? I didn't want to set up any more complicated infra that would sidestep Babel for this.
In the ideal situation, we'd just provide those options through regular options but that can't be done easily to the best of my knowledge (unless we read files in test and do our own parsing there).
fixes #2219