diff --git a/src/bun.js/bindings/ImportMetaObject.cpp b/src/bun.js/bindings/ImportMetaObject.cpp index b7df44de7e..c4c678244b 100644 --- a/src/bun.js/bindings/ImportMetaObject.cpp +++ b/src/bun.js/bindings/ImportMetaObject.cpp @@ -335,6 +335,13 @@ extern "C" JSC::EncodedJSValue functionImportMeta__resolveSyncPrivate(JSC::JSGlo auto bunStr = Bun::toString(parentIdStr); args.append(jsBoolean(Bun__isBunMain(lexicalGlobalObject, &bunStr))); + // Pass options object with paths if provided + if (!userPathList.isUndefinedOrNull()) { + JSObject* options = JSC::constructEmptyObject(globalObject); + options->putDirect(vm, JSC::Identifier::fromString(vm, "paths"_s), userPathList); + args.append(options); + } + JSValue result = JSC::profiledCall(lexicalGlobalObject, ProfilingReason::API, overrideHandler, JSC::getCallData(overrideHandler), parentModuleObject, args); RETURN_IF_EXCEPTION(scope, {}); if (!isRequireDotResolve) { diff --git a/src/bun.js/modules/NodeModuleModule.cpp b/src/bun.js/modules/NodeModuleModule.cpp index 6655ea9fbc..c0e5355b63 100644 --- a/src/bun.js/modules/NodeModuleModule.cpp +++ b/src/bun.js/modules/NodeModuleModule.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "JavaScriptCore/Completion.h" #include "JavaScriptCore/JSNativeStdFunction.h" #include "JSCommonJSExtensions.h" @@ -311,31 +312,106 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionResolveFileName, default: { JSC::JSValue moduleName = callFrame->argument(0); JSC::JSValue fromValue = callFrame->argument(1); + JSC::JSValue optionsValue = callFrame->argument(3); // 4th argument is options + auto& names = builtinNames(vm); if (moduleName.isUndefinedOrNull()) { JSC::throwTypeError(globalObject, scope, "Module._resolveFilename expects a string"_s); return {}; } - if ( - // fast path: it's a real CommonJS module object. - auto* cjs = jsDynamicCast(fromValue)) { - fromValue = cjs->filename(); - } else if - // slow path: userland code did something weird. lets let them do that - // weird thing. - (fromValue.isObject()) { + // Extract filename string from fromValue + // Follows pattern: typeof this === "string" ? this : (this?.filename ?? this?.id ?? "") + if (!fromValue.isString()) { + if ( + // fast path: it's a real CommonJS module object. + auto* cjs = jsDynamicCast(fromValue)) { + fromValue = cjs->filename(); + } else if (fromValue.isObject()) { + // slow path: userland code did something weird. Try filename first, then id + auto* obj = fromValue.getObject(); + auto filenameValue = obj->getIfPropertyExists(globalObject, names.filenamePublicName()); + RETURN_IF_EXCEPTION(scope, {}); - auto idValue = fromValue.getObject()->getIfPropertyExists(globalObject, builtinNames(vm).filenamePublicName()); - RETURN_IF_EXCEPTION(scope, {}); - if (idValue) { - if (idValue.isString()) { - fromValue = idValue; + if (filenameValue && filenameValue.isString()) { + fromValue = filenameValue; + } else { + auto idValue = obj->getIfPropertyExists(globalObject, vm.propertyNames->id); + RETURN_IF_EXCEPTION(scope, {}); + + if (idValue && idValue.isString()) { + fromValue = idValue; + } else { + // Fallback to empty string if no valid filename or id + fromValue = jsEmptyString(vm); + } } + } else { + // Not a string, not an object - use empty string + fromValue = jsEmptyString(vm); } } - auto result = Bun__resolveSync(globalObject, JSC::JSValue::encode(moduleName), JSValue::encode(fromValue), false, true); + // Handle options.paths if provided + JSC::JSValue pathsValue = JSC::jsUndefined(); + if (optionsValue.isObject()) { + pathsValue = optionsValue.getObject()->getIfPropertyExists(globalObject, JSC::Identifier::fromString(vm, "paths"_s)); + RETURN_IF_EXCEPTION(scope, {}); + } + + JSC::EncodedJSValue result; + + // If paths are provided, use Bun__resolveSyncWithPaths + if (!pathsValue.isUndefinedOrNull()) { + // Node.js requires options.paths to be an array + if (!JSC::isArray(globalObject, pathsValue)) { + Bun::throwError(globalObject, scope, + Bun::ErrorCode::ERR_INVALID_ARG_TYPE, + "options.paths must be an array"_s); + return {}; + } + + WTF::Vector paths; + + // Iterate through the array using forEachInIterable + forEachInIterable(globalObject, pathsValue, [&](JSC::VM&, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue item) { + if (scope.exception()) + return; + + WTF::String pathStr = item.toWTFString(lexicalGlobalObject); + if (scope.exception()) + return; + + paths.append(Bun::toStringRef(pathStr)); + }); + + if (scope.exception()) { + // Clean up on exception + for (auto& path : paths) { + path.deref(); + } + return {}; + } + + result = Bun__resolveSyncWithPaths(globalObject, JSC::JSValue::encode(moduleName), JSValue::encode(fromValue), false, true, paths.begin(), paths.size()); + + // Clean up BunStrings to avoid leaking + for (auto& path : paths) { + path.deref(); + } + + RETURN_IF_EXCEPTION(scope, {}); + + if (!JSC::JSValue::decode(result).isString()) { + JSC::throwException(globalObject, scope, JSC::JSValue::decode(result)); + return {}; + } + + return result; + } + + // No paths provided, use regular resolution + result = Bun__resolveSync(globalObject, JSC::JSValue::encode(moduleName), JSValue::encode(fromValue), false, true); RETURN_IF_EXCEPTION(scope, {}); if (!JSC::JSValue::decode(result).isString()) { diff --git a/test/js/node/module/module-resolve-filename-paths.test.js b/test/js/node/module/module-resolve-filename-paths.test.js new file mode 100644 index 0000000000..43740bee95 --- /dev/null +++ b/test/js/node/module/module-resolve-filename-paths.test.js @@ -0,0 +1,201 @@ +// Use bun:test in Bun, or node:test in Node.js +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "fs"; +import Module from "module"; +import { tmpdir } from "os"; +import { dirname, join, resolve } from "path"; + +// Detect runtime and import appropriate test framework +const isBun = typeof Bun !== "undefined"; +let test, expect; + +if (isBun) { + ({ test, expect } = await import("bun:test")); +} else { + // Node.js + const { createRequire } = await import("module"); + const nodeTest = await import("node:test"); + const assert = await import("node:assert/strict"); + + // In Node.js ES modules, require is not available, so create it + globalThis.require = createRequire(import.meta.url); + + test = nodeTest.test; + // Create Bun-compatible expect from Node assert + expect = value => ({ + toBe: expected => assert.strictEqual(value, expected), + toEqual: expected => assert.deepStrictEqual(value, expected), + toBeDefined: () => assert.notStrictEqual(value, undefined), + toThrow: () => { + // This is used with expect(() => ...) + assert.throws(value); + }, + }); +} + +// Helper to create temp directory - works in both Bun and Node +function createTempDir(prefix, files) { + // Use realpathSync to resolve symlinks (handles /tmp -> /private/tmp on macOS) + const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix + "-"))); + + for (const [filePath, content] of Object.entries(files)) { + const fullPath = join(dir, filePath); + const dirPath = dirname(fullPath); + + // Create parent directories if needed + mkdirSync(dirPath, { recursive: true }); + writeFileSync(fullPath, content, "utf-8"); + } + + return { + path: dir, + cleanup: () => rmSync(dir, { recursive: true, force: true }), + }; +} + +test("Module._resolveFilename respects options.paths for package resolution", () => { + const { path: dir, cleanup } = createTempDir("module-resolve-paths", { + "node_modules/test-package/package.json": JSON.stringify({ name: "test-package", main: "index.js" }), + "node_modules/test-package/index.js": "module.exports = 'test-package';", + }); + + try { + // Create a fake parent module in a different directory + const fakeParent = new Module("/some/other/directory/file.js"); + fakeParent.filename = "/some/other/directory/file.js"; + fakeParent.paths = Module._nodeModulePaths("/some/other/directory"); + + // Without paths option, this should fail + expect(() => { + Module._resolveFilename("test-package", fakeParent); + }).toThrow(); + + // With paths option, this should succeed + const resolved = Module._resolveFilename("test-package", fakeParent, false, { + paths: [dir], + }); + + expect(resolved).toBe(resolve(dir, "node_modules/test-package/index.js")); + } finally { + cleanup(); + } +}); + +test("Module._resolveFilename respects options.paths for relative paths", () => { + const { path: dir, cleanup } = createTempDir("module-resolve-relative", { + "target.js": "module.exports = 'target';", + }); + + try { + const fakeParent = new Module("/some/other/directory/file.js"); + fakeParent.filename = "/some/other/directory/file.js"; + fakeParent.paths = Module._nodeModulePaths("/some/other/directory"); + + // With paths option pointing to dir, should resolve relative to that dir + const resolved = Module._resolveFilename("./target.js", fakeParent, false, { + paths: [dir], + }); + + expect(resolved).toBe(resolve(dir, "target.js")); + } finally { + cleanup(); + } +}); + +test("Module._resolveFilename with overridden function receives options.paths", () => { + const originalResolveFilename = Module._resolveFilename; + let capturedOptions; + + try { + // Override _resolveFilename to capture the options + Module._resolveFilename = function (request, parent, isMain, options) { + capturedOptions = options; + return originalResolveFilename.call(this, request, parent, isMain, options); + }; + + const { path: dir, cleanup } = createTempDir("module-resolve-override", { + "node_modules/test-pkg/package.json": JSON.stringify({ name: "test-pkg", main: "index.js" }), + "node_modules/test-pkg/index.js": "module.exports = 'test';", + }); + + try { + const fakeParent = new Module("/some/other/directory/file.js"); + fakeParent.filename = "/some/other/directory/file.js"; + fakeParent.paths = Module._nodeModulePaths("/some/other/directory"); + + const testPaths = [dir]; + + // Call _resolveFilename with paths option + Module._resolveFilename("test-pkg", fakeParent, false, { + paths: testPaths, + }); + + // Verify the override function received the options with paths + expect(capturedOptions).toBeDefined(); + expect(capturedOptions.paths).toEqual(testPaths); + } finally { + cleanup(); + } + } finally { + Module._resolveFilename = originalResolveFilename; + } +}); + +test("require.resolve respects options.paths for package resolution", () => { + const { path: dir, cleanup } = createTempDir("require-resolve-paths", { + "node_modules/resolve-test-pkg/package.json": JSON.stringify({ + name: "resolve-test-pkg", + main: "index.js", + }), + "node_modules/resolve-test-pkg/index.js": "module.exports = 'resolve-test';", + }); + + try { + // require.resolve should work with paths option + const resolved = require.resolve("resolve-test-pkg", { + paths: [dir], + }); + + expect(resolved).toBe(resolve(dir, "node_modules/resolve-test-pkg/index.js")); + } finally { + cleanup(); + } +}); + +test("require.resolve with relative path and options.paths (Next.js use case)", () => { + // This reproduces the Next.js babel-plugin-react-compiler resolution issue + const { path: dir, cleanup } = createTempDir("nextjs-style-resolve", { + "node_modules/babel-plugin-react-compiler/package.json": JSON.stringify({ + name: "babel-plugin-react-compiler", + main: "dist/index.js", + }), + "node_modules/babel-plugin-react-compiler/dist/index.js": "module.exports = {};", + }); + + try { + // Simulate what Next.js does: resolve a relative path with explicit paths + const resolved = require.resolve("./node_modules/babel-plugin-react-compiler", { + paths: [dir], + }); + + expect(resolved).toBe(resolve(dir, "node_modules/babel-plugin-react-compiler/dist/index.js")); + } finally { + cleanup(); + } +}); + +test("Module._resolveFilename throws ERR_INVALID_ARG_TYPE if options.paths is not an array", () => { + // Test with string (which is iterable but not an array) + expect(() => { + Module._resolveFilename("path", __filename, false, { paths: "/some/path" }); + }).toThrow(); + + // Test with Set (which is iterable but not an array) + expect(() => { + Module._resolveFilename("path", __filename, false, { paths: new Set(["/some/path"]) }); + }).toThrow(); + + // Test with object (not iterable) + expect(() => { + Module._resolveFilename("path", __filename, false, { paths: { 0: "/some/path" } }); + }).toThrow(); +}); diff --git a/test/no-validate-exceptions.txt b/test/no-validate-exceptions.txt index 67563774a7..62134a7a00 100644 --- a/test/no-validate-exceptions.txt +++ b/test/no-validate-exceptions.txt @@ -26,6 +26,7 @@ test/js/node/test/parallel/test-worker.mjs test/js/node/test/system-ca/test-native-root-certs.test.mjs test/js/node/events/event-emitter.test.ts test/js/node/module/node-module-module.test.js +test/js/node/module/module-resolve-filename-paths.test.js test/js/node/process/call-constructor.test.js test/js/node/stubs.test.js test/js/node/timers/node-timers.test.ts