gregberge / loadable-components

The recommended Code Splitting library for React ✂️✨
https://loadable-components.com
MIT License
7.68k stars 380 forks source link

`loadable.lib` Webpack 5 dynamic import/chunk name bug #750

Closed wvanrensselaer closed 3 years ago

wvanrensselaer commented 3 years ago

🐛 Bug Report

Using a dynamic import with loadable.lib and Webpack 5 leads to ChunkExtractor error in SSR. Can reproduce in this repo.

Relevant code in Application.js:

const Translations = loadable.lib((props) =>
  import(`./translations.${props.locale}.json`)
);

Chunk names in loadable-stats.json come out as:

{
  "assetsByChunkName": {
    "main": [
      "main.fad06d2e.bundle.js"
    ],
    "translations-translations-de-DE-json": [
      "translations-translations-de-DE-json.169270e6.bundle.js"
    ],
    "translations-translations-en-US-json": [
      "translations-translations-en-US-json.e77ca876.bundle.js"
    ],
    "translations-translations-fr-CA-json": [
      "translations-translations-fr-CA-json.d1acb43c.bundle.js"
    ]
  }
}

Error during SSR is:

UnhandledPromiseRejectionWarning: Invariant Violation: loadable: cannot find translations-en-US-json in stats
  at invariant (...)
  at ChunkExtractor.getChunkGroup (...)

Trying to get translations-en-US-json, but the chunk name is translations-translations-en-US-json. I think it's an issue generating the chunk name in the Babel plugin here but not quite sure how best to fix yet. Compiled code in the bundle comes out to:

chunkName(props) {
  return `translations-${props.locale}-json`.replace(/[^a-zA-Z0-9_!§$()=\-^°]+/g, "-");
},

But that should be translations-translations-${props.locale}-json in this case.

To Reproduce

Clone the demo repo here and follow instructions in README to run the code.

Expected behavior

Resolves the correct chunk name in SSR.

Link to repl or repo (highly encouraged)

https://github.com/wvanrensselaer/loadable-components-bug

Run npx envinfo --system --binaries --npmPackages @loadable/component,@loadable/server,@loadable/webpack-plugin,@loadable/babel-plugin --markdown --clipboard

Paste the results here:

## System:
 - OS: macOS 10.15.7
 - CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
 - Memory: 110.46 MB / 16.00 GB
 - Shell: 5.7.1 - /bin/zsh
## Binaries:
 - Node: 14.16.1 - ~/.nvm/versions/node/v14.16.1/bin/node
 - Yarn: 1.22.5 - ~/.yarn/bin/yarn
 - npm: 6.14.12 - ~/.nvm/versions/node/v14.16.1/bin/npm
## npmPackages:
 - @loadable/babel-plugin: ^5.13.2 => 5.13.2 
 - @loadable/component: ^5.14.1 => 5.14.1 
 - @loadable/server: ^5.14.2 => 5.14.2 
 - @loadable/webpack-plugin: ^5.14.2 => 5.14.2 
open-collective-bot[bot] commented 3 years ago

Hey @wvanrensselaer :wave:, Thank you for opening an issue. We'll get back to you as soon as we can. Please, consider supporting us on Open Collective. We give a special attention to issues opened by backers. If you use Loadable at work, you can also ask your company to sponsor us :heart:.

wvanrensselaer commented 3 years ago

Actually impacts regular loadable components too: https://github.com/wvanrensselaer/loadable-components-bug/compare/test-regular-loadable

theKashey commented 3 years ago

Actually a very interesting problem. So "sometimes" webpack 5 does not obey the chunk name it was given.

The simplest and most error-resilient way would be to stop relaying on chunk names/generating chunk names, and do a reverse search from the stats for the given file name - there is a piece of extra information on how files are connected to chunks as well as how chunks are connected to each other... and it was just removed - https://github.com/gregberge/loadable-components/pull/735

The issue requires a little more investigation before moving forward, as it might cause a major architecture change.

theKashey commented 3 years ago

Related: https://github.com/gregberge/loadable-components/issues/740

wvanrensselaer commented 3 years ago

That could be good. Might be tricky too though since Webpack does its best to hide the generated filenames, only accessible by calling __webpack_require__.u with the chunk ID. So still need the chunk ID first unless there's some plugin magic we can do 🤔 .

theKashey commented 3 years ago

Both webpack and babel plugins know everything about everyone.

The problem is right now babel "thinks" that it controls webpack, while (by fact) it is not, and in order to fix it we have to "postprocess" result of babel transformation and correct chunks names.

There are at least two ways to do it::

wvanrensselaer commented 3 years ago

Had some time to dig into this some more. I think we can patch the Babel plugin as a quick fix, need to rework chunkNameProperty slightly. Can't base webpackChunkName on the transformed template literal as it has replaced the directory separator and [request] refers to the whole file name. So when the template literal expression is only a partial file name, loadable breaks:

const Translations = loadable.lib((props) => import(`./translations.${props.locale}.json`));

Transforms to:

const Translations = loadable.lib({
  chunkName(props) {
    return `translations-${props.locale}-json`.replace(/[^a-zA-Z0-9_!§$()=\-^°]+/g, "-");
  },
  importAsync: props => import(
-   /* webpackChunkName: "translations-[request]" */
+   /* webpackChunkName: "[request]" */
    `./translations/messages.${props.locale}.json`
  )
});

I'll try to put a PR together this week. Can work around this for now by using the full file name in the template literal expression:

const Translations = loadable.lib((props) => import(`./${props.file}.json`));

<Translations file={`translations.${locale}`}>
theKashey commented 3 years ago

Not sure this is the right way to move forward. We can or 100% control chunk names, or 100% not. Something in between is not an option, and will for example, not work for wp4, or some future wp5 version, as in this very internal-ish moment.

wvanrensselaer commented 3 years ago

This would still be 100% control of chunk names, just fixing the bug with [request] and partial file names. Verified that it affects Webpack 4 too. But I can hold off and use the workaround above for now.

theKashey commented 3 years ago

So the problem also actual for wp4 and the fix is valid for wp4 as well? That removes all my concerns.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.