feat: add support for makeResetStyles transforms to CSS extraction#250
Conversation
📊 Bundle size reportUnchanged fixtures
|
3c471d6 to
f5260f6
Compare
| if (isResetClassName(element.value)) { | ||
| return 'r'; | ||
| } |
There was a problem hiding this comment.
This does not seem to scale well. We will need probably to revisit this place with a more solid options. But before we will need to confirm that there are real troubles already...
There was a problem hiding this comment.
sorry I don't understand why this is not scalable
There was a problem hiding this comment.
It's not handling global selectors and few other cases:
body .baz { color: orange; }
/* ⬇️⬇️⬇️ */
/* element.value is "body .baz" */
I will work on integration of CSS extraction to one of products and will be looking into potential issues to see how to make sorting better (or reimplement it completely).
P.S. It worth creating an issue, but as I don't all cases yet I postponed creating it.
| import { COMMENT, compile, Element, KEYFRAMES, MEDIA, RULESET, serialize, stringify, SUPPORTS, tokenize } from 'stylis'; | ||
|
|
||
| function isResetClassName(className: string) { | ||
| return className[0] === '.' && className[1] === RESET_HASH_PREFIX; |
There was a problem hiding this comment.
why do you need to check that the first character is . ?
There was a problem hiding this comment.
can stylis return something other than . as the first character ?
There was a problem hiding this comment.
Yes, tr .foo for example.
| const cssRules = evaluationResult.value as string[]; | ||
|
|
||
| state.cssRules!.push(...cssRules); | ||
| argumentPaths[2].remove(); |
There was a problem hiding this comment.
| argumentPaths[2].remove(); | |
| argumentPaths.remove(); |
you set the argumentPath correctly for both cases
There was a problem hiding this comment.
I would suggest moving argumentPath.remove() out of conditions. Likewise you can also just initialize cssRules as an empty array earlier on and move the state update out of conditions too
| } | ||
|
|
||
| if (argumentPaths.length !== 2) { | ||
| if (functionKind === '__styles' && argumentPaths.length !== 2) { |
There was a problem hiding this comment.
nit: you could just parametrize the two errors ?
There was a problem hiding this comment.
you could actually parametrize a lot:
- argument position
- argument length
- replacement function kind
It avoids some duplication, but it's not a lot so it's your call
Fixes #231.
__resetCSSfunctions to@griffel/core&@griffel/react, similar to chore: add __resetStyles functions to core & react #249, but these functions don't do insertion to DOM@griffel/webpack-extraction-pluginto handle__resetStylessortRulesto handle new classes