material-ui
material-ui copied to clipboard
[docs] Modularize Imports for Nextjs
https://github.com/mui/material-ui/issues/35450
Signed-off-by: Ahmed Hany [email protected]
- [x] I have followed (at least) the PR section of the contributing guide.
Messages | |
---|---|
:book: | Netlify deploy preview: https://deploy-preview-35457--material-ui.netlify.app/ |
Generated by :no_entry_sign: dangerJS against 4ddd6dec915ac85e0471871658f9f2d41daec9ae
Gave this a quick try on Toolpad and this breaks on imports like:
import { Box, styled } from '@mui/material'
As there is no @mui/material/styled
. Perhaps it should only be done for @mui/icons-material
?
@Janpot
but @mui/material
is also at the docs
'babel-plugin-direct-import',
{ modules: ['@mui/material', '@mui/icons-material'] },
Could you add it as Option 3, after a Babel plugin?
Good catch, @Janpot. I see that the Babel plugins also fail to import styled
from @mui/material. I suppose your suggestion to apply this only to @mui/icons-material makes the most sense. CC @mui/core
I think that it would be great to update the /examples
so that developers get this optimization by default.
@dev-ahmedhany, to sum up - if you'd like to continue with this PR, please remove the instructions for @mui/material
imports (only leave @mui/icons-material
). Also, remove the experimental
field and update the Next version to 13.1.
@michaldudak done
Thanks. Please merge in the latest master, this should resolve the failing checks. Also, as @oliviertassinari pointed out, it would be great to update the examples to use this feature (/examples/nextjs and /examples/nextjs-with-typescript).
How about @mui/x-data-grid example ? hit some error with the config below:
modularizeImports: {
'@mui/x-data-grid': {
transform: '@mui/x-data-grid/{{member}}',
},
'@mui/x-date-pickers': {
transform: '@mui/x-date-pickers/{{member}}',
},
},
How about @mui/x-data-grid example
@zigang93 These are already one npm package per "component" (not in the React definition, from the perspective of end-users), there isn't as much value to modularizing imports as with icons.
- https://bundlephobia.com/package/@mui/[email protected] the main component is 70% of the bundle size. I think that it makes more sense to look into tree-shaking. Right now, modularizing imports makes little sense. cc @cherniavskii
- https://bundlephobia.com/package/@mui/[email protected] has more tree-shaking opportunities, and we took advantage of it to support deep imports. If you have there specific errors with the date pickers, please open an issue on https://github.com/mui/mui-x. cc @flaviendelangle
In any case, we also need to modularize all the internal imports: https://github.com/mui/material-ui/issues/35840.
bundlephobia.com/package/@mui/[email protected] the main component is 70% of the bundle size. I think that it makes more sense to look into tree-shaking. Right now, modularizing imports makes little sense.
@oliviertassinari Yeah, I think most of the data grid's code is imported anyway, there are only a few components/hooks that are optional (like GridToolbar
).
Regarding the tree-shaking, do you have any specific issues in mind?
Regarding the tree-shaking, do you have any specific issues in mind?
@cherniavskii I'm thinking of https://github.com/mui/mui-x/issues/7358, and https://github.com/mui/material-ui/issues/35840 for the data grid.
I wonder what we should do with this PR considering https://github.com/vercel/next.js/pull/50900.
If I understand it correctly, this is applied by default by Next.js and doesn't require additional configuration. I believe we can close this PR.
I reverted one of those: https://github.com/vercel/next.js/pull/51953
This PR is related only to icons-material, so the code you reverted doesn't affect it, does it?
This PR is related only to icons-material, so the code you reverted doesn't affect it, does it?
@michaldudak Agree. I also agree that we can close this PR.
However, the opportunity I see is we could open a new issue so we can scale this to all the MUI npm packages, beyond @mui/icons-material
. It could be done with special rules, like in https://github.com/vercel/next.js/blob/261db496f7cf04b70f078a6eae0c8baf6fe5c238/packages/next/src/server/config.ts#L738. I didn't have time in https://github.com/vercel/next.js/pull/51953 to do it right, there was an urgency to fix it before it was rolled from their canary release to stable. Then, the question is is the Next.js job to keep this up to date? Is there a similar opportunity with Vite, should we update the Vite example as well? At the end of the day, it's about improving the DX.