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.

Sign up for GitHub

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

[code-infra] Remove @mui/material-nextjs dependency #15925

Merged
merged 3 commits into from
Dec 18, 2024
Merged

[code-infra] Remove @mui/material-nextjs dependency #15925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b47e827
[code-infra] Remove `@mui/material-nextjs` dependency
LukasTy Dec 18, 2024
e18ae00
Add 2 additional flaky argos tests
LukasTy Dec 18, 2024
7d03e5e
Merge branch 'master' into remove-material-nextjs-dep
LukasTy Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion babel.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const defaultAlias = {
'@mui/x-tree-view': resolveAliasPath('./packages/x-tree-view/src'),
'@mui/x-tree-view-pro': resolveAliasPath('./packages/x-tree-view-pro/src'),
'@mui/x-internals': resolveAliasPath('./packages/x-internals/src'),
'@mui/material-nextjs': '@mui/monorepo/packages/mui-material-nextjs/src',
Copy link
Member Author

Choose a reason for hiding this comment

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

We had the dependency, but it was not even used as we were aliasing it... 🤷 🤯 🙈

Copy link
Member

Choose a reason for hiding this comment

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

some day we like to remove the alias and just rely on the dependency

Copy link
Member

Choose a reason for hiding this comment

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

I think the point is that it is not even used

Copy link
Member Author

Choose a reason for hiding this comment

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

But do you think there is a need for this dependency to exist?
I can find /~https://github.com/mui/material-ui/blob/1920b80bdf58e6fb1f6bb4394f55fa0a07d0faf0/docs/pages/_document.js#L3 that seems like a downstream usage, but I struggle to understand if anything is impacted by the removal of this dependency. 🤔
A style element with insertion-point-jss id is still inserted in the DOM. 🤷

Copy link
Member

Choose a reason for hiding this comment

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

I think it's one of the implicit dependencies coming from the docs. since the docs dependencies aren't installed automatically but we are using monorepo docs code that uses those dependencies, we need to copy them exactly from the monorepo. hopefully we can move all that usage and dependencies to the @mui/docs package some day so that it happens automatically.

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 merging this "optimization".
Once we further the progress on the @mui/docs migration I hope that the package will handle the necessary dependencies installation. 👍

Copy link
Member

Choose a reason for hiding this comment

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

If it's a dependency of the docs workspace in core, it should probably be one of the docs workspace in X/toolpad/... as well. If it's not broken, then that's only by coincidence.
But I'm not going to die on this hill, it'll break when it wants to break 😄

'@mui-internal/api-docs-builder': resolveAliasPath(
'./node_modules/@mui/monorepo/packages/api-docs-builder',
),
Expand Down
1 change: 0 additions & 1 deletion docs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"@mui/joy": "^5.0.0-beta.48",
"@mui/lab": "^5.0.0-alpha.173",
"@mui/material": "^5.16.11",
"@mui/material-nextjs": "^5.16.8",
"@mui/styles": "^5.16.11",
"@mui/system": "^5.16.8",
"@mui/utils": "^5.16.8",
Expand Down
32 changes: 0 additions & 32 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion renovate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
"@mui/joy",
"@mui/lab",
"@mui/material",
"@mui/material-nextjs",
"@mui/styles",
"@mui/system",
"@mui/types",
Expand Down
1 change: 0 additions & 1 deletion scripts/useMaterialUIv6.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const pnpmUpdate = childProcess.spawnSync(
'@mui/system@6.x',
'@mui/icons-material@6.x',
'@mui/utils@6.x',
'@mui/material-nextjs@6.x',
'@mui/styles@6.x',
'@mui/lab@latest',
],
Expand Down
7 changes: 6 additions & 1 deletion test/regressions/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ function sleep(timeoutMS: number | undefined) {
const isMaterialUIv6 = materialPackageJson.version.startsWith('6.');

// Tests that need a longer timeout.
const timeSensitiveSuites = ['ColumnAutosizingAsync', 'DensitySelectorGrid'];
const timeSensitiveSuites = [
'ColumnAutosizingAsync',
'DensitySelectorGrid',
'DataGridOverlays',
'PopularFeaturesDemo',
Comment on lines +20 to +21
Copy link
Member Author

Choose a reason for hiding this comment

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

These are very flaky on Argos lately, hopefully increasing their timeout helps. 🤞 🙈

];

const isConsoleWarningIgnored = (msg?: string) => {
const isMuiV6Error =
Expand Down
4 changes: 0 additions & 4 deletions webpackBaseConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ module.exports = {
'@mui/x-tree-view-pro': path.resolve(__dirname, './packages/x-tree-view-pro/src'),
'@mui/x-license': path.resolve(__dirname, './packages/x-license/src'),
'@mui/x-internals': path.resolve(__dirname, './packages/x-internals/src'),
'@mui/material-nextjs': path.resolve(
__dirname,
'./node_modules/@mui/monorepo/packages/mui-material-nextjs/src',
),
docs: path.resolve(__dirname, './node_modules/@mui/monorepo/docs'),
docsx: path.resolve(__dirname, './docs'),
},
Expand Down
Loading