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

[docs] Shift aliasing responsibilities from babel to webpack #33199

Closed
wants to merge 14 commits into from

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jun 17, 2022

  • Move aliasing responsibilities from babel to webpack. Leverage the webpack resolver aliasing capabilities. Webpack is more suitable for this task as it provides the modules runtime. It acts as a linker, so it should be in charge of the resolution process.
  • Let next-transpile-modules compile workspace dependencies

This avoids the need to specify the aliasing again for transpiled modules.

In an ideal world we'd avoid aliasing altogether. In Toolpad we avoid them by building and watching each of the workspaces separately.

@Janpot Janpot added docs Improvements or additions to the documentation core Infrastructure work going on behind the scenes labels Jun 17, 2022
@mui-bot
Copy link

mui-bot commented Jun 17, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-33199--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against 99987fa

@Janpot Janpot changed the title [docs] Move aliasing responsibilities from babel to webpack [docs] Shift aliasing responsibilities from babel to webpack Jun 17, 2022
test: /\.(js|mjs|jsx)$/,
resourceQuery: { not: [/raw/] },
include:
/node_modules(\/|\\)(notistack|@mui(\/|\\)x-data-grid|@mui(\/|\\)x-data-grid-pro|@mui(\/|\\)x-license-pro|@mui(\/|\\)x-data-grid-generator|@mui(\/|\\)x-date-pickers-pro|@mui(\/|\\)x-date-pickers)/,
Copy link
Member

@oliviertassinari oliviertassinari Jun 17, 2022

Choose a reason for hiding this comment

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

We will need to test that notistack and x-data-grid are still using the local version of Material UI.

Copy link
Member Author

@Janpot Janpot Jun 17, 2022

Choose a reason for hiding this comment

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

Seems to work for x-data-grid (I left the externals in). Inspected notistack, the version installed doesn't seem to be importing any @mui/ packages anywhere. It added us in peerDependencies for some reason though.

Screen.Recording.2022-06-17.at.23.43.09.mov

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 8, 2022
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Jul 8, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 12, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 1, 2022
@michaldudak
Copy link
Member

What about scripts executed with babel-node? There is a couple of them that import from @mui/ scoped packages. They may need the aliases defined in Babel config.

@Janpot
Copy link
Member Author

Janpot commented Nov 10, 2022

This PR only changes it for the docs. We can phase out babel-node in separate efforts

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 23, 2022
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.

What about scripts executed with babel-node? There is a couple of them that import from @mui/ scoped packages. They may need the aliases defined in Babel config.

Actually, the scripts will just use linked packages from yarn workspaces, so they should work just fine.

'@mui/base': path.resolve(__dirname, '../packages/mui-base/src'),
'@mui/material-next': path.resolve(__dirname, '../packages/mui-material-next/src'),
'@mui/joy': path.resolve(__dirname, '../packages/mui-joy/src'),
docs: path.resolve(__dirname, './'),
Copy link
Member

Choose a reason for hiding this comment

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

docs are likely not necessary to be specified here. Workspace linking should work just fine.

@Janpot
Copy link
Member Author

Janpot commented Dec 1, 2022

@michaldudak FYI this PR may be superseded by #35197

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 7, 2023
@Janpot
Copy link
Member Author

Janpot commented Apr 19, 2024

@Janpot Janpot closed this Apr 19, 2024
@Janpot Janpot deleted the ntm branch April 19, 2024 14:05
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 docs Improvements or additions to the documentation PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants