eslint-plugin-chakra-ui
eslint-plugin-chakra-ui copied to clipboard
[New Rule Proposal]: Use Value with Unit
Rule Summary
When non-united number is not predefined in theme, suggest adding unit for explicitness.
Example of incorrect code for this rule:
import { Box } from "@chakra-ui/react";
<Box p="11" mx="17" width="200">
Hello
</Box>;
Example of correct code for this rule:
import { Box } from "@chakra-ui/react";
<Box p="11px" mx="17px" width="200px">
Hello
</Box>;
It may be desirable to have this rule turned off by default.
Why needed?
In Chakra UI, we can use numbers for spacing without units. However, Chakra UI doesn't offer consistent behavior, and there are times when it is preferable to explicitly add units.
Inconsistency 1: Different conversion methods depending value is predefined or not.
First of all, the behavior differs depending on whether the value has already been defined.
- let x is 20, then output is
5rem
, it will be equal to80px
in most case. - let x is 21, then output is
21px
Chakra UI claims mental model of spacing:
Mental model: If you need a spacing of 40px, divide it by 4. That'll give you 10. Then use it in your component.
But it is not true model. Most numbers will not be quadruple.
For example, if you pass a variable to width
, some values are quadrupled, some are not. This is something that many developers would not expect.
Inconsistency 2: Some prop does not convert numbers as ${number}px
Secondly, in some case width
props can output numbers without px
or any units. ref: Different width depending on how props are passed
-
<Box w='11'/>
// output will be11
-
<Box w={11}/>
// output will be11px
I think both of these problems should be improved in Chakra UI itself, but I don't have much motivation to do so right now.
@Monchi what do you think? And if you think it's worth implementing, do you think it's better to avoid mixing p={12} and w="12px"? I mean,
-
Mixing
<Box w="11px" h="12">
-
Not mixing
<Box w="11px" h="48px">
@no-yan Totally agree. I can't believe the second inconsistency...
And if you think it's worth implementing, do you think it's better to avoid mixing p={12} and w="12px"?
I think we can prohibit number values since there is no merit except a very little reduction of bundle size.
Also I prefer the strict rule which forces the use of the single unit not only in the same element but in the whole application. How about providing options like below?
type Option = {
scope: "wholeApp" | "element",
allowedUnits: ("px" | "rem")[],
}
Thank you. I'm glad to hear it.
Also I prefer the strict rule which forces the use of the single unit not only in the same element but in the whole application. How about providing options like below?
LGTM!
Now let's implement it. I'll work on it after #21, but I don't mind if you do.
@no-yan Ok, so please assign you to the issue when getting to work on it to avoid double work!
I've made the prototype of the implementation, but I'm not sure which attribute is the one that receives the spacing value.
I will try to determine it by theme key from https://chakra-ui.com/docs/features/style-props. Column "Theme Key" seems to represent internal handling, and "space" correspond to "spacing" defined in theme.
If you have another idea, please let me know.
@no-yan Maybe you can refer this object but i'm not sure
This does not handle width etc, so the other objects are also involved? I will look it closer tomorrow.
@no-yan
First, chakra calls t.spaceT
for margin
here.
Then, t.spaceT
calls toConfig("space", transforms.px)
here.
space
is ThemeScale
that represents "Theme Key" in the doc, and transforms.px
is a transformer to convert unitless value to px value.
You can see the same thing for width
here.
So in summary, we can refer to ThemeScale
like space.margin.scale
or layout.width.scale
to know which prop is treated as a spacing value.
btw width
is t.sizesT
while height
is t.sizes
. This is the cause of your confusion 😂