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

fi3ework · fi3ework · commit 60f189871a4c · 2024-08-20T13:57:40.000+08:00
diff --git a/lib/ExternalModule.js b/lib/ExternalModule.js
@@ -166,7 +166,7 @@ const getSourceForImportExternal = (
 	const importName = runtimeTemplate.outputOptions.importFunctionName;
 	if (
 		!runtimeTemplate.supportsDynamicImport() &&
-		(importName === "import" || importName !== "module-import")
+		(importName === "import" || importName === "module-import")
 	) {
 		throw new Error(
 			"The target environment doesn't support 'import()' so it's not possible to use external type 'import'"
@@ -546,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
@@ -597,14 +616,7 @@ class ExternalModule extends Module {
 			case "module":
 			case "import":
 			case "module-import": {
-				const type =
-					externalType === "module-import" &&
-					this.dependencyMeta &&
-					/** @type {ImportDependencyMeta} */ (this.dependencyMeta).externalType
-						? /** @type {ImportDependencyMeta} */ (this.dependencyMeta)
-								.externalType
-						: externalType;
-
+				const type = this.getModuleImportType(externalType);
 				if (type === "module") {
 					if (this.buildInfo.module) {
 						if (!Array.isArray(request) || request.length === 1) {
@@ -742,13 +754,7 @@ class ExternalModule extends Module {
 			case "module":
 			case "import":
 			case "module-import": {
-				const type =
-					externalType === "module-import" &&
-					dependencyMeta &&
-					/** @type {ImportDependencyMeta} */ (dependencyMeta).externalType
-						? /** @type {ImportDependencyMeta} */ (dependencyMeta).externalType
-						: externalType;
-
+				const type = this.getModuleImportType(externalType);
 				if (type === "import") {
 					return getSourceForImportExternal(
 						request,
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"]
+};
diff --git a/test/configCases/externals/module-import/webpack.config.js b/test/configCases/externals/module-import/webpack.config.js
@@ -0,0 +1,41 @@
+/** @type {import("../../../../types").Configuration} */
+module.exports = {
+	target: ["web", "es2020"],
+	node: {
+		__dirname: false,
+		__filename: false
+	},
+	output: {
+		module: true,
+		filename: "[name].js"
+	},
+	entry: {
+		a: "./a",
+		main: "./index"
+	},
+	optimization: {
+		concatenateModules: true
+	},
+	experiments: {
+		outputModule: true
+	},
+	externalsType: "module-import",
+	externals: [
+		function (
+			{ context, request, contextInfo, getResolve, dependencyType },
+			callback
+		) {
+			if (request === "external2") {
+				return callback(null, "node-commonjs external2");
+			}
+			callback();
+		},
+		{
+			external0: "external0",
+			external1: "external1",
+			external3: "external3",
+			fs: "commonjs fs",
+			path: "commonjs path"
+		}
+	]
+};
diff --git a/types.d.ts b/types.d.ts
@@ -4582,6 +4582,7 @@ declare class ExternalModule extends Module {
 	externalType: string;
 	userRequest: string;
 	dependencyMeta?: ImportDependencyMeta | CssImportDependencyMeta;
+	getModuleImportType(externalType: string): string;
 
 	/**
 	 * restore unsafe cache data

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+module.exports = {`
	`2`	`+ findBundle: (i, options) => ["main.js"]`
	`3`	`+};`