fix: Path pitfall for loader type

crates/rspack_binding_options/src/plugins/js_loader/resolver.rs

@@ -110,6 +110,10 @@ pub(crate) async fn resolve_loader(
         description_data,
         ..
       } = resource;
+      // Pitfall: `Path::ends_with` is different from `str::ends_with`
+      // So we need to convert `PathBuf` to `&str`
+      // Use `str::ends_with` instead of `Path::extension` to avoid unnecessary allocation
+      let path = path.as_str();
 
       let r#type = if path.ends_with(".mjs") {
         Some("module")

packages/rspack/src/loader-runner/index.ts

@@ -105,7 +105,9 @@ function createLoaderObject(
 				obj.ident = ident;
 			}
 
-			obj.type = value.type;
+			// CHANGE: `rspack_core` returns empty string for `undefined` type.
+			// Comply to webpack test case: tests/webpack-test/cases/loaders/cjs-loader-type/index.js
+			obj.type = value.type === "" ? undefined : value.type;
 			if (obj.options === null) obj.query = "";
 			else if (obj.options === undefined) obj.query = "";
 			else if (typeof obj.options === "string") obj.query = `?${obj.options}`;

tests/webpack-test/cases/loaders/cjs-loader-type/index.js

@@ -0,0 +1,15 @@
+it("should pass package.json type to loader", function () {
+	expect(require("cjs/loader.js!")).toBe("commonjs");
+	expect(require("./loader.js!")).toBe("undefined");
+});
+
+it("should pass 'commonjs' type to loader for .cjs", function () {
+	expect(require("cjs/loader.cjs!")).toBe("commonjs");
+	expect(require("./loader.cjs!")).toBe("commonjs");
+	// ORIGINAL WEBPACK COMMENT: TODO need fix in v8 https://github.com/nodejs/node/issues/35889
+	// ORIGINAL WEBPACK COMMENT: TODO otherwise this test case cause segment fault
+	// Turned on this as rspack checks extensions for loader type.
+	// So this will not fall into dynamic import which causes segment fault.
+	// See: crates/rspack_binding_options/src/plugins/js_loader/resolver.rs
+	expect(require("esm/loader.cjs!")).toBe("commonjs");
+});

tests/webpack-test/cases/loaders/cjs-loader-type/loader.cjs

@@ -0,0 +1,4 @@
+/** @type {import("../../../../").LoaderDefinition} */
+module.exports = function loader() {
+	return `module.exports = "${this.loaders[this.loaderIndex].type}";`;
+};

tests/webpack-test/cases/loaders/cjs-loader-type/loader.js

@@ -0,0 +1,4 @@
+/** @type {import("../../../../").LoaderDefinition} */
+module.exports = function loader() {
+	return `module.exports = "${this.loaders[this.loaderIndex].type}";`;
+};