Errors when dynamically importing components

[mbrowne]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When doing a dynamic import with an expression that can only be evaluated at run-time (e.g. import('@material-ui/core/' + window.buttonComponentName)), there are a bunch of errors, e.g. (just to list one of them):

Error: Can't resolve './props' in '/Users/mbrowne/_temp/react-webpack-error-template/node_modules/@material-ui/core/styles'

Interestingly, in my demo repo, the button actually still renders despite the errors, but you can't do a production build.

Expected Behavior 🤔

Dynamic imports should work without errors

Steps to Reproduce 🕹

Clone the material-ui-dynamic-imports branch of my demo repo:
git clone [email protected]:mbrowne/react-webpack-error-template.git --single-branch --branch material-ui-dynamic-imports.

Run yarn start and you'll see the compile errors (both at the command line and in the browser console).

Context 🔦

A practical use case for doing this would be receiving the name/ID of which component to use for rendering something from an API response, so that the import would really need to be fully dynamic. window.buttonComponentName (if set elsewhere, e.g. in index.html) mimics such a fully dynamic import.

Details

The errors are probably at least partially due to issues with webpack. I tried a very simple test module with a fake Button component (instead of material-ui), and I was still getting warnings and errors as soon as I introduced .d.ts files, even if I removed the typings entry from the package.json of the test module. It seems that fully dynamic imports cause webpack to scan the entire package being imported (at build time) no matter what, including .d.ts files.

So as an experiment, I tried removing all the .d.ts files and many of the errors went away. There were some remaining errors related to test-utils, so I tried deleting test-utils, es/test-utils, and esm/test-utils, and those errors went away too. Afterwards there were only a couple warnings left—"unexpected character" and "unexpected token"s in the markdown files and the LICENSE file, which obviously aren't JS files.

I'm not sure what, if anything, that material-ui can do about all of this, since clearly webpack is doing something odd here, but I wanted to report it so you all are aware.

Your Environment 🌎

Tech	Version
Material-UI	v4.4.0
React	16.9.0
Browser	Chrome 76
Webpack	4.39.3

[eps1lon]

Thank you for opening this issue. The level of detail is greatly appreciated.

Just as a heads up: digging into this will likely take some time since it requires looking into chunking of different bundlers (webpack (4+5), rollup, parcel etc).

The most robust solution currently would be to create intermediate files in a dedicated folder that simply re-export their respective Material-UI counterpart e.g.

// mui-proxies/button.js
export { default } from '@material-ui/core/Button'
// some-app.js
import('./mui-proxies/' + componentName);

That way you don't have to know anything about our internal package structure (and what kind of files we ship) and webpack doesn't create chunks for more than what you actually use.

[mbrowne]

Thanks for the suggestions. Based on your experience, do you think there is indeed a bug in webpack involved here?

[eps1lon]

I don't think so. I think this is still working as expected since it's "symbolic execution" is/was very simple: any string is possible in componentName and therefore it has to prepare chunks for everything under @material-ui/core.

You could maybe improve it with '@material-ui/core/' + componentName + '/' so that it doesn't go too deep but then you still have to deal with test-utils. That's just how dynamic vs. static works in programming.

But I just remembered that the behavior was documented and it seems like they did expand the docs: https://webpack.js.org/api/module-methods/#magic-comments. Maybe you can use webpackInclude to only match component submodules i.e. submodules that start with a capital letter or just exclude test-utils, styles etc.

Again a very interesting topic to explore. Any findings you can share (or even make a docs PR) would be greatly appreciated.

[mbrowne]

Thanks for the tips; I think I get it now. When doing dynamic imports, it seems webpack doesn't know which files are actually JS files—it probably doesn't want to make assumptions just based on the file extension. Adding the magic comments you mentioned seems to work quite well...I just tested it on the Button component and Avatar component. For example:

const Button = loadable(() =>
  import(
    /* webpackInclude: /\.js$/ */
    /* webpackExclude: /\/test-utils\// */
    `@material-ui/core/${window.buttonComponentName}`
  )
)

There could of course be errors with some of the other components that I haven't seen because I haven't tested them. But hopefully, no big changes would be needed to support this for the library as a whole (as long as the current structure with an entry point for each component is maintained), and the main task would be to document it.

[eps1lon]

Without dismissing the use case I would question why you would want to lazy load low-level components. Usually you do this to lazy load routes or more general parts of your view. While lazy loading each individual component is certainly possible it doesn't seem like it would really solve the core issue import() is trying to solve which is reducing bundle size of the initial page load.

May I ask what the actual use case in your app looks like?

[mbrowne]

We are planning to build a CMS that will include a feature for A/B (or multivariate) testing. We will be querying the data from the CMS via GraphQL, and that data will include a uiComponentId field that will allow the CMS to specify which UI component to use to render a particular section of the page. Longer-term we also hope to have a visual page builder tool that will allow staff to create at least some of our simpler pages on their own, and choose which UI components should be used to display each section of the page (so not much different than the A/B testing scenario, just a different reason).

Our investigation of how to accomplish this is far from complete. We will have server-side rendering at the very least, and might also generate a fully static site in advance. So the performance of dynamic imports for the initial render is less of a concern for us (or maybe we should have a condition in the code so that we just use regular imports when running on the server). Our bigger concern is ensuring good performance as users navigate to subsequent pages.

You may be right that dynamically loading each component separately would be inefficient. I was thinking that it's probably unlikely that navigating to a different page would require loading more than 5-10 components that hadn't already been loaded, which seems like it should be fairly efficient with HTTP/2, but I did read that gzip compression doesn't work as well if the chunks are too small. Looking at the documentation of webpack's magic comments that you linked to, I noticed an interesting option for webpackMode called lazy-once:

Generates a single lazy-loadable chunk that can satisfy all calls to import(). The chunk will be fetched on the first call to import(), and subsequent calls to import() will use the same network response. Note that this only makes sense in the case of a partially dynamic statement, e.g. import(./locales/${language}.json), where there are multiple module paths that could potentially be requested.

...so that might be an interesting option for getting somewhat larger chunks, if the components were grouped into subfolders. Of course, in the case of material-ui, the library is already divided into separate packages...but material-ui/core is still fairly large, especially considering that you might only need a few components from it for a given page.

So I don't really know anything for sure at this point... I figured we can try out loading components individually since that's simplest (but still avoids loading the whole package) and do some performance testing and go from there. I suppose we should also compare that against the performance of just loading the whole package up-front regardless.