Skip to content

Conversation

@DiegoAndai
Copy link
Member

@DiegoAndai DiegoAndai commented Dec 19, 2024

After #44720, we forgot to keep using React 18 for the UMD builds. React 19 removed their UMD builds, so we need to use React 18 for it.

To fix this, this PR:

  • Adds a react18 dependency to the mui-material package, which is an alias for react@^18.3.1
  • In the rollup config, resolve jsx/runtime to the react18 dependency

To make this work, I had to update:

  • rollup-plugin-commonjs (deprecated) to @rollup/plugin-commonjs
  • rollup-plugin-node-resolve (deprecated) to @rollup/plugin-node-resolve

These are the updated packages that are recommended. Without this update, the fix still works, but with the updated packages, replacing jsx/runtime is much, much cleaner.

This also means that namedExports is no longer required, as this has been fixed in @rollup/plugin-commonjs. I smoke tested with this PR's build and three components:

Smoke test: index.html.zip

This PR also fixes the UMD build test.

@DiegoAndai DiegoAndai added the internal Behind-the-scenes enhancement. Formerly called “core”. label Dec 19, 2024
@DiegoAndai DiegoAndai requested a review from Janpot December 19, 2024 18:48
@DiegoAndai DiegoAndai self-assigned this Dec 19, 2024
@DiegoAndai DiegoAndai changed the title [core] Use React 18 for v5.x UMD build [core] Use React 18 for v5.x UMD builds Dec 19, 2024
@DiegoAndai
Copy link
Member Author

DiegoAndai commented Dec 19, 2024

@Janpot do you think this approach might have some unwanted side effects? I tried to find a way to add a resolution through the rollup configuration but couldn't find any way to do it.

@mui-bot
Copy link

mui-bot commented Dec 19, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against d8244ba

Copy link
Member

@aarongarciah aarongarciah left a comment

Choose a reason for hiding this comment

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

Looks good. We can update it later if @Janpot proposes a better solution.

@DiegoAndai DiegoAndai marked this pull request as draft December 23, 2024 18:58
@DiegoAndai DiegoAndai marked this pull request as ready for review December 23, 2024 23:16
@DiegoAndai
Copy link
Member Author

@aarongarciah I reworked this as @Janpot and I didn't want to commit to the previous approach 😅

I updated the description with the new approach, which is much cleaner. I also attached an index.html file that uses this PR's UMD build to test it.

@DiegoAndai DiegoAndai changed the title [core] Use React 18 for v5.x UMD builds [core] Use React 18's JSX runtime for v5.x UMD builds Dec 23, 2024
Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

looks great. small suggestion to avoid reaching in the node_modules folder directly

@DiegoAndai DiegoAndai merged commit a37b3d4 into mui:v5.x Dec 24, 2024
21 checks passed
@DiegoAndai DiegoAndai deleted the fix-umd-build-react branch December 24, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Behind-the-scenes enhancement. Formerly called “core”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants