Skip to content
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

[Grid] Migrate to emotion #24395

Merged
merged 24 commits into from
Jan 15, 2021
Merged

Conversation

mnajdova
Copy link
Member

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 13, 2021

@material-ui/core: parsed: +0.33% , gzip: +0.34%
@material-ui/lab: parsed: +0.33% , gzip: +0.34%

Details of bundle changes

Generated by 🚫 dangerJS against eb4c9e1

.key as babel.types.Identifier).name;

if(!className) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We need this change in order to support classess that are in kebab-case like grid-xs-auto for example

@oliviertassinari oliviertassinari added the component: Grid The React component. label Jan 13, 2021
@mnajdova mnajdova requested review from oliviertassinari and removed request for mbrookes and oliviertassinari January 14, 2021 06:12
@@ -12,14 +12,28 @@ export default function requirePropFactory(componentNameInError) {
) => {
const propFullNameSafe = propFullName || propName;

// eslint-disable-next-line react/forbid-foreign-prop-types
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've got this failing test now https://app.circleci.com/pipelines/github/mui-org/material-ui/35659/workflows/8454c729-bc8f-49ba-8ae5-9fde972ca739/jobs/215603

Also, not sure how smart it is to disable this rule - https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-foreign-prop-types.md

I will revert the changes related to this and open a separte PR on this tomorrow.

Copy link
Member

@oliviertassinari oliviertassinari Jan 14, 2021

Choose a reason for hiding this comment

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

Also, not sure how smart it is to disable this rule - https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-foreign-prop-types.md

The eslint rule docs links https://github.com/oliviertassinari/babel-plugin-transform-react-remove-prop-types to the why. It's fine in our case. The block is wrapped with the process.env.NODE_ENV === 'production' condition.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, anyway will fix it separately to keep this PR clean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Grid The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants