Don't pull unused @mui modules by using path imports #168

anthonyalayo · 2023-01-15T00:10:11Z

Problem

Webpack only tree shakes during minification, so this package is pulling in everything from @mui when developing. This results in a very large amount of modules and causes a significant slowdown for development.

This is a known problem, and upon browsing the material-ui github, I could see they are importing the same way.

Attempting to fix this downstream by applying production settings in development doesn't seem to work, it just hangs.
On top of that, the attempted fix #169 doesn't reduce the bundle size either. It transforms the import statements but ends up pulling in other imports from those imports. This solution inlines the @mui dependencies so that the package is minimized in development.

Resources

Solution

Use path imports to only pull in necessary dependencies from @mui.

Tested with a next.js boilerplate using version 13.1.2.

Ran npm link inside @textea/json-viewer
Ran npm link @textea/json-viewer inside the dependent project

Compile time output:

event - compiled client and server successfully in 9.3s (1173 modules)

Next.js module trace output:

Previous size:

event - compiled client and server successfully in 17.2s (1936 modules)

Explored Alternatives

#169

netlify · 2023-01-15T00:10:17Z

❌ Deploy Preview for any-viewer failed.

Name	Link
🔨 Latest commit	`da029e9`
🔍 Latest deploy log	https://app.netlify.com/sites/any-viewer/deploys/63c951caf788bb00089ee0f9

codecov · 2023-01-15T05:25:33Z

Codecov Report

Base: 87.40% // Head: 87.40% // No change to project coverage 👍

Coverage data is based on head (da029e9) compared to base (7884a29).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #168   +/-   ##
=======================================
  Coverage   87.40%   87.40%           
=======================================
  Files          18       18           
  Lines        1969     1969           
  Branches      349      349           
=======================================
  Hits         1721     1721           
  Misses        248      248

