Skip to content

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

Closed
Janpot wants to merge 14 commits into
mui:masterfrom
Janpot:ntm
Closed

Janpot wants to merge 14 commits into
mui:masterfrom
Janpot:ntm

Conversation

@Janpot

@Janpot Janpot commented Jun 17, 2022 •

Copy link
Copy Markdown
Member
  • 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. internal Behind-the-scenes enhancement. Formerly called “core”. labels Jun 17, 2022
@mui-bot

mui-bot commented Jun 17, 2022 •

Copy link
Copy Markdown
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
Comment thread docs/next.config.js
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)/,

@oliviertassinari oliviertassinari Jun 17, 2022 •

Copy link
Copy Markdown
Member

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.

@Janpot Janpot Jun 17, 2022 •

Copy link
Copy Markdown
Member Author

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
Copy Markdown
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

Janpot commented Nov 10, 2022 •

Copy link
Copy Markdown
Member Author

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

@michaldudak michaldudak left a comment

Copy link
Copy Markdown
Member

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.

Comment thread docs/next.config.js
'@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
Copy Markdown
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

Janpot commented Dec 1, 2022

Copy link
Copy Markdown
Member Author

@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

Janpot commented Apr 19, 2024

Copy link
Copy Markdown
Member Author

@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

docs Improvements or additions to the documentation. internal Behind-the-scenes enhancement. Formerly called “core”. 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