Collect user-defined CSS that can be used in @apply
#13349
Conversation
RobinMalfait
force-pushed
the
feat/apply-user-css
branch
from
dfd601f
to
e7836e7
Compare
RobinMalfait
force-pushed
the
feat/apply-user-css
branch
from
e7836e7
to
0906a55
Compare
| node.kind === 'rule' && | ||
| node.selector[0] === '@' && | ||
| node.selector.startsWith('@apply') | ||
| ) { |
There was a problem hiding this comment.
I'd flip the condition and do an early return here.
| // processing, we have a circular dependency. | ||
| if (candidate === rootAsCandidate) { | ||
| throw new Error( | ||
| `You cannot \`@apply\` the \`${candidate}\` utility here because it creates a circular dependency.`, |
There was a problem hiding this comment.
I think it'd be worth printing the selector of root in the error message to help guide the user to the thing with the circular dependency.
| i += 1 | ||
| continue | ||
|
|
||
| case ' ': // Descendat combinator |
There was a problem hiding this comment.
| case ' ': // Descendat combinator | |
| case ' ': // Descendant combinator |
| // class that is a very simple selector, like `.foo` or `.bar`, but doesn't | ||
| // contain any spaces, combinators, pseudo-selectors, pseudo-elements, or | ||
| // attribute selectors. | ||
| node.selector[0] === '.' && |
There was a problem hiding this comment.
I believe you don't need node.selector[0] === '.' ?
Because inside isSimpleClassSelector, you perform this test with an early return:
if (selector[0] !== '.') return false
I would suggest to remove this line
| node.selector[0] === '.' && |
There was a problem hiding this comment.
This is still required, if you comment this out a test should fail. This is basically for the scenario where you have .foo.bar which contains 2 classes which is not allowed.
| node.selector.startsWith('@apply') | ||
| ) { | ||
| let candidates = node.selector | ||
| .slice(7 /* Ignore `@apply ` when parsing the selector */) |
There was a problem hiding this comment.
I really enjoy the comments in your code, you could have use
.slice('@apply '.length)instead of
.slice(7 /* Ignore `@apply ` when parsing the selector */)Off course the latest is slightly faster & shorter (if you omit the comment).
In the end, I prefer your version.
Longer with the comment but once you read it once, it feels right 🤓.
There was a problem hiding this comment.
Yep we focus on performance and re-allocating and re-computing the length is unnecessary work.
This allows you to apply user-defined CSS inside of
@apply. It works a tiny bit different compared to v3 because we only allow you to apply simple selectors. Only a single class can be applied such as a.foo {}.If you want more complex selectors, then you can use nesting to enhance the definition of what
.foomeans exactly.Implementing it this way allows us to keep the codebase simple and keep expectations simple.
In v3 you can do crazy things such as:
And you can apply
@apply fooor@apply baror@apply baz, but that's not intuitive if you look at the final result.This PR also ensures that you can't apply classes that would result in circular dependencies.
What this PR is not:
This PR does not make custom classes "utilities" or "components". This means that you won't be able to do this:
Fixes: #13342