Impacted Files	Coverage Δ
src/components/DataKeyPair.tsx	`68.68% <100.00%> (ø)`
src/components/DataTypes/Function.tsx	`100.00% <100.00%> (ø)`
src/components/DataTypes/Object.tsx	`86.84% <100.00%> (ø)`
src/components/DataTypes/createEasyType.tsx	`64.55% <100.00%> (ø)`
src/components/Icons.tsx	`85.10% <100.00%> (ø)`
src/components/mui/DataBox.tsx	`100.00% <100.00%> (ø)`
src/index.tsx	`98.78% <100.00%> (-0.01%)`	⬇️
src/stores/typeRegistry.tsx	`94.73% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

pionxzh · 2023-01-15T05:26:48Z

Actually SWC transform-imports plugin is capable of solving our problem.

This is the configuration I have tested

experimental: {
  plugins: config.browser
    ? []
    : [
        [
          '@swc/plugin-transform-imports',
          {
            '@mui/material': {
              transform: '@mui/material/{{member}}'
            }
          }
        ]
      ]
}

pionxzh · 2023-01-15T05:37:56Z

~~But, few problems here.~~
~~1. We need to external MUI with regex~~
~~2. non-browser build will become CJS/MJS (no UMD as we cannot provide the global for them~~

I just solved them all. Let me make a PR.

anthonyalayo · 2023-01-15T05:42:31Z

@pionxzh exciting! I'll close this one up.

pionxzh · 2023-01-15T15:49:58Z

Thanks for the research and detailed documentation. Let's go for this 🚀

anthonyalayo · 2023-01-16T08:59:47Z

Explored the issue in detail and filed an RFC for @mui here. I'll make the finishing touches on this.

anthonyalayo · 2023-01-16T11:10:00Z

@pionxzh @rtritto I'm seeing some sort of ESM/CJS interop build issues coming from the docs, any idea what could be causing it? The very first default import statement fails:

SyntaxError: Cannot use import statement outside a module

Seems related:

rtritto · 2023-01-16T14:11:25Z

Maybe the error can be resolved converting this project to ESM.
Can this conversion be considered and added to next milestone? @pionxzh

anthonyalayo · 2023-01-17T09:11:03Z

@rtritto @pionxzh created #181 as a first attempt, could you help get it working completely?

pionxzh · 2023-01-17T10:35:51Z

I thought we already provided ESM output 🤔
Never mind, I will check what happened.

pionxzh · 2023-01-17T14:25:43Z

should be good now 👍

anthonyalayo · 2023-01-17T18:04:20Z

Nice! Feel free to merge 😁

anthonyalayo · 2023-01-18T05:06:33Z

@pionxzh can we merge?

pionxzh · 2023-01-18T06:24:07Z

src/index.tsx

-  ThemeProvider
-} from '@mui/material'
+import Paper from '@mui/material/Paper'
+import { createTheme, ThemeProvider } from '@mui/material/styles'


@anthonyalayo is this expected?
maybe I mess it up in the rebasing 🤔

use this instead?

import createTheme from '@mui/material/styles/createTheme' import ThemeProvider from '@mui/material/styles/ThemeProvider'

@pionxzh looks like it's still using cjs for some reason?

10:41:43 PM: /opt/build/repo/node_modules/@mui/material/styles/createTheme.js:1 10:41:43 PM: import _extends from "@babel/runtime/helpers/esm/extends"; 10:41:43 PM: ^^^^^^ 10:41:43 PM: SyntaxError: Cannot use import statement outside a module 10:41:43 PM: at internalCompileFunction (node:internal/vm:74:18) 10:41:43 PM: at wrapSafe (node:internal/modules/cjs/loader:1141:20) 10:41:43 PM: at Module._compile (node:internal/modules/cjs/loader:1182:27) 10:41:43 PM: at Module._extensions..js (node:internal/modules/cjs/loader:1272:10) 10:41:43 PM: at Module.load (node:internal/modules/cjs/loader:1081:32) 10:41:43 PM: at Module._load (node:internal/modules/cjs/loader:922:12) 10:41:43 PM: at Module.require (node:internal/modules/cjs/loader:1105:19) 10:41:43 PM: at require (node:internal/modules/cjs/helpers:103:18) 10:41:43 PM: at 3898 (/opt/build/repo/docs/.next/server/pages/index.js:726:18)

it's quite weird... 🤔 still finding the root cause

pionxzh · 2023-01-18T18:09:57Z

I think this is more like an issue on Nextra? IDK how to fix it ._.
The bundling of that module being skipped (? so Node.js complains about the import statement

anthonyalayo · 2023-01-18T18:12:19Z

@pionxzh yeah I was really confused too, I thought it had to do with something on the rollup side... that's why I made the other PR to attempt to move away from rolling up modules. I have no idea where that problem is coming from

anthonyalayo · 2023-01-19T10:35:36Z

I think I figured out the bug @pionxzh ,

1. Next.js server side currently builds in CJS, not ESM:
vercel/next.js#25423
vercel/next.js#32239
vercel/next.js#24334

You can make it use ESM, but then it follows the Node.js definition, which requires .mjs files:
https://nodejs.org/api/modules.html#enabling

This isn't setup/supported around the entire ecosystem yet. The "module" and "main" fields in package.json were a proposal that ended up being used as a de facto solution for bundlers until the official solution (.mjs) came out.

2. The module resolution strategy:
https://www.typescriptlang.org/docs/handbook/module-resolution.html#node

@mui has the following in its package.json:

  "main": "./node/index.js",
  "module": "./index.js",

but (this is the part I'm not 100% certain on):

when the Next.js build encounters:

module.exports = require("@mui/material/styles/createTheme");

it resolves that without looking at main or module, and attempts to resolve it using that exact path. If that's the case, that path actually points to the esm location in @mui.

Hence:

10:41:43 PM: /opt/build/repo/node_modules/@mui/material/styles/createTheme.js:1
10:41:43 PM: import _extends from "@babel/runtime/helpers/esm/extends";
10:41:43 PM: ^^^^^^
10:41:43 PM: SyntaxError: Cannot use import statement outside a module
10:41:43 PM:     at internalCompileFunction (node:internal/vm:74:18)
10:41:43 PM:     at wrapSafe (node:internal/modules/cjs/loader:1141:20)
10:41:43 PM:     at Module._compile (node:internal/modules/cjs/loader:1182:27)
10:41:43 PM:     at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)
10:41:43 PM:     at Module.load (node:internal/modules/cjs/loader:1081:32)
10:41:43 PM:     at Module._load (node:internal/modules/cjs/loader:922:12)
10:41:43 PM:     at Module.require (node:internal/modules/cjs/loader:1105:19)
10:41:43 PM:     at require (node:internal/modules/cjs/helpers:103:18)
10:41:43 PM:     at 3898 (/opt/build/repo/docs/.next/server/pages/index.js:726:18)

If that make sense to you (and let me know if you can confirm/deny with a reference the part I wasn't 100% sure on), I think the fix would be:

emit the paths we are presently emitting, for the ESM build
emit the paths prefixed with /node for the CJS build

Hopefully you have an idea of how we can do that with rollup?

pionxzh · 2023-01-19T10:53:09Z

Wow. This is helpful and detailed.

If we need to do this kind of path rewriting Maybe it's time to bring back #169 ... 🤣

I will look into it and see how later. Thanks.

anthonyalayo · 2023-01-19T10:59:50Z

Agreed, I was thinking the same, we just need exceptions for those 2-3 odd ones out.

pionxzh · 2023-01-19T15:47:39Z

I think your assumption 1 is correct.
I tried but those workarounds are so weird.
Will see how #169 goes.

anthonyalayo · 2023-01-19T18:44:08Z

Sounds good, closing this one up for that one

anthonyalayo mentioned this pull request Jan 15, 2023

fix: inline icons from @mui/icons-material #147

Merged

pionxzh mentioned this pull request Jan 15, 2023

fix: reduce MUI size by using @swc/plugin-transform-imports #169

Merged

anthonyalayo closed this Jan 15, 2023

anthonyalayo reopened this Jan 15, 2023

pionxzh mentioned this pull request Jan 17, 2023

refactor: use sx rather than styled #182

Merged

pionxzh force-pushed the ref/reduceBundleSizeMui branch from 627b96d to 200d3e9 Compare January 17, 2023 14:06

pionxzh reviewed Jan 18, 2023

View reviewed changes

anthonyalayo and others added 4 commits January 19, 2023 22:20

Don't pull unused @mui modules by using path imports

b144429

Not using default import for createTheme and ThemeProvider

d223935

fix: external all path imported @mui

3acacd7

Update index.tsx

a73c2e3

fix: path import SvgIcon

da029e9

pionxzh force-pushed the ref/reduceBundleSizeMui branch from 67ab1ef to da029e9 Compare January 19, 2023 14:20

anthonyalayo closed this Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't pull unused @mui modules by using path imports #168

Don't pull unused @mui modules by using path imports #168

anthonyalayo commented Jan 15, 2023 •

edited

Loading

netlify bot commented Jan 15, 2023 •

edited

Loading

codecov bot commented Jan 15, 2023 •

edited

Loading

pionxzh commented Jan 15, 2023

pionxzh commented Jan 15, 2023

anthonyalayo commented Jan 15, 2023

pionxzh commented Jan 15, 2023

anthonyalayo commented Jan 16, 2023

anthonyalayo commented Jan 16, 2023

rtritto commented Jan 16, 2023

anthonyalayo commented Jan 17, 2023

pionxzh commented Jan 17, 2023

pionxzh commented Jan 17, 2023

anthonyalayo commented Jan 17, 2023

anthonyalayo commented Jan 18, 2023

pionxzh Jan 18, 2023

anthonyalayo Jan 18, 2023

pionxzh Jan 18, 2023

pionxzh commented Jan 18, 2023

anthonyalayo commented Jan 18, 2023

anthonyalayo commented Jan 19, 2023

pionxzh commented Jan 19, 2023

anthonyalayo commented Jan 19, 2023

pionxzh commented Jan 19, 2023

anthonyalayo commented Jan 19, 2023

Don't pull unused @mui modules by using path imports #168

Don't pull unused @mui modules by using path imports #168

Conversation

anthonyalayo commented Jan 15, 2023 • edited Loading

Problem

Resources

Solution

Explored Alternatives

netlify bot commented Jan 15, 2023 • edited Loading

❌ Deploy Preview for any-viewer failed.

codecov bot commented Jan 15, 2023 • edited Loading

Codecov Report

pionxzh commented Jan 15, 2023

pionxzh commented Jan 15, 2023

anthonyalayo commented Jan 15, 2023

pionxzh commented Jan 15, 2023

anthonyalayo commented Jan 16, 2023

anthonyalayo commented Jan 16, 2023

rtritto commented Jan 16, 2023

anthonyalayo commented Jan 17, 2023

pionxzh commented Jan 17, 2023

pionxzh commented Jan 17, 2023

anthonyalayo commented Jan 17, 2023

anthonyalayo commented Jan 18, 2023

pionxzh Jan 18, 2023

Choose a reason for hiding this comment

anthonyalayo Jan 18, 2023

Choose a reason for hiding this comment

pionxzh Jan 18, 2023

Choose a reason for hiding this comment

pionxzh commented Jan 18, 2023

anthonyalayo commented Jan 18, 2023

anthonyalayo commented Jan 19, 2023

pionxzh commented Jan 19, 2023

anthonyalayo commented Jan 19, 2023

pionxzh commented Jan 19, 2023

anthonyalayo commented Jan 19, 2023

anthonyalayo commented Jan 15, 2023 •

edited

Loading

netlify bot commented Jan 15, 2023 •

edited

Loading

codecov bot commented Jan 15, 2023 •

edited

Loading