Repository: webpack/webpack (JavaScript)

Issue:
Duplicate modules are found in both entrypoint chunks and async chunks
<!-- Please don't delete this template because we'll close your issue -->
<!-- Before creating an issue please make sure you are using the latest version of webpack. -->

# Bug report

<!-- Please ask questions on StackOverflow or the webpack Gitter. -->
<!-- https://stackoverflow.com/questions/ask?tags=webpack -->
<!-- https://gitter.im/webpack/webpack -->
<!-- Issues which contain questions or support requests will be closed. -->

**What is the current behavior?**
Under certain circumstances, modules in entrypoint chunks will be bundled into other async chunks.

**If the current behavior is a bug, please provide the steps to reproduce.**
Here is the reproducible test repo: https://github.com/Mensu/webpack-reproduce-issue-15173

To reproduce the issue, simply run `npm install` and `npm run build`. After the build is finished, search for `"moduleAB"` in the `dist` folder, and find out module `src/moduleAB.js` was bundled into chunk `dist/entryA.js`, chunk `dist/entryB.js` (entrypoint chunks, which is expected) and chunk `dist/commonAsync.js` (async chunks, which is unexpected)

Module `src/moduleAB.js` is statically imported by entrypoint modules `src/entryA.js` and `src/entryB.js`, therefore is available in entrypoint chunks `dist/entryA.js` and `dist/entryB.js`. Since `dist/commonAsync.js` is an async chunk, the factory of module `src/moduleAB.js` must have been available in the entrypoint chunks when we are to load `dist/commonAsync.js`, therefore it seems unnecessary to duplicately bundle `src/moduleAB.js` into `dist/commonAsync.js`.

<!-- A great way to do this is to provide your configuration via a GitHub repository -->
<!-- The most helpful is a minimal reproduction with instructions on how to reproduce -->
<!-- Repositories with too many files or large `webpack.config.js` files are not suitable -->
<!-- Please only add small code snippets directly into this issue -->
<!-- https://gist.github.com is a good place for longer code snippets -->
<!-- If your issue is caused by a plugin or loader, please create an issue on the loader/plugin repository instead -->

**What is the expected behavior?**
Module `src/moduleAB.js` was only bundled into entrypoint chunks `dist/entryA.js` and `dist/entryB.js`, not into `dist/commonAsync.js`.

I kind of believe the problem lies here `availableModules.has(module) || availableModules.plus.has(m)`
https://github.com/webpack/webpack/blob/v5.66.0/lib/buildChunkGraph.js#L906-L909

The first condition checks whether `availableModules` has `module`, but when it comes to the second condition, it checks whether `availableModules.plus` has `m` instead of `module`, which is inconsistent with the first condition. I'm not sure whether checking against `m` is intended for some reason I'm missing, or it's just a typo. Changing the condition to `availableModules.plus.has(module)` may fix this issue, but I don't know whether the change will break other codes or not.

<!-- "It should work" is not a helpful explanation -->
<!-- Explain exactly how it should behave -->

**Other relevant information:**
webpack version:  5.66.0
Node.js version: 12.10.0
Operating System: Mac OS
Additional tools: N/A



Hints:
You can set:
```
optimization: {
    splitChunks: {
      chunks: 'all',
      minSize: 0,
    }
  }
```

And they will be moved to own chunk
> You can set:
> 
> ```
> optimization: {
>     splitChunks: {
>       chunks: 'all',
>       minSize: 0,
>     }
>   }
> ```
> 
> And they will be moved to own chunk

@alexander-akait Thanks for your suggestion, but in my real project, there are many modules duplicately bundled in the way described in the OP. Applying the suggestion results in so many small chunks, one for each of the duplicate modules, which seems unnecessary to me.

I think it's natural to bundle the duplicate modules only into the entrypoint chunks when the modules are imported by modules of both entrypoint chunks and async chunks, as webpack has already achieved this in most of the cases (For example in the OP, webpack wouldn't duplicately bundle module `src/moduleAB.js` into `dist/featureA.js`, whereas `dist/featureA.js` imports `src/moduleAB.js`. I take it that webpack knows `src/moduleAB.js` is already available in the entrypoint chunk `dist/entryA.js`, so it's unnecessary to bundle `src/moduleAB.js` again into `dist/featureA.js`)
Not in all cases, that is why we provide `splitChunks` option, where you can setup necessary behavior, yes, sometimes it is not easy to setup, but we can't provide the best output for all
> Not in all cases, that is why we provide `splitChunks` option, where you can setup necessary behavior, yes, sometimes it is not easy to setup, but we can't provide the best output for all

@alexander-akait I agree that you can't provide the best output for all cases, but I think it's possible for this case.

