-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[system] Experiment with sx
as string
#23631
Conversation
That doesn't format nicely at all. What is the problem with the object API that the string solves? |
@eps1lon Agree, it's best to start the description of the experiment with the problem statement :).
It resonates with front-end developers and designers that write a lot of styles and don't spend a lot of time maintaining it.
@mnajdova Maybe we don't need to support |
My opinion on writing vs reading hasn't changed. We have enough data that suggest that we mostly read rather then write code so micro optimizations for writing aren't a problem worth working on and optimize the less used activity.
That would be a definitive rejection from me. Now people would not only need to know about two overloads abut also the feature set these overloads support. |
@material-ui/core: parsed: +0.11% , gzip: +0.13% |
Updated the PR description.
This is my biggest concern but as @oliviertassinari pointed out is mostly about the nested selectors, not that much about the pseudo selectors. We could expose the pseudo selectors in a form like: On the other hand, for nested class selectors, people could potentially use the sx prop on the element inside, but I wonder how easier this would be instead of writing the object on the top element. 🤷 Lots of people use tailwind obviously and are used to/prefer this syntax, that is the main reason for the experiment (see problems and possibilities) |
This is an awesome idea! Other reasons to choose this approach
However, to make this solution viable we need something like Tailwind CSS IntelliSense This is a great opportunity to remove the weight of type checking CSS types from typescript-language-server and make intellisense for styles much faster and theme aware via a custom extension |
This could be even statically extracted 🤯 A babel plugin could
The generated stylesheet would look something like this .color\:primary\.main {
color: #23633 ;
}
.m\:4 {
margin: 20px;
} |
This is a very bad idea, We can ship function Component() {
return <Box
sx={{
color: 'primary.main',
bgcolor: 'secondary.main',
m: 2,
p: 1,
fontFamily: 'fontFamily',
fontWeight: 'fontWeightLight',
fontSize: 'fontSize',
displayPrint: 'block',
border: [1, 2, 3, 4, 5],
}}
/>
} to: function Component() {
return <Box sx={styles.box(props)} />
}
const styles = createStyle({
box: (props, { primary, secondary }) => ({
color: primary.main,
bgcolor: secondary.main,
m: 2,
p: 1,
fontFamily: 'fontFamily',
fontWeight: 'fontWeightLight',
fontSize: 'fontSize',
displayPrint: 'block',
border: [1, 2, 3, 4, 5],
})
}) A bit like to React Native style |
I really like the exploration with the string options but wanted to give some input: I've been working extensively with the If vertical spacing is an issue, wouldn't it be easier just to extract the If strings are really faster, the babel plugin could translate down to the string or whatever css is optimal. The object is easy to hoist as a babel plugin: plugin codeexport default function () {
return {
name: 'hoist-attribute',
visitor: {
JSXAttribute(path, state) {
const attributesToHoist = state?.opts?.attributesToHoist || [
'sx',
]
if (attributesToHoist.includes(path?.node?.name?.name)) {
const expressionPath = path.get('value.expression')
expressionPath.hoist()
}
},
},
}
} I'd say the worst part about tailwind is actually having to learn the pseudo-language that is which classname corresponds to the actual css. I like how we're using the "normal" names when possible, but if I had a choice I'd opt for just an object sx which is a superset of |
Closing for now, as it was done simply as an experiment |
This PR is an experiment of how would the API of the
sx
prop look like if we were using a string template - similar to the tailwindcss classes, instead of an object. This is mean to be useable on any component, Box, core components, primitives (#23220)A codesandbox to play with it.
Problem
The reason for trying out this API is mostly because of the following:
"
,{
,}
and,
keys on your keyboard. With the sx string, you don't need them. It's faster to type: https://twitter.com/timolins/status/1295736204105842689. How much faster? I don't know.It resonates with front-end developers and designers that write a lot of styles and don't spend a lot of time maintaining it. It is also closer to the regular className syntax.
The new API
It would basically mean that we can support the following string as an example:
that corresponds with the following object definition
The first concern I had with this API was type-safety, but seems like with the new Typescript's literal type we could solve this. Here is a small POC - Validator for mui sx string - was inspired by this tweet :)
The implementation is quite straightforward:
Limitations
We could support the pseudo-selectors like ":hover", but more on that I am not sure at this moment...
Tradeoff
This is implemented with CSS-in-JS, see the tradeoff in https://next.material-ui.com/system/basics/#performance-tradeoff.
Benchmark