Skip to content

Commit

Permalink
Fix napi_is_buffer/napi_is_typedarray to match Node.js (#17034)
Browse files Browse the repository at this point in the history
  • Loading branch information
190n authored Feb 4, 2025
1 parent 0861c03 commit a8d159d
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3529,7 +3529,7 @@ const UnsafeObject = struct {
callframe: *JSC.CallFrame,
) bun.JSError!JSC.JSValue {
const args = callframe.arguments_old(2).slice();
if (args.len < 1 or !args[0].isCell() or !args[0].jsType().isTypedArray()) {
if (args.len < 1 or !args[0].isCell() or !args[0].jsType().isTypedArrayOrArrayBuffer()) {
return globalThis.throwInvalidArguments("Expected an ArrayBuffer", .{});
}

Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/api/html_rewriter.zig
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub const HTMLRewriter = struct {
const kind: ResponseKind = brk: {
if (response_value.isString())
break :brk .string
else if (response_value.jsType().isTypedArray())
else if (response_value.jsType().isTypedArrayOrArrayBuffer())
break :brk .array_buffer
else
break :brk .other;
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3805,7 +3805,7 @@ pub const JSValue = enum(i64) {
};
}

pub fn isTypedArray(this: JSType) bool {
pub fn isTypedArrayOrArrayBuffer(this: JSType) bool {
return switch (this) {
.ArrayBuffer,
.BigInt64Array,
Expand Down
24 changes: 24 additions & 0 deletions src/bun.js/bindings/napi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,30 @@ extern "C" napi_status napi_create_arraybuffer(napi_env env,
NAPI_RETURN_SUCCESS(env);
}

extern "C" napi_status napi_is_buffer(napi_env env, napi_value value, bool* result)
{
NAPI_PREAMBLE(env);
NAPI_CHECK_ARG(env, value);
NAPI_CHECK_ARG(env, result);

auto jsValue = toJS(value);
// Despite documentation, Node.js's version of this function returns true for all kinds of
// TypedArray, not just Uint8Array
*result = jsValue.isCell() && isTypedArrayTypeIncludingDataView(jsValue.asCell()->type());
NAPI_RETURN_SUCCESS(env);
}

extern "C" napi_status napi_is_typedarray(napi_env env, napi_value value, bool* result)
{
NAPI_PREAMBLE(env);
NAPI_CHECK_ARG(env, value);
NAPI_CHECK_ARG(env, result);

auto jsValue = toJS(value);
*result = jsValue.isCell() && isTypedArrayType(jsValue.asCell()->type());
NAPI_RETURN_SUCCESS(env);
}

// This is more efficient than using WTF::String::FromUTF8
// it doesn't copy the string
// but it's only safe to use if we are not setting a property
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/webcore/body.zig
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ pub const Body = struct {
};
}

if (js_type.isTypedArray()) {
if (js_type.isTypedArrayOrArrayBuffer()) {
if (value.asArrayBuffer(globalThis)) |buffer| {
const bytes = buffer.byteSlice();

Expand Down
20 changes: 4 additions & 16 deletions src/napi/napi.zig
Original file line number Diff line number Diff line change
Expand Up @@ -846,13 +846,9 @@ pub export fn napi_get_arraybuffer_info(env: napi_env, arraybuffer_: napi_value,
len.* = slice.len;
return env.ok();
}
pub export fn napi_is_typedarray(env: napi_env, value_: napi_value, result_: ?*bool) napi_status {
log("napi_is_typedarray", .{});
const value = value_.get();
const result = result_ orelse return env.invalidArg();
result.* = value.jsTypeLoose().isTypedArray();
return env.ok();
}

pub extern fn napi_is_typedarray(napi_env, napi_value, *bool) napi_status;

pub export fn napi_create_typedarray(env: napi_env, @"type": napi_typedarray_type, length: usize, arraybuffer_: napi_value, byte_offset: usize, result_: ?*napi_value) napi_status {
log("napi_create_typedarray", .{});
const arraybuffer = arraybuffer_.get();
Expand Down Expand Up @@ -1240,15 +1236,7 @@ pub export fn napi_create_buffer_copy(env: napi_env, length: usize, data: [*]u8,

return env.ok();
}
pub export fn napi_is_buffer(env: napi_env, value_: napi_value, result_: ?*bool) napi_status {
log("napi_is_buffer", .{});
const result = result_ orelse {
return env.invalidArg();
};
const value = value_.get();
result.* = value.isBuffer(env.toJS());
return env.ok();
}
extern fn napi_is_buffer(napi_env, napi_value, *bool) napi_status;
pub export fn napi_get_buffer_info(env: napi_env, value_: napi_value, data: ?*[*]u8, length: ?*usize) napi_status {
log("napi_get_buffer_info", .{});
const value = value_.get();
Expand Down
2 changes: 1 addition & 1 deletion src/sql/postgres/postgres_types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ pub const Tag = enum(short) {
return .timestamptz;
}

if (tag.isTypedArray()) {
if (tag.isTypedArrayOrArrayBuffer()) {
if (tag == .Int32Array)
return .int4_array;

Expand Down
19 changes: 19 additions & 0 deletions test/napi/napi-app/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,22 @@ static napi_value test_extended_error_messages(const Napi::CallbackInfo &info) {
return ok(env);
}

static napi_value test_is_buffer(const Napi::CallbackInfo &info) {
bool result;
napi_env env = info.Env();
NODE_API_CALL(info.Env(), napi_is_buffer(env, info[1], &result));
printf("napi_is_buffer -> %s\n", result ? "true" : "false");
return ok(env);
}

static napi_value test_is_typedarray(const Napi::CallbackInfo &info) {
bool result;
napi_env env = info.Env();
NODE_API_CALL(info.Env(), napi_is_typedarray(env, info[1], &result));
printf("napi_is_typedarray -> %s\n", result ? "true" : "false");
return ok(env);
}

Napi::Value RunCallback(const Napi::CallbackInfo &info) {
Napi::Env env = info.Env();
// this function is invoked without the GC callback
Expand Down Expand Up @@ -1204,6 +1220,9 @@ Napi::Object InitAll(Napi::Env env, Napi::Object exports1) {
Napi::Function::New(env, create_weird_bigints));
exports.Set("test_extended_error_messages",
Napi::Function::New(env, test_extended_error_messages));
exports.Set("test_is_buffer", Napi::Function::New(env, test_is_buffer));
exports.Set("test_is_typedarray",
Napi::Function::New(env, test_is_typedarray));

napitests::register_wrap_tests(env, exports);

Expand Down
21 changes: 21 additions & 0 deletions test/napi/napi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,27 @@ describe("napi", () => {
});
});

describe.each(["buffer", "typedarray"])("napi_is_%s", kind => {
const tests: Array<[string, boolean]> = [
["new Uint8Array()", true],
["new BigUint64Array()", true],
["new ArrayBuffer()", false],
["Buffer.alloc(0)", true],
["new DataView(new ArrayBuffer())", kind == "buffer"],
["new (class Foo extends Uint8Array {})()", true],
["false", false],
["[1, 2, 3]", false],
["'hello'", false],
];
it("returns consistent values with node.js", () => {
for (const [value, expected] of tests) {
// main.js does eval then spread so to pass a single value we need to wrap in an array
const output = checkSameOutput(`test_is_${kind}`, "[" + value + "]");
expect(output).toBe(`napi_is_${kind} -> ${expected.toString()}`);
}
});
});

it.each([
["nullptr", { number: 123 }],
["null", null],
Expand Down

0 comments on commit a8d159d

Please sign in to comment.