revert: do not use heuristic fallback for "module-import"

webpack · Aug 20, 2024 · f46a03c · f46a03c
2 parents 8bf907b + 60f1898
commit f46a03c
Show file tree

Hide file tree

Showing 11 changed files with 158 additions and 151 deletions.
diff --git a/lib/ExternalModule.js b/lib/ExternalModule.js
@@ -22,7 +22,6 @@ const propertyAccess = require("./util/propertyAccess");
 const { register } = require("./util/serialization");
 
 /** @typedef {import("webpack-sources").Source} Source */
-/** @typedef {import("../declarations/WebpackOptions").ExternalsPresets} ExternalsPresets */
 /** @typedef {import("../declarations/WebpackOptions").WebpackOptionsNormalized} WebpackOptions */
 /** @typedef {import("./Chunk")} Chunk */
 /** @typedef {import("./ChunkGraph")} ChunkGraph */
@@ -54,7 +53,7 @@ const { register } = require("./util/serialization");
 /** @typedef {import("./util/fs").InputFileSystem} InputFileSystem */
 /** @typedef {import("./util/runtime").RuntimeSpec} RuntimeSpec */
 
-/** @typedef {{ attributes?: ImportAttributes, externalType: "import" | "module" | undefined, externalsPresets: ExternalsPresets | undefined }} ImportDependencyMeta */
+/** @typedef {{ attributes?: ImportAttributes, externalType: "import" | "module" | undefined }} ImportDependencyMeta */
 /** @typedef {{ layer?: string, supports?: string, media?: string }} CssImportDependencyMeta */
 
 /** @typedef {ImportDependencyMeta | CssImportDependencyMeta} DependencyMeta */
@@ -547,6 +546,25 @@ class ExternalModule extends Module {
 		return callback(null, !this.buildMeta);
 	}
 
+	/**
+	 * @param {string} externalType raw external type
+	 * @returns {string} resolved external type
+	 */
+	getModuleImportType(externalType) {
+		if (externalType === "module-import") {
+			if (
+				this.dependencyMeta &&
+				/** @type {ImportDependencyMeta} */ (this.dependencyMeta).externalType
+			) {
+				return /** @type {ImportDependencyMeta} */ (this.dependencyMeta)
+					.externalType;
+			}
+			return "module";
+		}
+
+		return externalType;
+	}
+
 	/**
 	 * @param {WebpackOptions} options webpack options
 	 * @param {Compilation} compilation the compilation
@@ -579,25 +597,6 @@ class ExternalModule extends Module {
 					canMangle = true;
 				}
 				break;
-			case "module":
-				if (this.buildInfo.module) {
-					if (!Array.isArray(request) || request.length === 1) {
-						this.buildMeta.exportsType = "namespace";
-						canMangle = true;
-					}
-				} else {
-					this.buildMeta.async = true;
-					EnvironmentNotSupportAsyncWarning.check(
-						this,
-						compilation.runtimeTemplate,
-						"external module"
-					);
-					if (!Array.isArray(request) || request.length === 1) {
-						this.buildMeta.exportsType = "namespace";
-						canMangle = false;
-					}
-				}
-				break;
 			case "script":
 				this.buildMeta.async = true;
 				EnvironmentNotSupportAsyncWarning.check(
@@ -614,18 +613,45 @@ class ExternalModule extends Module {
 					"external promise"
 				);
 				break;
+			case "module":
 			case "import":
-				this.buildMeta.async = true;
-				EnvironmentNotSupportAsyncWarning.check(
-					this,
-					compilation.runtimeTemplate,
-					"external import"
-				);
-				if (!Array.isArray(request) || request.length === 1) {
-					this.buildMeta.exportsType = "namespace";
-					canMangle = false;
+			case "module-import": {
+				const type = this.getModuleImportType(externalType);
+				if (type === "module") {
+					if (this.buildInfo.module) {
+						if (!Array.isArray(request) || request.length === 1) {
+							this.buildMeta.exportsType = "namespace";
+							canMangle = true;
+						}
+					} else {
+						this.buildMeta.async = true;
+						EnvironmentNotSupportAsyncWarning.check(
+							this,
+							compilation.runtimeTemplate,
+							"external module"
+						);
+						if (!Array.isArray(request) || request.length === 1) {
+							this.buildMeta.exportsType = "namespace";
+							canMangle = false;
+						}
+					}
 				}
+
+				if (type === "import") {
+					this.buildMeta.async = true;
+					EnvironmentNotSupportAsyncWarning.check(
+						this,
+						compilation.runtimeTemplate,
+						"external import"
+					);
+					if (!Array.isArray(request) || request.length === 1) {
+						this.buildMeta.exportsType = "namespace";
+						canMangle = false;
+					}
+				}
+
 				break;
+			}
 		}
 		this.addDependency(new StaticExportsDependency(true, canMangle));
 		callback();
@@ -659,36 +685,6 @@ class ExternalModule extends Module {
 
 	_getRequestAndExternalType() {
 		let { request, externalType } = this;
-
-		if (externalType === "module-import") {
-			const dependencyMeta = /** @type {ImportDependencyMeta} */ (
-				this.dependencyMeta
-			);
-
-			if (dependencyMeta && dependencyMeta.externalType) {
-				externalType = dependencyMeta.externalType;
-			} else if (dependencyMeta && dependencyMeta.externalsPresets) {
-				const presets = dependencyMeta.externalsPresets;
-				// TODO: what if user set multiple presets?
-				if (presets.web) {
-					externalType = "module";
-				} else if (presets.webAsync) {
-					externalType = "import";
-				} else if (
-					presets.electron ||
-					presets.electronMain ||
-					presets.electronPreload ||
-					presets.electronRenderer ||
-					presets.node ||
-					presets.nwjs
-				) {
-					externalType = "node-commonjs";
-				}
-			} else {
-				externalType = "commonjs";
-			}
-		}
-
 		if (typeof request === "object" && !Array.isArray(request))
 			request = request[externalType];
 		return { request, externalType };
@@ -753,43 +749,52 @@ class ExternalModule extends Module {
 					runtimeTemplate
 				);
 			}
-			case "import":
-				return getSourceForImportExternal(
-					request,
-					runtimeTemplate,
-					/** @type {ImportDependencyMeta} */ (dependencyMeta)
-				);
 			case "script":
 				return getSourceForScriptExternal(request, runtimeTemplate);
-			case "module": {
-				if (!(/** @type {BuildInfo} */ (this.buildInfo).module)) {
-					if (!runtimeTemplate.supportsDynamicImport()) {
-						throw new Error(
-							`The target environment doesn't support dynamic import() syntax so it's not possible to use external type 'module' within a script${
-								runtimeTemplate.supportsEcmaScriptModuleSyntax()
-									? "\nDid you mean to build a EcmaScript Module ('output.module: true')?"
-									: ""
-							}`
-						);
-					}
+			case "module":
+			case "import":
+			case "module-import": {
+				const type = this.getModuleImportType(externalType);
+				if (type === "import") {
 					return getSourceForImportExternal(
 						request,
 						runtimeTemplate,
 						/** @type {ImportDependencyMeta} */ (dependencyMeta)
 					);
 				}
-				if (!runtimeTemplate.supportsEcmaScriptModuleSyntax()) {
-					throw new Error(
-						"The target environment doesn't support EcmaScriptModule syntax so it's not possible to use external type 'module'"
+
+				if (type === "module") {
+					if (!(/** @type {BuildInfo} */ (this.buildInfo).module)) {
+						if (!runtimeTemplate.supportsDynamicImport()) {
+							throw new Error(
+								`The target environment doesn't support dynamic import() syntax so it's not possible to use external type 'module' within a script${
+									runtimeTemplate.supportsEcmaScriptModuleSyntax()
+										? "\nDid you mean to build a EcmaScript Module ('output.module: true')?"
+										: ""
+								}`
+							);
+						}
+						return getSourceForImportExternal(
+							request,
+							runtimeTemplate,
+							/** @type {ImportDependencyMeta} */ (dependencyMeta)
+						);
+					}
+					if (!runtimeTemplate.supportsEcmaScriptModuleSyntax()) {
+						throw new Error(
+							"The target environment doesn't support EcmaScriptModule syntax so it's not possible to use external type 'module'"
+						);
+					}
+					return getSourceForModuleExternal(
+						request,
+						moduleGraph.getExportsInfo(this),
+						runtime,
+						runtimeTemplate,
+						/** @type {ImportDependencyMeta} */ (dependencyMeta)
 					);
 				}
-				return getSourceForModuleExternal(
-					request,
-					moduleGraph.getExportsInfo(this),
-					runtime,
-					runtimeTemplate,
-					/** @type {ImportDependencyMeta} */ (dependencyMeta)
-				);
+
+				break;
 			}
 			case "var":
 			case "promise":

diff --git a/lib/ExternalModuleFactoryPlugin.js b/lib/ExternalModuleFactoryPlugin.js
@@ -14,7 +14,6 @@ const ImportDependency = require("./dependencies/ImportDependency");
 const { resolveByProperty, cachedSetProperty } = require("./util/cleverMerge");
 
 /** @typedef {import("../declarations/WebpackOptions").Externals} Externals */
-/** @typedef {import("../declarations/WebpackOptions").ExternalsPresets} ExternalsPresets */
 /** @typedef {import("./Compilation").DepConstructor} DepConstructor */
 /** @typedef {import("./ExternalModule").DependencyMeta} DependencyMeta */
 /** @typedef {import("./Module")} Module */
@@ -61,12 +60,10 @@ class ExternalModuleFactoryPlugin {
 	/**
 	 * @param {string | undefined} type default external type
 	 * @param {Externals} externals externals config
-	 * @param {ExternalsPresets} [externalsPresets] externals preset config
 	 */
-	constructor(type, externals, externalsPresets) {
+	constructor(type, externals) {
 		this.type = type;
 		this.externals = externals;
-		this.externalsPresets = externalsPresets;
 	}
 
 	/**
@@ -75,7 +72,6 @@ class ExternalModuleFactoryPlugin {
 	 */
 	apply(normalModuleFactory) {
 		const globalType = this.type;
-		const externalsPresets = this.externalsPresets;
 		normalModuleFactory.hooks.factorize.tapAsync(
 			"ExternalModuleFactoryPlugin",
 			(data, callback) => {
@@ -139,21 +135,14 @@ class ExternalModuleFactoryPlugin {
 
 						dependencyMeta = {
 							attributes: dependency.assertions,
-							externalType,
-							externalsPresets
+							externalType
 						};
 					} else if (dependency instanceof CssImportDependency) {
 						dependencyMeta = {
 							layer: dependency.layer,
 							supports: dependency.supports,
 							media: dependency.media
 						};
-					} else {
-						dependencyMeta = {
-							attributes: undefined,
-							externalType: undefined,
-							externalsPresets
-						};
 					}
 
 					callback(

diff --git a/lib/ExternalsPlugin.js b/lib/ExternalsPlugin.js
@@ -27,11 +27,9 @@ class ExternalsPlugin {
 	 */
 	apply(compiler) {
 		compiler.hooks.compile.tap("ExternalsPlugin", ({ normalModuleFactory }) => {
-			new ExternalModuleFactoryPlugin(
-				this.type,
-				this.externals,
-				compiler.options.externalsPresets
-			).apply(normalModuleFactory);
+			new ExternalModuleFactoryPlugin(this.type, this.externals).apply(
+				normalModuleFactory
+			);
 		});
 	}
 }

diff --git a/test/configCases/externals/module-import-externals-presets/index.js b/test/configCases/externals/module-import-externals-presets/index.js
diff --git a/test/configCases/externals/module-import-externals-presets/test.filter.js b/test/configCases/externals/module-import-externals-presets/test.filter.js
diff --git a/test/configCases/externals/module-import-externals-presets/webpack.config.js b/test/configCases/externals/module-import-externals-presets/webpack.config.js
diff --git a/test/configCases/externals/module-import/a.js b/test/configCases/externals/module-import/a.js
@@ -0,0 +1,6 @@
+import external0 from "external0"; // module
+const external1 = require("external1"); // module
+const external2 = require("external2"); // node-commonjs
+const external3 = import("external3"); // import
+
+console.log(external0, external1, external2, external3);
diff --git a/test/configCases/externals/module-import/index.js b/test/configCases/externals/module-import/index.js
@@ -0,0 +1,10 @@
+const fs = require("fs");
+const path = require("path");
+
+it("module-import should correctly get fallback type", function() {
+	const content = fs.readFileSync(path.resolve(__dirname, "a.js"), "utf-8");
+	expect(content).toContain(`import * as __WEBPACK_EXTERNAL_MODULE_external0__ from "external0";`); // module
+	expect(content).toContain(`import * as __WEBPACK_EXTERNAL_MODULE_external1__ from "external1";`); // module
+	expect(content).toContain(`module.exports = __WEBPACK_EXTERNAL_createRequire(import.meta.url)("external2");`); // node-commonjs
+	expect(content).toContain(`module.exports = import("external3");`); // import
+});
diff --git a/test/configCases/externals/module-import/test.config.js b/test/configCases/externals/module-import/test.config.js
@@ -0,0 +1,3 @@
+module.exports = {
+	findBundle: (i, options) => ["main.js"]
+};