-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix inferring data type of border-[…]
with multiple values
#17248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In order for a `line-width` to be valid, every part should be one of: 1. A keyword: `thin`, `medium`, `thick` 2. A length: `12px` 3. A number: `0`
return segment(value, ' ').every( | ||
(value) => | ||
isLength(value) || | ||
isNumber(value) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I know why isNumber
is here… it's for 0
support right? If so we should really adjust isLength
to account for unit-less zero instead I think.
Can do that in a separate PR tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah wasn't 100% sure where to put the unit-less numbers. Only annoying thing is that 0
can be put in spots where both a length
or percentage
is valid, so if we have a utility that differentiate between length
and percentage
then we run into an issue (and an explicit data type is necessary). Might not be a big deal 🤔
Also border-width: 1;
is interpreted as border-width: 1px;
but 0
stays as 0
(or at least by Lightning CSS). So we almost have to resolve the value based on the property we are setting: https://lightningcss.dev/playground/index.html#%7B%22minify%22%3Afalse%2C%22customMedia%22%3Atrue%2C%22cssModules%22%3Afalse%2C%22analyzeDependencies%22%3Afalse%2C%22targets%22%3A%7B%22chrome%22%3A6225920%7D%2C%22include%22%3A0%2C%22exclude%22%3A0%2C%22source%22%3A%22.foo%20%7B%5Cn%20%20border-width%3A%200%201%3B%20%2F*%200%201px%20*%2F%5Cn%20%20font-size%3A%201%3B%20%2F*%201px%20*%2F%5Cn%20%20--value%3A%201%3B%20%2F*%201%20*%2F%5Cn%7D%22%2C%22visitorEnabled%22%3Afalse%2C%22visitor%22%3A%22%7B%5Cn%20%20Color(color)%20%7B%5Cn%20%20%20%20if%20(color.type%20%3D%3D%3D%20'rgb')%20%7B%5Cn%20%20%20%20%20%20color.g%20%3D%200%3B%5Cn%20%20%20%20%20%20return%20color%3B%5Cn%20%20%20%20%7D%5Cn%20%20%7D%5Cn%7D%22%2C%22unusedSymbols%22%3A%5B%5D%2C%22version%22%3A%22local%22%7D
The inferring of data types is also not 100% correct conceptually because in this case it should be <line-width>{1,4}
and now we passthrough any number of arguments.
@RobinMalfait , The same problem persists in 4.0.15 when using a variable |
@christo-ivanov no that's not a bug. When using a variable we don't know the actual value of the variable, that's something you can only known at runtime. To solve this, you have to be explicit about its type: - border-[var(--bh-border-width)]
+ border-[length:var(--bh-border-width)] More info: https://tailwindcss.com/docs/adding-custom-styles#resolving-ambiguities |
This PR fixes an issue where arbitrary values such as
border-[12px_4px]
were incorrectly producingborder-color
instead ofborder-width
values.To solve it, I extended the
<line-width>
check to make sure that every part of the value is a valid<line-width>
.In order for a
line-width
to be valid, every part should be one of:thin
,medium
,thick
12px
0
Fixes: #17221
Test plan