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

[core] Fix ESM issues with regression tests #37963

Merged
merged 21 commits into from
Aug 26, 2023
Merged

[core] Fix ESM issues with regression tests #37963

merged 21 commits into from
Aug 26, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jul 14, 2023

Closes #37888

It's an alternative to #37888 (comment) in that it stops babel from compiling modules down to commonjs for the regression tests. The goal will be to do this for all targets (usesEsModules = true).

Included in this PR:

  • Switch the regressions tests babel config to not compile modules
  • update react-imask
  • Fix a few missing aliasing

@Janpot Janpot mentioned this pull request Jul 14, 2023
1 task
@Janpot Janpot added the core Infrastructure work going on behind the scenes label Jul 14, 2023
@mui-bot
Copy link

mui-bot commented Jul 14, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against b7742f6

@Janpot Janpot changed the title Attempt fixing ESM issues with regression tests [core] Attempt fixing ESM issues with regression tests Jul 14, 2023
@Janpot Janpot changed the base branch from renovate/react-imask-7.x to master July 18, 2023 09:24
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 30, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 23, 2023
@Janpot Janpot marked this pull request as ready for review August 23, 2023 15:14
@Janpot Janpot changed the title [core] Attempt fixing ESM issues with regression tests [core] Fix ESM issues with regression tests Aug 23, 2023
@Janpot Janpot requested a review from ZeeshanTamboli August 23, 2023 17:17
test/regressions/webpack.config.js Show resolved Hide resolved
@@ -32,11 +33,12 @@ module.exports = {
rules: [
{
test: /\.(js|ts|tsx)$/,
exclude: /node_modules/,
exclude: /node_modules|prism\.js/,
Copy link
Member

Choose a reason for hiding this comment

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

Are we excluding this so that it picks the prism.mjs (ES Module) file?

Copy link
Member Author

@Janpot Janpot Aug 24, 2023

Choose a reason for hiding this comment

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

It's because @mui/markdown/prism gets imported in several contexts, both ESM and Commonjs. It's a commonjs file but webpack resolves it as ESM and fails to find exports. Ideally we move this to ESM as well, but I don't want to expand the scope of this PR further

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'm adding a comment

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

Awesome work. Looks great! This marks one of the steps in the overall support for ES Modules.

@Janpot Janpot requested a review from michaldudak August 24, 2023 11:47
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

👍

@Janpot Janpot merged commit 8212d3d into mui:master Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants