From c7aed7e0a3d285ebf755c48295f1dd209bfa9dc9 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Sun, 28 Apr 2024 16:54:47 -0700 Subject: [PATCH] fix `node-module-module.test.js` on windows (#10620) --- src/bun.js/bindings/CommonJSModuleRecord.cpp | 21 +++++++++---------- src/bun.js/bindings/PathInlines.h | 17 ++++++++------- src/bun.js/modules/NodeModuleModule.h | 15 +++++++------ .../js/node/module/node-module-module.test.js | 18 +++++++++------- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index 64bd934a0bb726..eee6d0858684e2 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -489,7 +489,8 @@ JSC_DEFINE_HOST_FUNCTION(functionCommonJSModuleRecord_compile, (JSGlobalObject * String sourceString = callframe->argument(0).toWTFString(globalObject); RETURN_IF_EXCEPTION(throwScope, JSValue::encode({})); - String filenameString = callframe->argument(1).toWTFString(globalObject); + JSValue filenameValue = callframe->argument(1); + String filenameString = filenameValue.toWTFString(globalObject); RETURN_IF_EXCEPTION(throwScope, JSValue::encode({})); String wrappedString = makeString( @@ -505,16 +506,14 @@ JSC_DEFINE_HOST_FUNCTION(functionCommonJSModuleRecord_compile, (JSGlobalObject * WTF::TextPosition(), JSC::SourceProviderSourceType::Program); - auto index = filenameString.reverseFind(PLATFORM_SEP, filenameString.length()); - // filenameString is coming from js, any separator could be used - if (index == WTF::notFound) - index = filenameString.reverseFind(NOT_PLATFORM_SEP, filenameString.length()); - String dirnameString; - if (index != WTF::notFound) { - dirnameString = filenameString.substring(0, index); - } else { - dirnameString = "/"_s; - } + EncodedJSValue encodedFilename = JSValue::encode(filenameValue); +#if OS(WINDOWS) + JSValue dirnameValue = JSValue::decode(Bun__Path__dirname(globalObject, true, &encodedFilename, 1)); +#else + JSValue dirnameValue = JSValue::decode(Bun__Path__dirname(globalObject, false, &encodedFilename, 1)); +#endif + + String dirnameString = dirnameValue.toWTFString(globalObject); WTF::NakedPtr exception; evaluateCommonJSModuleOnce( diff --git a/src/bun.js/bindings/PathInlines.h b/src/bun.js/bindings/PathInlines.h index 1bedaa091b758c..f39445dc28be56 100644 --- a/src/bun.js/bindings/PathInlines.h +++ b/src/bun.js/bindings/PathInlines.h @@ -1,16 +1,17 @@ #pragma once #include "root.h" +#define POSIX_PATH_SEP_s "/"_s +#define POSIX_PATH_SEP '/' +#define WINDOWS_PATH_SEP_s "\\"_s +#define WINDOWS_PATH_SEP '\\' + #if OS(WINDOWS) -#define PLATFORM_SEP_s "\\"_s -#define PLATFORM_SEP '\\' -#define NOT_PLATFORM_SEP_s "/"_s -#define NOT_PLATFORM_SEP '/' +#define PLATFORM_SEP_s WINDOWS_PATH_SEP_s +#define PLATFORM_SEP WINDOWS_PATH_SEP #else -#define PLATFORM_SEP_s "/"_s -#define PLATFORM_SEP '/' -#define NOT_PLATFORM_SEP_s "\\"_s -#define NOT_PLATFORM_SEP '\\' +#define PLATFORM_SEP_s POSIX_PATH_SEP_s +#define PLATFORM_SEP POSIX_PATH_SEP #endif ALWAYS_INLINE bool isAbsolutePath(WTF::String input) diff --git a/src/bun.js/modules/NodeModuleModule.h b/src/bun.js/modules/NodeModuleModule.h index f7639da82edcb4..58e0146871460b 100644 --- a/src/bun.js/modules/NodeModuleModule.h +++ b/src/bun.js/modules/NodeModuleModule.h @@ -394,15 +394,14 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionResolveLookupPaths, (JSC::JSGlobalObject * gl return JSValue::encode(array); } - JSString* dirname = nullptr; + JSValue dirname; if (parent.filename) { - String str = parent.filename->value(globalObject); - if (str.endsWith(PLATFORM_SEP_s)) { - str = str.substring(0, str.length() - 1); - } else if (str.contains(PLATFORM_SEP)) { - str = str.substring(0, str.reverseFind(PLATFORM_SEP)); - } - dirname = jsString(vm, str); + EncodedJSValue encodedFilename = JSValue::encode(parent.filename); +#if OS(WINDOWS) + dirname = JSValue::decode(Bun__Path__dirname(globalObject, true, &encodedFilename, 1)); +#else + dirname = JSValue::decode(Bun__Path__dirname(globalObject, false, &encodedFilename, 1)); +#endif } else { dirname = jsString(vm, String("."_s)); } diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index 6fa6dd670f300d..d61e67a006ec97 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -87,21 +87,25 @@ test("Overwriting Module.prototype.require", () => { expect(exitCode).toBe(0); }); -test("Module.prototype._compile", () => { +test.each([ + "/file/name/goes/here.js", + "file/here.js", + "file\\here.js", + "/file\\here.js", + "\\file\\here.js", + "\\file/here.js", +])("Module.prototype._compile", filename => { const module = new Module("module id goes here"); const starting_exports = module.exports; - const r = module._compile( - "module.exports = { module, exports, require, __filename, __dirname }", - "/file/path/goes/here.js", - ); + const r = module._compile("module.exports = { module, exports, require, __filename, __dirname }", filename); expect(r).toBe(undefined); expect(module.exports).not.toBe(starting_exports); const { module: m, exports: e, require: req, __filename: fn, __dirname: dn } = module.exports; expect(m).toBe(module); expect(e).toBe(starting_exports); expect(req).toBe(module.require); - expect(fn).toBe("/file/path/goes/here.js"); - expect(dn).toBe("/file/path/goes"); + expect(fn).toBe(filename); + expect(dn).toBe(path.dirname(filename)); }); test("Module._extensions", () => {