Fix tagged templates minifier #1836
Conversation
🦋 Changeset is good to goLatest commit: 1ed6211 We got this. This PR includes changesets to release 4 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 |
|
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 1ed6211:
|
Codecov Report
|
|
@mitchellhamilton this is ready for review now - I'm targeting |
| @@ -710,7 +710,7 @@ import { css } from '@emotion/react'; | |||
| function media(...args) { | |||
| return ( | |||
| /*#__PURE__*/ | |||
| css(\\"@media (min-width:100px){\\", | |||
| css(\\"@media (min-width: 100px){\\", | |||
There was a problem hiding this comment.
Because stylis is crazy-ass parser 🤣and it doesn't try to minify the input too hard, just some whitespace collapsing, nothing more.
It also tries to minimize information that it needs to track along the way, especially given the fact that it often doesn't yet know what it is currently parsing - is it a rule? is it a declaration? don't know - until it reaches either { or ;
So what happens here is basically that whitespace needs to be preserved in css functions like calc (probably not all of it, but at least collapsed to a single space - although it doesn't try to collapse it here). So it just consumes anything between ( and ) as is - or rather between opening and closing tokens. The same delimit function is used for single quotes, double quotes, parenthesis, square brackets and obviously for strings we need to preserve all whitespace whatsoever.
I will probably revisit this later because we can minify this on our side, but didn't want to overcomplicate this PR.
* Stylis v4 tryout * Move @import rules in test to be first * Improved compat plugin * Add tests for orphanated pseudos * orphanated -> orphaned * Upgrade stylis and improve compat plugin * Improve compat plugin - avoid double compilation * Shorten compat plugin a little bit * Add guard for global top-level rules in the compat plugin * Fix tagged templates minifier (#1836) * Add TS test to check for excessive instantiation regressions * Fix tagged templates minifier * Use non-forked stylis for minification * Pass missing argument to toInputTree and fix isAutoInsertedRule for parent-less rules * Temporarily add @emotion/stylis as dep of @emotion/babel-plugin * Update snapshots * Make compat plugin be always included * move removeLabel into omnipresentPlugins * Stop special-casing @import insertion * fix getServerStylisCache * Remove outdated docs around stylisPlugins and prefix options * Add changesets * Add note about prefixer being just a plugin to stylisPlugins docs * Actually use officially published Stylis v4 * Improve error message about incorrect @import insertion * Fix flow errors * Reword error messages and changeset content Co-authored-by: Mitchell Hamilton <mitchell@hamil.town> * update snapshots * Remove mention of `@import` not being usable in `css` calls as it wasnt true for @emotion/react+<Global/> * Add mention to the changeset about where one can find a prefixer * Update .changeset/warm-ties-drop.md Co-authored-by: Mitchell Hamilton <mitchell@hamil.town>
* Stylis v4 tryout * Move @import rules in test to be first * Improved compat plugin * Add tests for orphanated pseudos * orphanated -> orphaned * Upgrade stylis and improve compat plugin * Improve compat plugin - avoid double compilation * Shorten compat plugin a little bit * Add guard for global top-level rules in the compat plugin * Fix tagged templates minifier (emotion-js#1836) * Add TS test to check for excessive instantiation regressions * Fix tagged templates minifier * Use non-forked stylis for minification * Pass missing argument to toInputTree and fix isAutoInsertedRule for parent-less rules * Temporarily add @emotion/stylis as dep of @emotion/babel-plugin * Update snapshots * Make compat plugin be always included * move removeLabel into omnipresentPlugins * Stop special-casing @import insertion * fix getServerStylisCache * Remove outdated docs around stylisPlugins and prefix options * Add changesets * Add note about prefixer being just a plugin to stylisPlugins docs * Actually use officially published Stylis v4 * Improve error message about incorrect @import insertion * Fix flow errors * Reword error messages and changeset content Co-authored-by: Mitchell Hamilton <mitchell@hamil.town> * update snapshots * Remove mention of `@import` not being usable in `css` calls as it wasnt true for @emotion/react+<Global/> * Add mention to the changeset about where one can find a prefixer * Update .changeset/warm-ties-drop.md Co-authored-by: Mitchell Hamilton <mitchell@hamil.town>
Just how HTML should not be parsed with RegExp, CSS should not be either 🤪
By using a proper CSS parser we can write context-based minifying rules to avoid some pitfalls, the code also becomes much more readable.
I'm using a forked Stylis here to achieve the goal because I don't want to expand the input string to proper~ CSS nodes, but rather maintain hierarchy so I can serialize it back to a string which is just a minified input and nothing more than that. It's just a single change in Stylis source code, but it can't be achieved without forking. cc @thysultan - it might be interesting to you to see how I've bent and used Stylis here. Would you maybe also like some license/author header put into the fork file? If yes - how it should look like?
Keep in mind that this probably should land after Stylis v4 gets published to npm. It would be great to avoid checking in its source code.