Merge pull request #12831 from animecyc/bugfix/shared-plugin-runtime-bug

fix: use runtime globals in shared plugin
webpack · Mar 11, 2021 · 8ea0a6a · 8ea0a6a
2 parents e711608 + cf57a14
commit 8ea0a6a
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 74 deletions.
diff --git a/lib/node/RequireChunkLoadingRuntimeModule.js b/lib/node/RequireChunkLoadingRuntimeModule.js
@@ -105,28 +105,30 @@ class RequireChunkLoadingRuntimeModule extends RuntimeModule {
 			withLoading
 				? Template.asString([
 						"// require() chunk loading for javascript",
-						`${fn}.require = function(chunkId, promises) {`,
-						hasJsMatcher !== false
-							? Template.indent([
-									"",
-									'// "1" is the signal for "already loaded"',
-									"if(!installedChunks[chunkId]) {",
-									Template.indent([
-										hasJsMatcher === true
-											? "if(true) { // all chunks have JS"
-											: `if(${hasJsMatcher("chunkId")}) {`,
+						`${fn}.require = ${runtimeTemplate.basicFunction(
+							"chunkId, promises",
+							hasJsMatcher !== false
+								? [
+										'// "1" is the signal for "already loaded"',
+										"if(!installedChunks[chunkId]) {",
 										Template.indent([
-											`installChunk(require(${JSON.stringify(
-												rootOutputDir
-											)} + ${RuntimeGlobals.getChunkScriptFilename}(chunkId)));`
+											hasJsMatcher === true
+												? "if(true) { // all chunks have JS"
+												: `if(${hasJsMatcher("chunkId")}) {`,
+											Template.indent([
+												`installChunk(require(${JSON.stringify(
+													rootOutputDir
+												)} + ${
+													RuntimeGlobals.getChunkScriptFilename
+												}(chunkId)));`
+											]),
+											"} else installedChunks[chunkId] = 1;",
+											""
 										]),
-										"} else installedChunks[chunkId] = 1;",
-										""
-									]),
-									"}"
-							  ])
-							: "installedChunks[chunkId] = 1;",
-						"};"
+										"}"
+								  ]
+								: "installedChunks[chunkId] = 1;"
+						)};`
 				  ])
 				: "// no chunk loading",
 			"",

diff --git a/lib/sharing/ConsumeSharedPlugin.js b/lib/sharing/ConsumeSharedPlugin.js
@@ -292,6 +292,7 @@ class ConsumeSharedPlugin {
 					PLUGIN_NAME,
 					(chunk, set) => {
 						set.add(RuntimeGlobals.module);
+						set.add(RuntimeGlobals.moduleCache);
 						set.add(RuntimeGlobals.moduleFactoriesAddOnly);
 						set.add(RuntimeGlobals.shareScopeMap);
 						set.add(RuntimeGlobals.initializeSharing);

diff --git a/lib/sharing/ConsumeSharedRuntimeModule.js b/lib/sharing/ConsumeSharedRuntimeModule.js
@@ -268,17 +268,16 @@ class ConsumeSharedRuntimeModule extends RuntimeModule {
 				? Template.asString([
 						`var initialConsumes = ${JSON.stringify(initialConsumes)};`,
 						`initialConsumes.forEach(${runtimeTemplate.basicFunction("id", [
-							`__webpack_modules__[id] = ${runtimeTemplate.basicFunction(
-								"module",
-								[
-									"// Handle case when module is used sync",
-									"installedModules[id] = 0;",
-									"delete __webpack_module_cache__[id];",
-									"var factory = moduleToHandlerMapping[id]();",
-									'if(typeof factory !== "function") throw new Error("Shared module is not available for eager consumption: " + id);',
-									`module.exports = factory();`
-								]
-							)}`
+							`${
+								RuntimeGlobals.moduleFactories
+							}[id] = ${runtimeTemplate.basicFunction("module", [
+								"// Handle case when module is used sync",
+								"installedModules[id] = 0;",
+								`delete ${RuntimeGlobals.moduleCache}[id];`,
+								"var factory = moduleToHandlerMapping[id]();",
+								'if(typeof factory !== "function") throw new Error("Shared module is not available for eager consumption: " + id);',
+								`module.exports = factory();`
+							])}`
 						])});`
 				  ])
 				: "// no consumes in initial chunks",
@@ -302,21 +301,22 @@ class ConsumeSharedRuntimeModule extends RuntimeModule {
 											"factory",
 											[
 												"installedModules[id] = 0;",
-												`__webpack_modules__[id] = ${runtimeTemplate.basicFunction(
-													"module",
-													[
-														"delete __webpack_module_cache__[id];",
-														"module.exports = factory();"
-													]
-												)}`
+												`${
+													RuntimeGlobals.moduleFactories
+												}[id] = ${runtimeTemplate.basicFunction("module", [
+													`delete ${RuntimeGlobals.moduleCache}[id];`,
+													"module.exports = factory();"
+												])}`
 											]
 										)};`,
 										`var onError = ${runtimeTemplate.basicFunction("error", [
 											"delete installedModules[id];",
-											`__webpack_modules__[id] = ${runtimeTemplate.basicFunction(
-												"module",
-												["delete __webpack_module_cache__[id];", "throw error;"]
-											)}`
+											`${
+												RuntimeGlobals.moduleFactories
+											}[id] = ${runtimeTemplate.basicFunction("module", [
+												`delete ${RuntimeGlobals.moduleCache}[id];`,
+												"throw error;"
+											])}`
 										])};`,
 										"try {",
 										Template.indent([

diff --git a/lib/webworker/ImportScriptsChunkLoadingRuntimeModule.js b/lib/webworker/ImportScriptsChunkLoadingRuntimeModule.js
@@ -8,8 +8,10 @@ const RuntimeGlobals = require("../RuntimeGlobals");
 const RuntimeModule = require("../RuntimeModule");
 const Template = require("../Template");
 const {
-	getChunkFilenameTemplate
+	getChunkFilenameTemplate,
+	chunkHasJs
 } = require("../javascript/JavascriptModulesPlugin");
+const compileBooleanMatcher = require("../util/compileBooleanMatcher");
 const { getUndoPath } = require("../util/identifier");
 
 class ImportScriptsChunkLoadingRuntimeModule extends RuntimeModule {
@@ -25,6 +27,7 @@ class ImportScriptsChunkLoadingRuntimeModule extends RuntimeModule {
 		const {
 			chunk,
 			compilation: {
+				chunkGraph,
 				runtimeTemplate,
 				outputOptions: { globalObject, chunkLoadingGlobal, hotUpdateGlobal }
 			}
@@ -43,6 +46,10 @@ class ImportScriptsChunkLoadingRuntimeModule extends RuntimeModule {
 		const chunkLoadingGlobalExpr = `${globalObject}[${JSON.stringify(
 			chunkLoadingGlobal
 		)}]`;
+		const hasJsMatcher = compileBooleanMatcher(
+			chunkGraph.getChunkConditionMap(chunk, chunkHasJs)
+		);
+
 		const outputName = this.compilation.getPath(
 			getChunkFilenameTemplate(chunk, this.compilation.outputOptions),
 			{
@@ -51,6 +58,7 @@ class ImportScriptsChunkLoadingRuntimeModule extends RuntimeModule {
 			}
 		);
 		const rootOutputDir = getUndoPath(outputName, false);
+
 		return Template.asString([
 			withBaseURI
 				? Template.asString([
@@ -71,42 +79,54 @@ class ImportScriptsChunkLoadingRuntimeModule extends RuntimeModule {
 			withLoading
 				? Template.asString([
 						"// importScripts chunk loading",
-						`var chunkLoadingCallback = ${runtimeTemplate.basicFunction(
-							"data",
-							[
-								runtimeTemplate.destructureArray(
-									["chunkIds", "moreModules", "runtime"],
-									"data"
-								),
-								"for(var moduleId in moreModules) {",
-								Template.indent([
-									`if(${RuntimeGlobals.hasOwnProperty}(moreModules, moduleId)) {`,
-									Template.indent(
-										`${RuntimeGlobals.moduleFactories}[moduleId] = moreModules[moduleId];`
-									),
-									"}"
-								]),
-								"}",
-								"if(runtime) runtime(__webpack_require__);",
-								"while(chunkIds.length)",
-								Template.indent("installedChunks[chunkIds.pop()] = 1;"),
-								"parentChunkLoadingFunction(data);"
-							]
-						)};`,
-						`${fn}.i = ${runtimeTemplate.basicFunction("chunkId, promises", [
-							'// "1" is the signal for "already loaded"',
-							"if(!installedChunks[chunkId]) {",
+						`var installChunk = ${runtimeTemplate.basicFunction("data", [
+							runtimeTemplate.destructureArray(
+								["chunkIds", "moreModules", "runtime"],
+								"data"
+							),
+							"for(var moduleId in moreModules) {",
 							Template.indent([
-								`importScripts(${JSON.stringify(rootOutputDir)} + ${
-									RuntimeGlobals.getChunkScriptFilename
-								}(chunkId));`
+								`if(${RuntimeGlobals.hasOwnProperty}(moreModules, moduleId)) {`,
+								Template.indent(
+									`${RuntimeGlobals.moduleFactories}[moduleId] = moreModules[moduleId];`
+								),
+								"}"
 							]),
-							"}"
-						])};`,
+							"}",
+							"if(runtime) runtime(__webpack_require__);",
+							"while(chunkIds.length)",
+							Template.indent("installedChunks[chunkIds.pop()] = 1;"),
+							"parentChunkLoadingFunction(data);"
+						])};`
+				  ])
+				: "// no chunk install function needed",
+			withLoading
+				? Template.asString([
+						`${fn}.i = ${runtimeTemplate.basicFunction(
+							"chunkId, promises",
+							hasJsMatcher !== false
+								? [
+										'// "1" is the signal for "already loaded"',
+										"if(!installedChunks[chunkId]) {",
+										Template.indent([
+											hasJsMatcher === true
+												? "if(true) { // all chunks have JS"
+												: `if(${hasJsMatcher("chunkId")}) {`,
+											Template.indent(
+												`importScripts(${JSON.stringify(rootOutputDir)} + ${
+													RuntimeGlobals.getChunkScriptFilename
+												}(chunkId));`
+											),
+											"}"
+										]),
+										"}"
+								  ]
+								: "installedChunks[chunkId] = 1;"
+						)};`,
 						"",
 						`var chunkLoadingGlobal = ${chunkLoadingGlobalExpr} = ${chunkLoadingGlobalExpr} || [];`,
 						"var parentChunkLoadingFunction = chunkLoadingGlobal.push.bind(chunkLoadingGlobal);",
-						"chunkLoadingGlobal.push = chunkLoadingCallback;"
+						"chunkLoadingGlobal.push = installChunk;"
 				  ])
 				: "// no chunk loading",
 			"",

diff --git a/test/hotCases/sharing/share-plugin/common.js b/test/hotCases/sharing/share-plugin/common.js
@@ -0,0 +1 @@
+export default 'common-lib'
diff --git a/test/hotCases/sharing/share-plugin/index.js b/test/hotCases/sharing/share-plugin/index.js
@@ -0,0 +1,14 @@
+import value, { getValue } from "./module";
+
+it("should accept a shared dependency", async () => {
+	expect(value).toBe("module");
+	await expect(getValue()).resolves.toHaveProperty("default", "module");
+	module.hot.accept("./module");
+
+	await new Promise((resolve, reject) =>
+		NEXT(require("../../update")(reject, true, resolve))
+	);
+
+	expect(value).toBe("common-lib");
+	await expect(getValue()).resolves.toHaveProperty("default", "common-lib");
+});
diff --git a/test/hotCases/sharing/share-plugin/module.js b/test/hotCases/sharing/share-plugin/module.js
@@ -0,0 +1,7 @@
+export default "module"
+
+export const getValue = () => Promise.resolve({ default: "module" });
+---
+export { default } from "common"
+
+export const getValue = () => import("common2");
diff --git a/test/hotCases/sharing/share-plugin/webpack.config.js b/test/hotCases/sharing/share-plugin/webpack.config.js
@@ -0,0 +1,23 @@
+// eslint-disable-next-line node/no-unpublished-require
+const { SharePlugin } = require("../../../../").sharing;
+
+/** @type {import("../../../../").Configuration} */
+module.exports = {
+	mode: "development",
+	devtool: false,
+	plugins: [
+		new SharePlugin({
+			shared: {
+				common: {
+					eager: true,
+					import: "./common?1",
+					requiredVersion: "1.1.1"
+				},
+				common2: {
+					import: "./common?2",
+					requiredVersion: "1.1.1"
+				}
+			}
+		})
+	]
+};