Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chunk graph / "available modules optimisation" refactor in 4.38.0 breaks our bundles #9634

Closed
marcins opened this issue Aug 27, 2019 · 8 comments · Fixed by #9635
Closed

Comments

@marcins
Copy link

marcins commented Aug 27, 2019

Bug report

What is the current behavior?

We have a large, 25,000+ module, build with multiple entry points and judicious usage of code splitting. In the latest version of Webpack we have a scenario where one of our async bundles when loaded through a chain starting from a particular entry point cannot find one of the modules it requires, because it is only included in a different entry point. Running a git bisect identified the commit where this behaviour changed as 126fb99

Unfortunately at this stage I am unable to produce a minimal reproduction, and our codebase is not public. The commit in question moved code as well as refactored, so it's hard to tell from the diff exactly what changed in the chunk graph algorithm, however based on what we're seeing I am suspecting the "available modules optimization" is at fault, where a module is not being bundled where it should be because it's considered "available" when it isn't.

Here is the scenario, slightly simplified:

  • given two entry points, entry-a and entry-b
  • entry-a eventually depends on module-a via a mix of sync & async imports (code splits)
  • entry-a also depends on module-b via a mix of sync & async imports (code splits), through a path including shared-module-a.
  • module-b depends on module-a
  • entry-b depends on shared-module-a through a mix of sync & sync imports (and so eventually depends on module-a via module-b

(Dashes are code splits, solid are direct imports - there's a lot more of these in the real codebase, which might be relevant)

image

After commit 126fb99, module-a is bundled into entry-a, so when we load entry-b and module-b tries to load module-a a runtime error is thrown because module-a cannot be found.

Before commit 126fb99, module-a will be bundled into module-b (and entry-a), so when we load entry-b it can be found when module-b tries to load it.

If the current behavior is a bug, please provide the steps to reproduce.

As above, I cannot currently provide a minimal reproduction, but I am working on it and will update the issue when I have narrowed it down further.

It does not appear that any optimisation settings affect this - I can repro in dev mode, prod mode, with webpack-dev-server, and anything in between - it's, to my current understanding, a fundamental bug in the graph building code.

I have tried to produce a simplified scenario as described above, but it's something more specific about our particular combination of sync/async imports that is obviously causing this issue to occur, hopefully if I can identify which part of 126fb99 causes the issue I can work how to reproduce the issue (and/or provide a fix).

What is the expected behavior?

I expect that when module-b is loaded via entry-b, it can still access module-a because it has been bundled somewhere in the modules available to entry-b.

Other relevant information:
webpack version: 4.39.2
Node.js version: 10.9.0
Operating System: macOS 10.14.6
Additional tools:

@sokra
Copy link
Member

sokra commented Aug 27, 2019

it's, to my current understanding, a fundamental bug in the graph building code.

Yep, I think so too.


One thing makes me wondering: There is no sync dependency from entry-a to module-a, so there is no reason why module-a should be included in entry-a. Maybe intermediate-a to intermediate-b is a sync dependency instead of an async one?

@sokra
Copy link
Member

sokra commented Aug 27, 2019

Ok I was able to reproduce it, thanks

@marcins
Copy link
Author

marcins commented Aug 27, 2019

Great news! Curious as to what the minimal reproduction case is, as a colleague and I spent some time on this today without much luck.

@marcins
Copy link
Author

marcins commented Aug 27, 2019

After looking at my real dependency graph some more you're right - the shortest path from entry-a to module-a is all sync. I know you said you already repro'd it, but I was still curious - I was able to repro with a minimal case using this setup, this was the smallest tree that worked, removing any of the async chain would "fix" the issue:

image

@sokra
Copy link
Member

sokra commented Aug 27, 2019

Path from entry-a to module-a need to have less async dependencies than from entry-b to module-a.

sokra added a commit that referenced this issue Aug 27, 2019
when minAvailableModules of a ChunkGroup shrink, children of this ChunkGroup need to be recalculated
@sokra sokra mentioned this issue Aug 27, 2019
@marcins
Copy link
Author

marcins commented Aug 27, 2019

Confirm that applying your PR as a patch to webpack in my node_modules appears to fix the issue in our real codebase. Thanks for the quick turnaround, especially given the vague repro information! ❤️

sokra added a commit that referenced this issue Aug 27, 2019
@sokra
Copy link
Member

sokra commented Aug 27, 2019

https://github.com/webpack/webpack/releases/tag/v4.39.3

@sokra
Copy link
Member

sokra commented Aug 27, 2019

Thanks for the quick turnaround, especially given the vague repro information! ❤️

Thanks for the report. The graph was very helpful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants