Open mnajdova opened 4 years ago
There’s no need for a pragma, you can release a Babel macro in a similar fashion to Styled Components’.
How would Pragma work with the new automatic runtime? Has that been considered?
Looking forward to this feature!
As @yuchi mentioned, the Babel macro also seems to be a possible alternative.
Some reference I found for the implementation of the css
prop in styled-components:
https://medium.com/styled-components/css-prop-support-for-create-react-app-37e8c5d96861
https://styled-components.com/docs/tooling#babel-macro
Please note that Styled's Babel Macro is a "bad macro", what I mean is that it's actually a Babel Plugin which is triggered file-wide at macro import (they release both the plug-in and the macro).
In this regard it is not an hygienic macro, it doesn't have a trigger reference: every occurrence of a css
prop (by name! not by hygienic reference!) gets transpired.
There were also discussions about changing the prefered API from:
<Box component="footer" sx={{ display: "block" }}>
My footer
</Box>
to
<x.footer sx={{ display: "block" }}>
My footer
</x.footer>
inspired by https://xstyled.dev/. This is superior when you have long children. The closing tag is more helpful in the latter option (</x.footer>
is more helpful than </Box>
).
See https://github.com/mui-org/material-ui/pull/27207#issuecomment-880731270 for why it could be valuable for developers.
@oliviertassinari @mnajdova I would like to push this forward in the upcoming v5-beta. I think that this could make migration to v5 a lot easier. Take this as an example from notistack repo
// This is what normal v4 looks like
const styles = (theme: Theme) => createStyles({
root: {
// styles
}
});
const SnackbarContent = forwardRef<HTMLDivElement, Props>(({ classes, className, ...props }, ref) => (
<div ref={ref} className={clsx(classes.root, className)} {...props} />
))
export default withStyles(styles)(SnackbarContent);
import { styled } from '@material-ui/core/styles';
const Div = styled('div')(({ theme }) => ({
// styles
}))
const SnackbarContent = forwardRef<HTMLDivElement, Props>(({ classes, className, ...props }, ref) => (
<Div ref={ref} className={clsx(classes.root, className)} {...props} />
))
export default SnackbarContent;
the issue with this approach is it make the structure of the project completely change, the component (Div
) is created and usually there are more than 1 div which will be hard to migrate.
// just an example of importing primitives
import { mui } from '@material-ui/core/primitives';
// remain the same
const styles = (theme: Theme) => createStyles({
root: {
// styles
}
});
const SnackbarContent = forwardRef<HTMLDivElement, Props>(({ classes, className, ...props }, ref) => (
<mui.div
ref={ref}
className={clsx(classes.root, className)}
sx={theme => styles(theme).root}
{...props}
/>
))
export default SnackbarContent;
The styles should remain the same and the html tag is replaced with primitives which make the code looks easier to review.
@mnajdova Hi, do you know if supporting sx
on any html element is still being considered? Thanks :)
Hey @robphoenix we are not actively working on this at this moment. I believe we can kick off this effort again when we are closer to v6.
Ok, grand, thanks for the update @mnajdova 🙇🏻
@oliviertassinari is sx
now supported on html elements? It looks like https://github.com/mui/mui-x/pull/9831 doesn't actually implement this? Should this issue be closed?
Sorry, my bad.
@mui/pigment-css It looks like there is movement on this: https://twitter.com/PigmentCSS/status/1784828168748011578.
Can we systematically link PRs with issues? I can't find where this is happening, thanks. Edit: same as https://github.com/mui/pigment-css/issues/35?
Summary 💡
It would be great if we can support the
sx
props on the primitives with jsx pragma. That would basically help us to replacewith
For instance https://xstyled.dev/docs/emotion/#use-css-prop or https://theme-ui.com/sx-prop. It seems that we could only support it with emotion, which is OK-ish as it's the default engine 🙃.
Motivation 🔦
This would help us to move away from templates that would use the
Box
component everywhere. On the other hand, we plan to support thesx
prop on all core components, so it may not be that bad, but still, I think this is something that will bring us in the right direction.Raised in #23053