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

[Slider][material-ui] Convert to support CSS extraction #41201

Merged
merged 16 commits into from
Mar 13, 2024

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Feb 20, 2024

@mnajdova mnajdova added component: slider This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Feb 20, 2024
@mnajdova mnajdova force-pushed the slider/convert-to-zero-runtime branch from ed00a4e to a28f3f0 Compare February 20, 2024 14:34
@mui-bot
Copy link

mui-bot commented Feb 20, 2024

Netlify deploy preview

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

@material-ui/core: parsed: +0.19% , gzip: +0.14%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 739f777

@mnajdova mnajdova added the on hold There is a blocker, we need to wait label Feb 21, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 1, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 8, 2024
export const rootShouldForwardProp = (prop) => shouldForwardProp(prop) && prop !== 'classes';

export const slotShouldForwardProp = shouldForwardProp;
export { default as slotShouldForwardProp } from './slotShouldForwardProp';
Copy link
Member Author

@mnajdova mnajdova Mar 8, 2024

Choose a reason for hiding this comment

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

I had to extract these to separate files, otherwise we were ending up with importing from the styled.js, which resulted with the error:

/Users/marijanajdova/workspace/mui/apps/node_modules/.pnpm/@wyw-in-js+transform@0.5.0_typescript@5.3.3/node_modules/@wyw-in-js/transform/lib/module.js:223
      throw new EvalError(`${e.message} in${this.callstack.join('\n| ')}\n`);
      ^

EvalError: _systemDefaultTheme is not defined in/Users/marijanajdova/workspace/mui/packages/mui-system/build/createStyled.js
| /Users/marijanajdova/workspace/mui/packages/mui-material/build/styles/slotShouldForwardProp.js
| /Users/marijanajdova/workspace/mui/packages/mui-material/build/Slider/Slider.js

Once the PR is merged, I will update all imports we have in Material UI, so that we don't have to add additional check in the migration steps.

cc @brijeshb42, @siriwatknp

Copy link
Member

Choose a reason for hiding this comment

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

@mnajdova mnajdova marked this pull request as ready for review March 8, 2024 11:57
@mnajdova mnajdova removed the on hold There is a blocker, we need to wait label Mar 8, 2024
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: slider This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material package: pigment-css Specific to @pigment-css/*
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants