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

[zero] Set up Material UI migration demos #41267

Merged
merged 16 commits into from Feb 27, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Feb 25, 2024

Goal

This PR sets up the structure of internal nextjs and vite apps to test the Material UI components while converting them to support Zero Runtime.

Changes

  • Added a utility script to generate a page for both nextjs and vite apps. The page will use existing demos that show in the docs. (The usage of the script will be added to an umbrella issue for people who want to contribute to the conversion)
  • Added file system routing to vite app for a consistent pattern with the nextjs app.
  • Started the effort using Avatar as the first conversion.

Test

  1. pnpm install
  2. pnpm build - build mui-material and zero-* packages
  3. Run the apps pnpm --filter @app/vite-app run dev and pnpm --filter @app/next-app run dev in separate terminals

Run the script

node scripts/zero-render-mui-demos.mjs react-<component>

The component comes from the valid path of https://mui/material-ui/react-<component>, e.g. react-avatar, react-chip, etc.


@mui-bot
Copy link

mui-bot commented Feb 25, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against e83898e

@@ -97,6 +116,7 @@ const zeroPluginOptions = {
transformLibraries: ['local-ui-lib'],
sourceMap: true,
displayName: true,
transformSx: false,
Copy link
Member Author

Choose a reason for hiding this comment

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

sx support is not fully working. Need to disable it to be able to render demos.

@@ -0,0 +1,86 @@
'use client';
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,13 @@
// theme for MUI System (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.

A lot of demos are using Stack component by targeting margin of its children. The converted component styles are extracted so margin can not be overridden by Stack. We can't change the demos, so need to use ThemeProvider to change to gap instead.

Comment on lines +19 to +20
"react-router": "^6.22.1",
"react-router-dom": "^6.22.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

Required by vite-plugin-pages

@@ -29,6 +32,7 @@
"@vitejs/plugin-react": "^4.2.1",
"postcss": "^8.4.35",
"postcss-combine-media-query": "^1.0.1",
"vite": "5.0.12"
"vite": "5.0.12",
"vite-plugin-pages": "^0.32.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

To enable file system routing in vite.

@@ -1,7 +1,8 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"jsx": "react-jsx"
"jsx": "react-jsx",
"allowImportingTsExtensions": true
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to explicitly do import Demos from '../path/to/Demo.tsx to use tsx file because .js is not out of the box supported in Vite.

package.json Outdated
@@ -112,6 +112,7 @@
"@mui/joy": "workspace:*",
"@mui/material": "workspace:^",
"@mui/utils": "workspace:^",
"@mui/zero-runtime": "workspace:^",
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to add @mui/zero-runtime at the root project, so that@mui/material works inside nextjs app.

@siriwatknp siriwatknp marked this pull request as ready for review February 26, 2024 04:00
@@ -114,5 +115,5 @@ export function zeroVitePlugin(options: ZeroVitePluginOptions) {
...rest,
});

return [injectMUITokensPlugin(), intermediateBabelPlugin(), zeroPlugin];
return [injectMUITokensPlugin(), transformSx ? intermediateBabelPlugin() : null, zeroPlugin];
Copy link
Member Author

Choose a reason for hiding this comment

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

Vite plugin was missing transformSx config.

@mnajdova
Copy link
Member

The usage of the script will be added to an umbrella issue for people who want to contribute to the conversion

Please add the instructions on how to run the script in the PR description too, so we can test it with a few components.

@mnajdova
Copy link
Member

Did you check the sx prop? It doesn't seem to work on the Letters and Sizes avatar examples.

@siriwatknp
Copy link
Member Author

Did you check the sx prop? It doesn't seem to work on the Letters and Sizes avatar examples.

Yes, sx prop is not working properly, need to fix it from the Zero side. It's out-of-scope of this PR, that's why I set transformSx to false.

const filenames = await fse.readdir(
path.join(process.cwd(), `docs/data/material/components/${dataFolderName}`),
);
const tsFiles = filenames.filter((filename) => filename.endsWith('.tsx'));
Copy link
Member

Choose a reason for hiding this comment

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

Nit-pick, for some demos we have only js version, but we can simplify and depend on the TSX demos for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: .js demo won't work with Vite with the current configuration.

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 looks good, I pushed some small improvements on the script. I tried it with the badge component, it works as expected.

@brijeshb42
Copy link
Contributor

I think it'd be best if there is a way to test with the built files in @mui/material as that's what the users will end up using. It can be a simple vite plugin that replaces the imported paths with the ones from the build folder.

