fix node-module-module.test.js on windows (#10620)

oven-sh · Apr 28, 2024 · c7aed7e · c7aed7e
1 parent 84d81c3
commit c7aed7e
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 34 deletions.
diff --git 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<JSC::Exception> exception;
     evaluateCommonJSModuleOnce(

diff --git 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
@@ -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
@@ -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", () => {