Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
@mnajdova mnajdova requested a review from siriwatknp March 13, 2024 10:20
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.

👍

# for free to join this conversation on GitHub. Already have an account? # 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