@zannager zannager added the package: pigment-css Specific to @pigment-css/* label Feb 26, 2024
@siriwatknp
Copy link
Member Author

I think it'd be best if there is a way to test with the built files in @mui/material as that's what the users will end up using. It can be a simple vite plugin that replaces the imported paths with the ones from the build folder.

Got your point, the current setup uses workspace which is not the build folder, is that what you meant?

@brijeshb42
Copy link
Contributor

brijeshb42 commented Feb 26, 2024

Got your point, the current setup uses workspace which is not the build folder, is that what you meant?

Correct. If not in dev, at least in the build step it should consider the build folder so that we can verify if everything is working as expected.

@siriwatknp
Copy link
Member Author

siriwatknp commented Feb 27, 2024

@brijeshb42 I think I got it working in b81f297

This is the files pass through wyw-in-js which looks the same I got in intermediate-repo (it contains emotion).

Avatar.js
Badge.js
Box.js
Box.jsx
ClassNameConfigurator.js
ClassNameGenerator.js
Container.js
GlobalStyles.js
Grid.js
GridProps.js
Person.js
PolymorphicComponent.js
ReactPropTypesSecret.js
Slider.js
SliderValueLabel.js
Stack.js
StackProps.js
StyledEngineProvider.js
SvgIcon.js
ThemeProvider.js
ZeroSlider.tsx
appendOwnerState.js
applyStyles.js
areArraysEqual.js
arrayLikeToArray.js
arrayWithHoles.js
arrayWithoutHoles.js
badgeClasses.js
blue.js
borders.js
boxClasses.js
breakpoints.js
capitalize.js
chainPropTypes.js
clamp.js
clsx.js
colorManipulator.js
common.js
compose.js
composeClasses.js
containerClasses.js
createBox.js
createBreakpoints.js
createChainedFunction.js
createContainer.js
createCssVarsProvider.js
createCssVarsTheme.js
createGetCssVar.js
createGrid.js
createMixins.js
createPalette.js
createSpacing.js
createStack.js
createStyled.js
createSvgIcon.js
createTheme.js
createTransitions.js
createTypography.js
cssGrid.js
cssVarsParser.js
debounce.js
deepmerge.js
defaultSxConfig.js
defaultTheme.js
defineProperty.js
deprecatedPropType.js
display.js
emotion-cache.cjs.js
emotion-cache.cjs.prod.js
emotion-element-4300ad44.cjs.prod.js
emotion-hash.cjs.js
emotion-hash.cjs.prod.js
emotion-is-prop-valid.cjs.js
emotion-is-prop-valid.cjs.prod.js
emotion-memoize.cjs.js
emotion-memoize.cjs.prod.js
emotion-react-_isolated-hnrs.cjs.prod.js
emotion-react.cjs.js
emotion-react.cjs.prod.js
emotion-serialize.cjs.js
emotion-serialize.cjs.prod.js
emotion-sheet.cjs.js
emotion-sheet.cjs.prod.js
emotion-styled-base.cjs.prod.js
emotion-styled.cjs.js
emotion-styled.cjs.prod.js
emotion-unitless.cjs.js
emotion-unitless.cjs.prod.js
emotion-use-insertion-effect-with-fallbacks.cjs.js
emotion-use-insertion-effect-with-fallbacks.cjs.prod.js
emotion-utils.cjs.js
emotion-utils.cjs.prod.js
emotion-weak-memoize.cjs.js
emotion-weak-memoize.cjs.prod.js
exactProp.js
extendSxProp.js
extends.js
extractEventHandlers.js
factoryWithThrowingShims.js
flexbox.js
formatMuiErrorMessage.js
generateUtilityClass.js
generateUtilityClasses.js
getDisplayName.js
getInitColorSchemeScript.js
getThemeProps.js
getThemeValue.js
green.js
grey.js
gridClasses.js
hoist-non-react-statics.cjs.js
identifier.js
index.js
interopRequireDefault.js
isHostComponent.js
isMuiElement.js
iterableToArray.js
iterableToArrayLimit.js
jsx-runtime.js
layout.tsx
lightBlue.js
memoize.js
merge.js
mergeSlotProps.js
nonIterableRest.js
nonIterableSpread.js
objectWithoutPropertiesLoose.js
omitEventHandlers.js
orange.js
ownerDocument.js
ownerWindow.js
page.tsx
palette.js
positions.js
prepareCssVars.js
prepareForSlot.js
purple.js
react-is.production.min.js
react-jsx-runtime.production.min.js
react.production.min.js
red.js
requirePropFactory.js
resolveComponentProps.js
resolveProps.js
responsivePropType.js
setRef.js
shadows.js
shape.js
shouldSpreadAdditionalProps.js
sizing.js
slicedToArray.js
sliderClasses.js
spacing.js
stackClasses.js
style.js
styleFunctionSx.js
styled.js
stylis.js
svgIconClasses.js
toConsumableArray.js
toPrimitive.js
toPropertyKey.js
traverseBreakpoints.js
typeof.js
types.js
typography.js
unsupportedIterableToArray.js
unsupportedProp.js
useControlled.js
useEnhancedEffect.js
useEventCallback.js
useForkRef.js
useId.js
useIsFocusVisible.js
useMediaQuery.js
useRootElementName.js
useSlider.js
useSlider.types.js
useSlotProps.js
useTheme.js
useThemeProps.js
useThemeWithoutDefault.js
zIndex.js

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.

I hope the changes in the packages to use file paths is just temporary. Rest all looks good.

@siriwatknp
Copy link
Member Author

I hope the changes in the packages to use file paths is just temporary. Rest all looks good.

Okay, I'll go with this until we have a better solution for both dev and production environment.

@siriwatknp siriwatknp merged commit 49960f8 into mui:master Feb 27, 2024
19 checks passed
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 8, 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.

None yet

5 participants