-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[material-ui] Support CSS Extraction using codemod #41935
Conversation
if (data.key) { | ||
variant.style = j.objectExpression([ | ||
{ | ||
...j.objectProperty(data.key, variant.style), | ||
computed: data.key.type === 'TemplateLiteral', | ||
}, | ||
]); | ||
} |
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.
Found a small bug, see the fix in the test case below.
Netlify deploy previewhttps://deploy-preview-41935--material-ui.netlify.app/ packages/material-ui/material-ui.production.min.js: parsed: +0.24% , gzip: +0.13% Bundle size reportDetails of bundle changes (Toolpad) |
packages/mui-codemod/src/v6.0.0/styled/test-cases/BasicStyled.expected.js
Show resolved
Hide resolved
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.
Found one issue, the rest looks good 👍
}, | ||
{ | ||
props: { | ||
alignItems: 'flex-start', |
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.
Does this component support all css properties? From this particular style, it seems like it.
And I also see that all the items are direct props. This won't work with Pigment components then because the underlying styled
does not collect all the unknown props and spread it on the style
prop.
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.
Does this component support all css properties
No, alignItems
is explicitly a prop for this component.
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.
@mnajdova The order is preserved which leads to changes for the test cases.
Use codemod to transform these components: