Named imports scans entire directory

[mbrevda]

In some packages with a large number of named imports, every listed import is scanned for even when just a single import is imported, causing a noticeable slowdown in build time. This is particularly apparent with constant rebuilds during development.

Here is a reproducible repo, showing an large time difference

[evanw]

Just took a look. The module graph for defaultImport.mjs has 353 modules while the module graph namedImport.mjs has 7349 modules. That's over 20x more modules, so it's expected that it would be slower. This is just part of how the JavaScript language works. Importing more files always takes more time because it involves doing more work. The primary entry point of the mdi-material-ui package is particularly inefficient with 6996 imports. With the way JavaScript is specified, it's not valid for esbuild to skip traversing into unused imports when instantiating a module, so there's nothing esbuild can do to make the slower one as fast as the faster one.

If you want optimal performance here, I recommend importing these icons into your project individually instead of importing every single icon into your project at once.

If this package author wanted optimal performance here, I recommend that they inline all of these icon strings into the primary entry point. I tried changing the package to do that and it was a huge speedup for bundling time. On my machine, importing all icons as separate files was 16x slower than importing just one icon, but with inlined strings importing all icons is only 2x slower than importing just one icon (one icon was 15ms, all icons as inlined strings was 31ms, all icons as separate files was 236ms). Putting all strings into one file shouldn't cause bigger bundle sizes as long as ESM is used and the createIcon calls are annotated with /* @__PURE__ */ comments. Instead, doing this will actually likely lead to smaller bundle sizes and also be more efficient at run-time because switching to pure ESM avoids the overhead of potentially many thousands of CommonJS wrappers (one for each icon module).

[mbrevda]

That's over 20x more modules, so it's expected that it would be slower.

Understood. I guess the question here is: is it necessary for the compiler to resolve every single named export when only a single import was requested? Wouldn't it be more efficient to resolve only imported exports?

[hyrious]

is it necessary for the compiler to resolve every single named export when only a single import was requested?

It is, as Evan said it's not valid for esbuild to skip traversing into unused imports when instantiating a module. Because those imports may introduce some global side effects (like window.xxx = 1) that must be evaluated instead of being ignored.

[leMaik]

Hi, mdi-material-ui maintainer here. 👋
I understand that making the imports more efficient is important, but I do feel like this is a bundler issue and not a library issue (we're just using es features after all). As was previously discussed, this is impossible due to the way modules are specified.

However: Webpack takes a "sideEffects": false flag in the package.json file into account to know that modules won't have such global side effects (like @heypiotr mentioned). Couldn't esbuild recognize this and skip those modules?

If that's not possible then we'd have to solve it in library-land. We could change it to one big file that exports all the icons, but that would break existing apps where the icons are imported from the icon file (import Coffee from 'mdi-material-ui/Coffee). If we do both approaches, that will lead to icons being bundled twice.

[mbrevda]

reopening for discussion

[mbrevda]

@evanw is adding /* @__PURE__ */ to all the exports enough? @leMaik is this simple to do in webpack?

[leMaik]

@mbrevda I'm generating the index.js using a script, so I can add that annotation easily.

[evanw]

Hi, mdi-material-ui maintainer here. 👋
I understand that making the imports more efficient is important, but I do feel like this is a bundler issue and not a library issue (we're just using es features after all).

Keep in mind that your package is currently working correctly! You may not need to change your package. This issue is only about bundling performance, which you may be willing to trade off vs. other concerns such as compatibility across different tools.

However: Webpack takes a "sideEffects": false flag in the package.json file into account to know that modules won't have such global side effects (like @heypiotr mentioned). Couldn't esbuild recognize this and skip those modules?

This is something esbuild recognizes. It means that esbuild can omit these modules from the output if they are unused. However, JavaScript is still specified such that the whole module graph must be traversed during module instantiation, so it doesn't mean that esbuild can avoid parsing them. If there is a parse error or module resolution error, the build is still supposed to fail.

If that's not possible then we'd have to solve it in library-land. We could change it to one big file that exports all the icons, but that would break existing apps where the icons are imported from the icon file (import Coffee from 'mdi-material-ui/Coffee). If we do both approaches, that will lead to icons being bundled twice.

Not necessarily. You could just have the individual files re-export from the main file. For optimal results you'd want to use ESM instead of CommonJS, but that might mean introducing issues with old node versions that expect CommonJS files, so that could be one reason you'd want to keep your current approach.

Anyway I tried this out myself because I was curious. The results are here: https://github.com/evanw/mdi-material-ui-inline. The TLDR is that doing this makes importing from an individual file slower but importing from the entry point faster. So it's a trade-off and isn't necessarily a clear win, especially if people aren't supposed to be importing from the primary entry point. My approach also makes bundle sizes smaller across all bundlers, although I didn't investigate exactly why this is.

Here are the bundling speeds (more details here) from the current approach compared with the bundling speeds from this suggestion (ESM only, icons in entry point, individual files just re-export entry point):

	esbuild	rollup	parcel	webpack
Import from individual file	Same	1.2s slower	2.2s slower	4.3s slower
Import from entry point	1.2s faster	24.0s faster	2.9s faster	7.9s faster

[leMaik]

@evanw Thanks a lot for looking into this! 👍 I'm not really sure how people use the imports but at least me and my colleagues always use the imports from the primary entry point because it's pretty convenient – and with bundlers being smart about tree shaking that's not an issue anymore anyways (it was back in Material-UI 0.x times).

I'll probably go with the inline approach in the next updates then.

[mbrevda]

@evanw thanks so much for jumping in here! Do you think it would make sense for esbuild to warn in such situations (where it might be doing more work than necessary due to how the package is published)?

[evanw]

@evanw thanks so much for jumping in here! Do you think it would make sense for esbuild to warn in such situations (where it might be doing more work than necessary due to how the package is published)?

No, I don’t think so. Warnings are mainly supposed to call attention to things that are important for correctness and/or that are directly actionable. This situation is not about correctness (it’s about performance) and isn’t something that the user will be able to fix themselves (because it’s part of a package).

[mbrevda]

While perhaps not the compilers job per se, it would be useful if the compiler could let the user know that there is room for improvement, even if it's just a speed up.

Thanks a ton for the feedback!

Closing this as there doesn't seem to be any further action to take.