Skip to content

[pigment-css] Fix props forwarding#41688

Merged
siriwatknp merged 15 commits intomui:nextfrom
siriwatknp:pigment/styled-forward-prop
Mar 28, 2024
Merged

[pigment-css] Fix props forwarding#41688
siriwatknp merged 15 commits intomui:nextfrom
siriwatknp:pigment/styled-forward-prop

Conversation

@siriwatknp
Copy link
Member

To unblock #41378

  • filter props for HTML tag using @emotion/is-prop-valid
  • add a default shouldForwardProp for root and other slots

@siriwatknp siriwatknp added the package: pigment-css Specific to Pigment CSS. label Mar 28, 2024
@siriwatknp siriwatknp requested a review from brijeshb42 March 28, 2024 06:48
);
const newProps = {};
// eslint-disable-next-line no-restricted-syntax
for (const key in props) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is comparatively faster than Object.keys since optimizing this styled call is the most important part as it'll be called a lot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, it's the same logic as emotion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styled-component also do the same.

Comment on lines +33 to +35
loader: {
'.js': 'jsx',
},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this after switching styled.jsx to styled.js to make the build work. I can't run the test with .jsx and there is no .jsx found the codebase.

@mui-bot
Copy link

mui-bot commented Mar 28, 2024

Netlify deploy preview

https://deploy-preview-41688--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against ff21a5b

Copy link
Contributor

@brijeshb42 brijeshb42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good. There is a bundle size increase significantly but it should be fine given the amount of functionality it is adding.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to see this one handled 👌

@siriwatknp siriwatknp merged commit e9db5b8 into mui:next Mar 28, 2024
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 29, 2024
tejasparkar pushed a commit to tejasparkar/material-ui that referenced this pull request Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: pigment-css Specific to Pigment CSS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants