random fixes that help vite/sveltekit (#3140)

* existsSync with invalid paths should return false * partially support file urls (does not do percent encoding) * add utf16 support for Path.isAbsoluteString * Update src/resolver/resolver.zig Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> * fixups * revert * prettier format --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
oven-sh · Jun 1, 2023 · 42d8b71 · 42d8b71
1 parent f9809f0
commit 42d8b71
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 37 deletions.
diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts
@@ -282,7 +282,7 @@ declare module "bun" {
    * @returns A promise that resolves with the concatenated chunks or the concatenated chunks as an `ArrayBuffer`.
    */
   export function readableStreamToArrayBuffer(
-    stream: ReadableStream,
+    stream: ReadableStream<ArrayBuffer>,
   ): Promise<ArrayBuffer> | ArrayBuffer;
 
   /**
@@ -323,7 +323,7 @@ declare module "bun" {
    *
    */
   export function readableStreamToArray<T>(
-    stream: ReadableStream,
+    stream: ReadableStream<T>,
   ): Promise<T[]> | T[];
 
   /**

diff --git a/src/bun.js/builtins/WebCoreJSBuiltins.cpp b/src/bun.js/builtins/WebCoreJSBuiltins.cpp
@@ -2420,9 +2420,9 @@ const char* const s_readableStreamReadableStreamToTextCode = "(function (_){\"us
 const JSC::ConstructAbility s_readableStreamReadableStreamToArrayBufferCodeConstructAbility = JSC::ConstructAbility::CannotConstruct;
 const JSC::ConstructorKind s_readableStreamReadableStreamToArrayBufferCodeConstructorKind = JSC::ConstructorKind::None;
 const JSC::ImplementationVisibility s_readableStreamReadableStreamToArrayBufferCodeImplementationVisibility = JSC::ImplementationVisibility::Private;
-const int s_readableStreamReadableStreamToArrayBufferCodeLength = 212;
+const int s_readableStreamReadableStreamToArrayBufferCodeLength = 271;
 static const JSC::Intrinsic s_readableStreamReadableStreamToArrayBufferCodeIntrinsic = JSC::NoIntrinsic;
-const char* const s_readableStreamReadableStreamToArrayBufferCode = "(function (_){\"use strict\";var p=@getByIdDirectPrivate(_,\"underlyingSource\");if(p!==@undefined)return @readableStreamToArrayBufferDirect(_,p);return @Bun.readableStreamToArray(_).@then(@Bun.concatArrayBuffers)})\n";
+const char* const s_readableStreamReadableStreamToArrayBufferCode = "(function (_){\"use strict\";var f=@getByIdDirectPrivate(_,\"underlyingSource\");if(f!==@undefined)return @readableStreamToArrayBufferDirect(_,f);var A=@Bun.readableStreamToArray(_);if(@isPromise(A))return A.@then(@Bun.concatArrayBuffers);return @Bun.concatArrayBuffers(A)})\n";
 
 // readableStreamToJSON
 const JSC::ConstructAbility s_readableStreamReadableStreamToJSONCodeConstructAbility = JSC::ConstructAbility::CannotConstruct;

diff --git a/src/bun.js/builtins/builtins.d.ts b/src/bun.js/builtins/builtins.d.ts
@@ -441,10 +441,6 @@ declare class OutOfMemoryError {
   constructor();
 }
 
-declare class ReadableStream {
-  constructor(stream: unknown, view?: unknown);
-  values(options?: unknown): AsyncIterableIterator<unknown>;
-}
 declare class ReadableStreamDefaultController {
   constructor(
     stream: unknown,

diff --git a/src/bun.js/builtins/ts/ReadableStream.ts b/src/bun.js/builtins/ts/ReadableStream.ts
@@ -102,7 +102,7 @@ export function initializeReadableStream(this: any, underlyingSource: Underlying
 }
 
 $linkTimeConstant;
-export function readableStreamToArray(stream) {
+export function readableStreamToArray(stream: ReadableStream): Promise<unknown[]> {
   // this is a direct stream
   var underlyingSource = $getByIdDirectPrivate(stream, "underlyingSource");
   if (underlyingSource !== undefined) {
@@ -113,7 +113,7 @@ export function readableStreamToArray(stream) {
 }
 
 $linkTimeConstant;
-export function readableStreamToText(stream) {
+export function readableStreamToText(stream: ReadableStream): Promise<string> {
   // this is a direct stream
   var underlyingSource = $getByIdDirectPrivate(stream, "underlyingSource");
   if (underlyingSource !== undefined) {
@@ -124,24 +124,29 @@ export function readableStreamToText(stream) {
 }
 
 $linkTimeConstant;
-export function readableStreamToArrayBuffer(stream) {
+export function readableStreamToArrayBuffer(stream: ReadableStream<ArrayBuffer>): Promise<ArrayBuffer> | ArrayBuffer {
   // this is a direct stream
   var underlyingSource = $getByIdDirectPrivate(stream, "underlyingSource");
 
   if (underlyingSource !== undefined) {
     return $readableStreamToArrayBufferDirect(stream, underlyingSource);
   }
 
-  return Promise.resolve(Bun.readableStreamToArray(stream)).$then(Bun.concatArrayBuffers);
+  var array = Bun.readableStreamToArray(stream);
+  if ($isPromise(array)) {
+    return array.$then(Bun.concatArrayBuffers);
+  }
+
+  return Bun.concatArrayBuffers(array);
 }
 
 $linkTimeConstant;
-export function readableStreamToJSON(stream) {
+export function readableStreamToJSON(stream: ReadableStream): unknown {
   return Bun.readableStreamToText(stream).$then(globalThis.JSON.parse);
 }
 
 $linkTimeConstant;
-export function readableStreamToBlob(stream) {
+export function readableStreamToBlob(stream: ReadableStream): Promise<Blob> {
   return Promise.resolve(Bun.readableStreamToArray(stream)).$then(array => new Blob(array));
 }
 

diff --git a/src/bun.js/builtins/ts/ReadableStreamInternals.ts b/src/bun.js/builtins/ts/ReadableStreamInternals.ts
@@ -1750,8 +1750,6 @@ export async function readableStreamToArrayDirect(stream, underlyingSource) {
     stream = undefined;
     reader = undefined;
   }
-
-  return capability.$promise;
 }
 
 export function readableStreamDefineLazyIterators(prototype) {

diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig
@@ -1739,25 +1739,11 @@ pub const Arguments = struct {
         }
     };
     pub const Exists = struct {
-        path: PathLike,
+        path: ?PathLike,
 
         pub fn fromJS(ctx: JSC.C.JSContextRef, arguments: *ArgumentsSlice, exception: JSC.C.ExceptionRef) ?Exists {
-            const path = PathLike.fromJS(ctx, arguments, exception) orelse {
-                if (exception.* == null) {
-                    JSC.throwInvalidArguments(
-                        "path must be a string or buffer",
-                        .{},
-                        ctx,
-                        exception,
-                    );
-                }
-                return null;
-            };
-
-            if (exception.* != null) return null;
-
             return Exists{
-                .path = path,
+                .path = PathLike.fromJS(ctx, arguments, exception),
             };
         }
     };
@@ -2673,14 +2659,15 @@ pub const NodeFS = struct {
     }
     pub fn exists(this: *NodeFS, args: Arguments.Exists, comptime flavor: Flavor) Maybe(Return.Exists) {
         const Ret = Maybe(Return.Exists);
-        const path = args.path.sliceZ(&this.sync_error_buf);
         switch (comptime flavor) {
             .sync => {
+                const path = args.path orelse return Ret{ .result = false };
+                const slice = path.sliceZ(&this.sync_error_buf);
                 // access() may not work correctly on NFS file systems with UID
                 // mapping enabled, because UID mapping is done on the server and
                 // hidden from the client, which checks permissions. Similar
                 // problems can occur to FUSE mounts.
-                const rc = (system.access(path, std.os.F_OK));
+                const rc = (system.access(slice, std.os.F_OK));
                 return Ret{ .result = rc == 0 };
             },
             else => {},

diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig
@@ -1129,6 +1129,41 @@ pub const Resolver = struct {
             return .{ .not_found = {} };
         }
 
+        if (strings.hasPrefixComptime(import_path, "file:///")) {
+            const path = import_path[7..];
+
+            if (r.opts.external.abs_paths.count() > 0 and r.opts.external.abs_paths.contains(path)) {
+                // If the string literal in the source text is an absolute path and has
+                // been marked as an external module, mark it as *not* an absolute path.
+                // That way we preserve the literal text in the output and don't generate
+                // a relative path from the output directory to that path.
+                if (r.debug_logs) |*debug| {
+                    debug.addNoteFmt("The path \"{s}\" is marked as external by the user", .{path});
+                }
+
+                return .{
+                    .success = Result{
+                        .path_pair = .{ .primary = Path.init(import_path) },
+                        .is_external = true,
+                    },
+                };
+            }
+
+            if (r.loadAsFile(path, r.extension_order)) |file| {
+                return .{
+                    .success = Result{
+                        .dirname_fd = file.dirname_fd,
+                        .path_pair = .{ .primary = Path.init(file.path) },
+                        .diff_case = file.diff_case,
+                        .file_fd = file.file_fd,
+                        .jsx = r.opts.jsx,
+                    },
+                };
+            }
+
+            return .{ .not_found = {} };
+        }
+
         // Check both relative and package paths for CSS URL tokens, with relative
         // paths taking precedence over package paths to match Webpack behavior.
         const is_package_path = isPackagePath(import_path);

diff --git a/test.ts b/test.ts
diff --git a/test/js/bun/resolve/resolve.test.js → test/js/bun/resolve/resolve.test.ts b/test/js/bun/resolve/resolve.test.js → test/js/bun/resolve/resolve.test.ts
@@ -1,7 +1,7 @@
 import { it, expect } from "bun:test";
 import { mkdirSync, writeFileSync, existsSync, rmSync, copyFileSync } from "fs";
 import { join } from "path";
-import { bunExe, bunEnv } from "harness";
+import { bunExe, bunEnv, tempDirWithFiles } from "harness";
 
 it("spawn test file", () => {
   writePackageJSONImportsFixture();
@@ -85,3 +85,51 @@ function writePackageJSONImportsFixture() {
     ),
   );
 }
+
+it("file url in import resolves", async () => {
+  const dir = tempDirWithFiles("fileurl", {
+    "index.js": "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `import {foo} from 'file://${dir}/index.js';\nconsole.log(foo);`);
+
+  console.log("dir", dir);
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url in await import resolves", async () => {
+  const dir = tempDirWithFiles("fileurl", {
+    "index.js": "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `const {foo} = await import('file://${dir}/index.js');\nconsole.log(foo);`);
+
+  console.log("dir", dir);
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url in require resolves", async () => {
+  const dir = tempDirWithFiles("fileurl", {
+    "index.js": "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `const {foo} = require('file://${dir}/index.js');\nconsole.log(foo);`);
+
+  console.log("dir", dir);
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts
@@ -1159,3 +1159,10 @@ it("repro 1516: can use undefined/null to specify default flag", () => {
   expect(readFileSync(path, { encoding: "utf8", flag: null })).toBe("b");
   rmSync(path);
 });
+
+it("existsSync with invalid path doesn't throw", () => {
+  expect(existsSync(null as any)).toBe(false);
+  expect(existsSync(123 as any)).toBe(false);
+  expect(existsSync(undefined as any)).toBe(false);
+  expect(existsSync({ invalid: 1 } as any)).toBe(false);
+});