Remove the negative flag from the Candidate AST #14938
Conversation
| utilities.functional('-scale', handleScale({ negative: true })) | ||
| utilities.functional('scale', handleScale({ negative: false })) |
There was a problem hiding this comment.
Could these use functionalUtility?
There was a problem hiding this comment.
Maybe if we apply some gymnastics. But I think we didn't use functionalUtility before because we register scale-x, scale-y and scale-z as separate functional utilities as well. If we make scale a functional utility, then candidates such as scale-x-50 will be handled by the scale functional utility with a value of x-50.
There was a problem hiding this comment.
Looked into it quickly, looks like the real reason we don't do it this way is because we handle arbitrary values differently than regular values — scale-[4_6] just produces scale: 4 6 and bypasses all the composition variables and stuff.
There was a problem hiding this comment.
That is also true. We could handle that when handling the functional utility.
Don't think that's worth it 🤔
| utilities.functional('-rotate', handleRotate({ negative: true })) | ||
| utilities.functional('rotate', handleRotate({ negative: false })) |
There was a problem hiding this comment.
Could these use functionalUtility?
| // Candidates that start with a dash are the negative versions of another | ||
| // candidate, e.g. `-mx-4`. | ||
| if (base[0] === '-') { | ||
| negative = true | ||
| base = base.slice(1) | ||
| } |
There was a problem hiding this comment.
Wondering if there are any migrations in the upgrade script that will no longer work now because we compare the base ignoring a potential -. I looked over simple-legacy-classes and it looks fine though. max-w-screen-* looked sus but didn't support negative prefixes in v3 either.
| @@ -228,8 +228,7 @@ export function buildPluginApi( | |||
| ) | |||
| } | |||
|
|
|||
| designSystem.utilities.static(name.slice(1), (candidate) => { | |||
| if (candidate.negative) return | |||
| if (supportsNegative) { | ||
| utilities.static(`-${name}-px`, () => handle('-1px')) | ||
| } | ||
| utilities.static(`${name}-px`, () => handle('1px')) |
There was a problem hiding this comment.
Wondering if this should use staticUtility for consistency now, since we also use the functionalUtility helper for the case below (Is it even necessary to add the staticUtility abstraction now in the first place though? 🤔)
| utilities.static(`${name}-full`, () => [decl(property, '100%')]) | ||
| utilities.static(`-${name}-full`, () => [decl(property, '-100%')]) |
There was a problem hiding this comment.
For consistency I think these should also use staticUtility:
| utilities.static(`${name}-full`, () => [decl(property, '100%')]) | |
| utilities.static(`-${name}-full`, () => [decl(property, '-100%')]) | |
| staticUtility(`${name}-full`, () => [property, '100%']) | |
| staticUtility(`-${name}-full`, () => [property, '-100%']) |
|
Thinking out loud: maybe |
adamwathan
force-pushed
the
feat/remove-negative-flag
branch
from
8d323b2 to
ca8e0c1
Compare
adamwathan
force-pushed
the
feat/remove-negative-flag
branch
from
ca8e0c1 to
a95c2d6
Compare
This PR removes the
negativeflag from theCandidateAST. The system itself doesn't this information at all, but it's up to each plugin to handle thenegativeflag themselves.This also means that if you don't handle it, that
fooand-fooresults in the same CSS output.To make sure that the negative version of utilities that supported it still work, this PR also adds the negative versions as separate utilities. E.g.:
-scaleis registered in addition toscale.This is an internal refactor only, and doesn't change any behavior.