Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

random fixes that help vite/sveltekit #3140

Merged
merged 7 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -323,7 +323,7 @@ declare module "bun" {
*
*/
export function readableStreamToArray<T>(
stream: ReadableStream,
stream: ReadableStream<T>,
): Promise<T[]> | T[];

/**
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/builtins/WebCoreJSBuiltins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions src/bun.js/builtins/builtins.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 11 additions & 6 deletions src/bun.js/builtins/ts/ReadableStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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));
}

Expand Down
2 changes: 0 additions & 2 deletions src/bun.js/builtins/ts/ReadableStreamInternals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1750,8 +1750,6 @@ export async function readableStreamToArrayDirect(stream, underlyingSource) {
stream = undefined;
reader = undefined;
}

return capability.$promise;
}

export function readableStreamDefineLazyIterators(prototype) {
Expand Down
4 changes: 1 addition & 3 deletions src/bun.js/events.exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ function EventEmitter(opts) {
}

this._maxListeners ??= undefined;
if (
(this[kCapture] = opts?.captureRejections ? Boolean(opts?.captureRejections) : EventEmitterPrototype[kCapture])
) {
if ((this[kCapture] = opts?.captureRejections ? Boolean(opts?.captureRejections) : EventEmitterPrototype[kCapture])) {
this.emit = emitWithRejectionCapture;
}
}
Expand Down
23 changes: 5 additions & 18 deletions src/bun.js/node/node_fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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),
paperdave marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this fix, it should be handled earlier in the function instead of these extra null checks.

But it does fix the issue. Please add a TODO note to address this.

};
}
};
Expand Down Expand Up @@ -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 => {},
Expand Down
35 changes: 35 additions & 0 deletions src/resolver/resolver.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,41 @@ pub const Resolver = struct {
return .{ .not_found = {} };
}

if (strings.hasPrefixComptime(import_path, "file:///")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't handle space characters or anything percent-encoded

but it is better than status quo

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);
Expand Down
2 changes: 0 additions & 2 deletions test.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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");
});
7 changes: 7 additions & 0 deletions test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});