Looking around the codes after `availableModules.plus.has(m)`
https://github.com/webpack/webpack/blob/v5.66.0/lib/buildChunkGraph.js#L906-L909

I find that other codes in similar logics are like `availableModules.plus.has(module)`, not checking agains `m`
https://github.com/webpack/webpack/blob/v5.66.0/lib/buildChunkGraph.js#L1006-L1009
https://github.com/webpack/webpack/blob/v5.66.0/lib/buildChunkGraph.js#L1060-L1063

Considering that changing `m` to `module` would fix this case, I'm so curious about the inconsistence.  @sokra might have some context for this inconsistence?


Patch:
diff --git a/lib/buildChunkGraph.js b/lib/buildChunkGraph.js
--- a/lib/buildChunkGraph.js
+++ b/lib/buildChunkGraph.js
@@ -905,7 +905,7 @@ const visitModules = (
 										const module = it.value;
 										if (
 											availableModules.has(module) ||
-											availableModules.plus.has(m)
+											availableModules.plus.has(module)
 										) {
 											cachedMinAvailableModules.add(module);
 										}


Test patch:
diff --git a/test/configCases/chunk-graph/issue-15173/commonAsync/index.js b/test/configCases/chunk-graph/issue-15173/commonAsync/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/commonAsync/index.js
@@ -0,0 +1,5 @@
+import { commonUtil } from "../commonSync";
+
+export function getCommonAsync() {
+	return commonUtil();
+}
diff --git a/test/configCases/chunk-graph/issue-15173/commonSync/index.js b/test/configCases/chunk-graph/issue-15173/commonSync/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/commonSync/index.js
@@ -0,0 +1,5 @@
+var EmptyObj = {};
+
+export function commonUtil() {
+	return EmptyObj;
+}
diff --git a/test/configCases/chunk-graph/issue-15173/entries/entryA.js b/test/configCases/chunk-graph/issue-15173/entries/entryA.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/entries/entryA.js
@@ -0,0 +1,19 @@
+import { commonUtil } from "../commonSync";
+
+export default {
+	doSomethingInEntryA() {
+		return commonUtil("entryA");
+	},
+	getFeatureA() {
+		return import(/* webpackChunkName: 'featureA' */ "../featureA");
+	},
+	getFeatureB() {
+		return import(/* webpackChunkName: 'featureB' */ "../featureB");
+	}
+};
+
+it("common async should contain self only", () => {
+	expect(
+		__STATS__.chunks.find(c => c.names.includes("commonAsync")).modules
+	).toHaveLength(1);
+});
diff --git a/test/configCases/chunk-graph/issue-15173/entries/entryB.js b/test/configCases/chunk-graph/issue-15173/entries/entryB.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/entries/entryB.js
@@ -0,0 +1,10 @@
+import { commonUtil } from "../commonSync";
+
+export default {
+	doSomethingInEntryB() {
+		return commonUtil("entryB");
+	},
+	getFeatureC() {
+		return import(/* webpackChunkName: 'featureC' */ "../featureC");
+	}
+};
diff --git a/test/configCases/chunk-graph/issue-15173/featureA/index.js b/test/configCases/chunk-graph/issue-15173/featureA/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/featureA/index.js
@@ -0,0 +1,5 @@
+import(/* webpackChunkName: 'commonAsync' */ "../commonAsync");
+
+export function getFeatureA() {
+	return "featureA";
+}
diff --git a/test/configCases/chunk-graph/issue-15173/featureB/index.js b/test/configCases/chunk-graph/issue-15173/featureB/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/featureB/index.js
@@ -0,0 +1,5 @@
+import(/* webpackChunkName: 'commonAsync' */ "../commonAsync");
+
+export function getFeatureB() {
+	return "featureB";
+}
diff --git a/test/configCases/chunk-graph/issue-15173/featureC/index.js b/test/configCases/chunk-graph/issue-15173/featureC/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/featureC/index.js
@@ -0,0 +1,5 @@
+import(/* webpackChunkName: 'commonAsync' */ "../commonAsync");
+
+export function getFeatureC() {
+	return "featuraC";
+}
diff --git a/test/configCases/chunk-graph/issue-15173/test.config.js b/test/configCases/chunk-graph/issue-15173/test.config.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/test.config.js
@@ -0,0 +1,5 @@
+module.exports = {
+	findBundle: function () {
+		return ["entryA.js"];
+	}
+};
diff --git a/test/configCases/chunk-graph/issue-15173/webpack.config.js b/test/configCases/chunk-graph/issue-15173/webpack.config.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/chunk-graph/issue-15173/webpack.config.js
@@ -0,0 +1,9 @@
+module.exports = {
+	entry: {
+		entryA: "./entries/entryA.js",
+		entryB: "./entries/entryB.js"
+	},
+	output: {
+		filename: "[name].js"
+	}
+};