isReady returns true even though bundles are not all loaded

[ephys]

🐛 Bug Report

My app will randomly crash depending on the load order of my bundles because loadable()'s isReady doesn't check that all bundles that are needed are loaded (only the main one is checked).

To Reproduce

Steps to reproduce the behavior:

Ok so this is pretty difficult to reproduce because it depends on the load order of different files

I have the following piece of code:

const LoginComp = loadable(() => import('../Login'));

which is being transpiled by @loadable/babel-plugin & @loadable/webpack-plugin into this:

const LoginComp = Object(_loadable_component__WEBPACK_IMPORTED_MODULE_1__[/* default */ "a"])({
  chunkName: function chunkName() {
    return "Login";
  },
  isReady: function isReady(props) {
    if (true) {
      return !!__webpack_require__.m[this.resolve(props)];
    }

    return false;
  },
  requireAsync: function requireAsync() {
    return Promise.all(/* import() | Login */[__webpack_require__.e(0), __webpack_require__.e(2), __webpack_require__.e(1), __webpack_require__.e(3), __webpack_require__.e(63)]).then(__webpack_require__.bind(null, 250));
  },
  requireSync: function requireSync(props) {
    var id = this.resolve(props);

    if (true) {
      return __webpack_require__(id);
    }

    return eval('module.require')(id);
  },
  resolve: function resolve() {
    if (true) {
      return /*require.resolve*/(250);
    }

    return eval('require.resolve')("../Login");
  }
})

In this piece of code, you can see that requireAsync first loads the bundles 0, 2, 1, 3 and 63 before loading module 250 (module 250 is contained by bundle 63 and depends on code present in 0, 2, 1, 3).

Now, I have a separate piece of code that will eventually cause the whole react tree to re-render as soon as another file is loaded (a workaround for react-router which still uses the old context api - I re-render the react tree once the proper locale file is loaded).

Because the whole react tree re-renders, the component generated by loadable() is re-built from scratch.

When that component constructs, it calls isReady to check if it should use requireSync or requireAsync. But isReady only checks that bundle 63 is loaded because it only checks for module 250. So if 0, 2, 1, or 3 are not loaded yet, it will load module 250 which will crash with cannot read property .call of undefined because module 250 will attempt to load a module from one of the other bundles

Expected behavior

isReady is smart enough to detect whether all modules are loaded or not

Link to repl or repo (highly encouraged)

I will attempt to produce one but it's very difficult to get the timing right

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.14.4
 - CPU: (4) x64 Intel(R) Core(TM) i7-6567U CPU @ 3.30GHz
 - Memory: 54.16 MB / 16.00 GB
 - Shell: 3.2.57 - /bin/bash
## Binaries:
 - Node: 10.16.1 - /usr/local/bin/node
 - Yarn: 1.2.1 - ~/.yarn/bin/yarn
 - npm: 6.9.0 - /usr/local/bin/npm
 - Watchman: 4.7.0 - /usr/local/bin/watchman
## npmPackages:
 - @loadable/component: ^5.10.1 => 5.10.2 

[ephys]

Potential ways to fix this:

Ideal way:

Check that bundles are loaded along with modules, but I cannot figure out a way to list which chunks needs to be requireEnsured to load a given module (webpack/webpack#8847 maybe?)

Workaround way:

When calling requireAsync, store a property isAsyncRequiring on a global object that will persist if the loadable component is reset, and set it back to false once the promise resolves. While this will fix my current issue, it won't resolve the problem if another piece of code loads those bundles too

[gregberge]

Hello @ephys, I think we need help from webpack to fix it. They should provide a method to be able to check if a module is completely loaded. Do we have a correct solution to know if a module is completely loaded?

[theKashey]

Do we have a correct solution to know if a module is completely loaded?

require.weak is returning an object with keys.

[gregberge]

If someone has a better solution than the one implemented, please feel free to submit a PR. Else we have to speak with webpack to add the missing method.

[joornby]

This particular issue is biting me as well.

Our application has a slightly different use case than @ephys. We use a mixture of SSR + SPA interactions, so the way I duplicated it was adding a delay on one of the chunks for a page, and then doing multiple single page navigations. After doing a few, I navigated back to the first page. The module for that page was loaded, but the delayed chunk was not. isReady returned true because the module was loaded. The loadSync function was fired, and it attempted to import code that didn't exist in the loaded modules resulting in a cannot read property .call of undefined

One option is to change isReady to use require.ensure on the chunks for a module, which would change it to a promise. I believe that changing isReady to a promise would break SSR, so that would need to be figured out first.

The other option is to convince weback (maybe on this issue: webpack/webpack#5429) to expose the array they use to keep track of chunks similar to how they expose the __webpack_modules___ array.

Thoughts?

[theKashey]

Track module states by our own.

• on server side all modules could be loaded in sync. It's safe.
• on client side all modules could be loaded in async way, as long as we could "wait" for them. It's safe.
• on client side we might remember what we did with some loadables, and that would also help with import error inside async loaded component is hidden by @loadable #420

[joornby]

https://github.com/smooth-code/loadable-components/blob/f1e75b6c8bac855c60b973f9fcb90c99d257ea65/packages/component/src/createLoadable.js#L84

This is the line that causes our application to throw the error. My initial understanding of that line was that it was loading the component synchronously in order to support SSR. The underlying component is already rendered, and assumed to be ready due to the initial payload scripts loading before it gets run.

I'm interested in the idea of loading the modules async all the time on the client, but how would you support SSR if you have to wait for the asyncRequire to resolve?

[theKashey]

So, in short - we are in trouble! Let me explain (and document) the problem:

• from your prospective you are doing no more import('./myFile')
• from loadable prospective - that's two different commands - loadSync, which is require('./myFile'), and loadAsync, which is import('./myFile')
• plus there is also webpack import, which uses relationships between chunks.

Let's make a pause here, and recall what aggressive code splitting in webpack 4 is - it might split your code up to 5 chunks by default.
So.... import('./myFile') would be webpacked into require('./myFile') + require('./vendor') + require('./commonMyFile~AnotherFile') and so on. This is how it works.

However - isReady checks only myFile existence which might be a false positive - and that's is the problem.

In short - isReady and requireSync should no longer be used. We have to call provided imports as-is, and keep track of the resulted promises.

[joornby]

I'm still lacking insight into how you're going to support SSR if you keep track of the import promises, but I'll let you run that out.

Here's a repro with steps in the readme: https://github.com/ColinxLLC/webpack-undefined-chunk-repro

• The webpack config does not use hashes so that the file names are predictable.
• The app uses 2 modules with a shared heavy chunk
• The server forces a 5 second wait on the shared chunk
• The client navigates on mount between the different modules
• The third navigation reliably (based off of 30 minutes of testing) has the first module loaded and the chunk not loaded

[theKashey]

Thank you the example, I will try to solve the problem on weekend.

[theKashey]

Attack plan:

• remove loadSync. We could not use it. Try to read result back from a provided loadable definition.
• remove isReady as readed from module cache. Just store result (of a promise) in a provided loadable definition
• so we are going to use loadable definition as a mutable state to store data, and unbound logic from webpack internal realization, we could not rely on.

Where loadable definition is the result of babel plugin transformation.

After talking to @neoziro I've realized that my approach would not play well with loadable SSR model.
The only way is to change hot resolve is working

resolve: function resolve() {
    if (true) {
      return /*require.resolve*/(250);
    }

    return eval('require.resolve')("../Login");
  }

// to
resolve: function resolve() {
    if (true) {
// aka [__webpack_require__.e(0), __webpack_require__.e(2), __webpack_require__.e(1), __webpack_require__.e(3), __webpack_require__.e(63)]).then(__webpack_require__.bind(null, 250) 
      return [0, 2, 1, 3, 63, 250]
    }

    return eval('require.resolve')("../Login");
  }

How it's actually might work

const chunks = [];
// overload import
const original = __webpack_require__.e;
__webpack_require__.e = chunkId => chunks.push(e);

// run import function
requireAsync();

// unmock
__webpack_require__.e = original;

// all required chunks are stored in `chunks`

Then isReady, would be aware of all required chunks, and requireSync might require these chunks in the same order (although that not needed).

I don't quite like this solution, but another way is to resolve all required chunks inside loadable webpack plugin, and I am not sure how...

[theKashey]

Ok, there is a very simple way to handle this case:

• the server is sending chunks, which all be loaded before the application can hydrate
• if the required chunk is listed among these ones - you are able to use requireSync as well as isReady
• on SSR, when you can require something synchronously - you are always isReady
• in all other cases, ie with client-side driven loading - always use requiredAsync, as long as we could not guarantee that chunk is loaded in full.

Plus - on a ServerSide we shall execute requireAsync for the every requireSync call, and if results do not match each other - throw an error (except the cases with the ssr: false flag)

[foglerek]

For what it's worth, I'm running into what looks like a similar issue when using MinChunkSizePlugin.

Essentially, given this:

loadable-component-a.js // loadable(() => import('./ComponentA.js'));
  |_merged-chunk-a.js // import { foo } from 'bar';

loadable-component-b.js // loadable(() => import('./ComponentB.js'));
  |_merged-chunk-a.js // import { foo } from 'bar';

merged-chunk-a.js is never requested when the loadable component(s) execute during client-side navigation, and webpack errors out when it tries to execute modules[moduleId].call(...), which is quickly followed by a stack-trace where loadable attempted to call loadSync. This does not happen if the route is SSR'd.

I don't think this is related specifically to MinChunkSizePlugin, but rather is a result of the chunk dependency tree generated by it, and loadable doing a loadSync even though the chunk has not actually been loaded.

[theKashey]

Yep, exactly our problem.

[wagnerjsilva]

We're also having the same problem, and what makes it hard is how sporadic things are.

[theKashey]

Ok, so:

• always load everything sync at SSR
• always load async at the client
• perform isReady check only for initially loaded chunks

[kfroemming-cb]

Any update on this issue?

[theKashey]

Waiting for @neoziro approval.

[mikaelgramont]

Hi @neoziro, is there a plan for a release with this fix? The bug is pretty painful and it'd be great to be able to move past it. Thanks!

[ephys]

Thank you @theKashey :)