feat: implement V8 Value type checking APIs#22462
Conversation
Adds V8 C++ API compatibility for: - v8::Value::IsMap() - checks if value is a Map - v8::Value::IsArray() - checks if value is an Array - v8::Value::IsInt32() - checks if value is a 32-bit integer - v8::Value::IsBigInt() - checks if value is a BigInt These APIs are commonly used by native Node.js modules and improve compatibility with the V8 API surface. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds four new v8::Value type-check methods (IsMap, IsArray, IsInt32, IsBigInt), exports their symbols via Zig and symbol lists (with platform-specific manglings), and adds native and TypeScript tests that exercise these predicates. Changes
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/bun.js/bindings/v8/V8Value.cpp (1)
73-81: IsArray uses the correct cross-realm check.JSC::isArray(defaultGlobalObject(), value) is the right call; the pre-check is fine but redundant as isArray would return false for non-objects. Optional micro-simplification below.
- if (!value.isObject()) { - return false; - } - return JSC::isArray(defaultGlobalObject(), value); + return JSC::isArray(defaultGlobalObject(), value);test/v8/v8.test.ts (1)
137-183: Reduce repetition with data-driven loops.Same assertion pattern repeated many times; switch to arrays/loops to cut boilerplate and speed up edits.
- describe("Value type checks", () => { - it("correctly identifies Maps", async () => { - await checkSameOutput("test_v8_value_type_checks", [new Map()]); - await checkSameOutput("test_v8_value_type_checks", [ - new Map([ - [1, "a"], - [2, "b"], - ]), - ]); - }); - it("correctly identifies Arrays", async () => { - await checkSameOutput("test_v8_value_type_checks", [[]]); - await checkSameOutput("test_v8_value_type_checks", [[1, 2, 3]]); - }); - it("correctly identifies Int32 values", async () => { - await checkSameOutput("test_v8_value_type_checks", [0]); - await checkSameOutput("test_v8_value_type_checks", [42]); - await checkSameOutput("test_v8_value_type_checks", [-42]); - await checkSameOutput("test_v8_value_type_checks", [2147483647]); // INT32_MAX - await checkSameOutput("test_v8_value_type_checks", [-2147483648]); // INT32_MIN - }); - it("correctly identifies BigInt values", async () => { - await checkSameOutput("test_v8_value_type_checks", [0n]); - await checkSameOutput("test_v8_value_type_checks", [123n]); - await checkSameOutput("test_v8_value_type_checks", [-123n]); - await checkSameOutput("test_v8_value_type_checks", [99999999999999999999999999999999n]); - }); - it("correctly identifies non-Int32 numbers", async () => { - await checkSameOutput("test_v8_value_type_checks", [3.14]); - await checkSameOutput("test_v8_value_type_checks", [2147483648]); // INT32_MAX + 1 - await checkSameOutput("test_v8_value_type_checks", [Number.MAX_SAFE_INTEGER]); - await checkSameOutput("test_v8_value_type_checks", [Infinity]); - await checkSameOutput("test_v8_value_type_checks", [NaN]); - }); - it("correctly identifies other types", async () => { - await checkSameOutput("test_v8_value_type_checks", ["string"]); - await checkSameOutput("test_v8_value_type_checks", [true]); - await checkSameOutput("test_v8_value_type_checks", [false]); - await checkSameOutput("test_v8_value_type_checks", [null]); - await checkSameOutput("test_v8_value_type_checks", [undefined]); - await checkSameOutput("test_v8_value_type_checks", [{}]); - await checkSameOutput("test_v8_value_type_checks", [new Date()]); - await checkSameOutput("test_v8_value_type_checks", [/regex/]); - await checkSameOutput("test_v8_value_type_checks", [new Set()]); - await checkSameOutput("test_v8_value_type_checks", [() => {}]); - }); - }); + describe("Value type checks", () => { + const cases = { + maps: [new Map(), new Map([[1, "a"], [2, "b"]])], + arrays: [[], [1, 2, 3]], + int32s: [0, 42, -42, 2147483647, -2147483648], + bigints: [0n, 123n, -123n, 99999999999999999999999999999999n], + notInt32Numbers: [3.14, 2147483648, Number.MAX_SAFE_INTEGER, Infinity, NaN], + others: ["string", true, false, null, undefined, {}, new Date(), /regex/, new Set(), () => {}], + }; + for (const [name, values] of Object.entries(cases)) { + it(`correctly identifies ${name}`, async () => { + for (const v of values) { + await checkSameOutput("test_v8_value_type_checks", [v]); + } + }); + } + });test/v8/v8-module/main.cpp (1)
1117-1142: Silence unused local warning.Local context is currently unused; either remove or cast-to-void to avoid warnings in stricter builds.
- Local<Context> context = isolate->GetCurrentContext(); + Local<Context> context = isolate->GetCurrentContext(); + (void)context;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/bun.js/bindings/v8/V8Value.cpp(2 hunks)src/bun.js/bindings/v8/V8Value.h(1 hunks)src/napi/napi.zig(2 hunks)src/symbols.dyn(1 hunks)src/symbols.txt(1 hunks)test/v8/v8-module/main.cpp(2 hunks)test/v8/v8.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{cpp,h}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible
Files:
src/bun.js/bindings/v8/V8Value.hsrc/bun.js/bindings/v8/V8Value.cpptest/v8/v8-module/main.cpp
**/*.{cpp,hpp,h,cc,cxx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run clang-format to format C/C++ files (bun run clang-format)
Files:
src/bun.js/bindings/v8/V8Value.hsrc/bun.js/bindings/v8/V8Value.cpptest/v8/v8-module/main.cpp
**/*.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Files:
src/bun.js/bindings/v8/V8Value.cpptest/v8/v8-module/main.cpp
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: When implementing a JS class with a public constructor in C++ bindings, create Foo, FooPrototype, and FooConstructor classes
Define JS-visible properties using HashTableValue arrays
Add iso subspaces for classes with C++ fields
Cache class structures in ZigGlobalObject for bindings
Files:
src/bun.js/bindings/v8/V8Value.cpp
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/v8/v8-module/main.cpptest/v8/v8.test.ts
test/v8/**/*
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place V8 C++ API tests under test/v8/
Files:
test/v8/v8-module/main.cpptest/v8/v8.test.ts
test/v8/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place V8 C++ API compatibility tests under test/v8/
Files:
test/v8/v8-module/main.cpptest/v8/v8.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/v8/v8.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Tests must be named with the suffix .test.ts or .test.tsx and live under the test/ directory
In tests, always use port: 0 and never hardcode ports or use custom random-port utilities
Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr
Do not write tests that assert absence of 'panic', 'uncaught exception', or similar strings in output
Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Files:
test/v8/v8.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Run Prettier to format JS/TS files (bun run prettier)
Files:
test/v8/v8.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/v8/v8.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup
Files:
src/napi/napi.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValueRun zig-format to format Zig files (bun run zig-format)
Files:
src/napi/napi.zig
🧠 Learnings (8)
📚 Learning: 2025-09-03T17:09:28.113Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define JS-visible properties using HashTableValue arrays
Applied to files:
src/bun.js/bindings/v8/V8Value.hsrc/bun.js/bindings/v8/V8Value.cpp
📚 Learning: 2025-09-03T17:09:28.113Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields
Applied to files:
src/bun.js/bindings/v8/V8Value.h
📚 Learning: 2025-09-05T18:44:43.223Z
Learnt from: markovejnovic
PR: oven-sh/bun#21728
File: src/bun.js/bindings/bindings.cpp:6429-6432
Timestamp: 2025-09-05T18:44:43.223Z
Learning: The JSC::JSMap::size() method in JavaScriptCore returns a value that can be represented as uint32_t, and the binding functions in src/bun.js/bindings/bindings.cpp correctly use uint32_t as the return type for JSC__JSMap__size. JavaScript Maps are practically limited to 2^32 - 1 elements.
Applied to files:
src/bun.js/bindings/v8/V8Value.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Applied to files:
src/bun.js/bindings/v8/V8Value.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Provide an extern "C" Bun__<Type>__toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Applied to files:
src/bun.js/bindings/v8/V8Value.cpp
📚 Learning: 2025-09-03T17:09:28.113Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to test/v8/** : Place V8 C++ API compatibility tests under test/v8/
Applied to files:
test/v8/v8-module/main.cpptest/v8/v8.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/v8/**/* : Place V8 C++ API tests under test/v8/
Applied to files:
test/v8/v8.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/node/**/*.{js,ts} : Place Node.js module compatibility tests under test/js/node/, separated by module (e.g., test/js/node/assert/)
Applied to files:
test/v8/v8.test.ts
🔇 Additional comments (11)
src/bun.js/bindings/v8/V8Value.h (1)
17-20: API surface addition looks correct.Signatures and placement match existing predicates. No header ordering or visibility issues spotted.
src/bun.js/bindings/v8/V8Value.cpp (4)
4-6: Needed JSC includes added.These headers are appropriate for inherits(), isArray, and isBigInt.
67-71: IsMap mapping LGTM.Using isCell() && inherits() matches V8::Value::IsMap semantics.
82-86: IsInt32 mapping LGTM.localToJSValue().isInt32() matches V8 expectations for 32-bit integer numbers.
87-91: IsBigInt mapping LGTM.Direct isBigInt() is correct.
src/symbols.dyn (1)
53-56: Symbol exports updated in the right spot.New predicates are listed immediately after IsFunction, matching the layout used elsewhere.
src/symbols.txt (1)
52-55: Static symbol list aligned with dyn exports.Entries for IsMap/IsArray/IsInt32/IsBigInt added consistently.
test/v8/v8-module/main.cpp (1)
1195-1196: Export wiring LGTM.Binding is correctly added to exports.
src/napi/napi.zig (3)
1875-1878: POSIX V8API DCE anchors added.New externs match mangled names and will satisfy link/DCE. No runtime call sites required.
1960-1963: Windows V8API DCE anchors added.MSVC manglings look correct and are covered by fixDeadCodeElimination().
1800-1882: Quick consistency check across symbol surfaces.Ensure the four new symbols appear in both src/symbols.{dyn,txt} and in both V8API branches here (POSIX/Windows).
Run this script from repo root:
#!/usr/bin/env bash set -euo pipefail need=(__ZNK2v85Value5IsMapEv __ZNK2v85Value7IsArrayEv __ZNK2v85Value7IsInt32Ev __ZNK2v85Value8IsBigIntEv \ '?IsMap@Value@v8@@QEBA_NXZ' '?IsArray@Value@v8@@QEBA_NXZ' '?IsInt32@Value@v8@@QEBA_NXZ' '?IsBigInt@Value@v8@@QEBA_NXZ') files=(src/symbols.dyn src/symbols.txt src/napi/napi.zig) for sym in "${need[@]}"; do echo "Checking ${sym}" rg -n --fixed-strings "${sym}" "${files[@]}" done echo "OK if every symbol emitted at least one line above."Also applies to: 1883-1966
The tests were calling checkSameOutput() too many times per test case, causing 30+ subprocess spawns which led to timeouts. Consolidated to test representative values only, matching the pattern of other tests in the file. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/v8/v8.test.ts (2)
137-149: Reduce duplication with a small table of cases; add key int32 boundaries.This keeps intent clear, cuts repetition, and exercises edge values for IsInt32 without being noisy.
- describe("Value type checks", () => { - it("correctly identifies value types with IsMap, IsArray, IsInt32, and IsBigInt", async () => { - // Test a representative sample of each type, not every possible value - // The C++ test function will be called once with each value - await checkSameOutput("test_v8_value_type_checks", [new Map()]); - await checkSameOutput("test_v8_value_type_checks", [[]]); - await checkSameOutput("test_v8_value_type_checks", [42]); - await checkSameOutput("test_v8_value_type_checks", [123n]); - await checkSameOutput("test_v8_value_type_checks", [3.14]); - await checkSameOutput("test_v8_value_type_checks", ["string"]); - await checkSameOutput("test_v8_value_type_checks", [{}]); - }); - }); + describe("Value type checks", () => { + it("matches Node for IsMap/IsArray/IsInt32/IsBigInt on representative values", async () => { + // Each entry is an argument list; we wrap each value so the addon receives it as info[0]. + const cases: any[][] = [ + [new Map()], + [[]], + [42], + [2147483647], // INT32_MAX + [2147483648], // INT32_MAX + 1 (should not be Int32) + [-2147483648], // INT32_MIN + [-2147483649], // INT32_MIN - 1 (should not be Int32) + [123n], + [3.14], + ["string"], + [{}], + ]; + for (const args of cases) { + await checkSameOutput("test_v8_value_type_checks", args); + } + }); + });
137-149: Scope the assertion to the four new predicates to reduce brittleness.Right now we diff the whole stdout, which can fail due to unrelated predicates the native helper also prints. Consider parsing the returned
nodeResultfromcheckSameOutput(...)and asserting equality only for lines starting with IsMap/IsArray/IsInt32/IsBigInt.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/v8/v8.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/v8/v8.test.ts
test/v8/**/*
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place V8 C++ API tests under test/v8/
Files:
test/v8/v8.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/v8/v8.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Tests must be named with the suffix .test.ts or .test.tsx and live under the test/ directory
In tests, always use port: 0 and never hardcode ports or use custom random-port utilities
Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr
Do not write tests that assert absence of 'panic', 'uncaught exception', or similar strings in output
Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Files:
test/v8/v8.test.ts
test/v8/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place V8 C++ API compatibility tests under test/v8/
Files:
test/v8/v8.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Run Prettier to format JS/TS files (bun run prettier)
Files:
test/v8/v8.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/v8/v8.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-03T17:09:28.113Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to test/v8/** : Place V8 C++ API compatibility tests under test/v8/
Applied to files:
test/v8/v8.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/v8/**/* : Place V8 C++ API tests under test/v8/
Applied to files:
test/v8/v8.test.ts
🔇 Additional comments (1)
test/v8/v8.test.ts (1)
137-149: Can the native helper accept multiple args? Consolidate calls if yes.If
test_v8_value_type_checks(...)iteratesinfo.Length()you can pass all values at once to cut process spawns.Run to confirm helper semantics:
#!/bin/bash # Inspect the native helper to see if it loops over all args or only uses info[0]. fd -t f -g 'main.cpp' test | xargs -I{} rg -n -C3 'test_v8_value_type_checks|FunctionCallbackInfo<\s*Value\s*>' {}If it supports multiple args, replace the loop with a single call:
await checkSameOutput("test_v8_value_type_checks", [new Map(), [], 42, 2147483647, 2147483648, -2147483648, -2147483649, 123n, 3.14, "string", {}]);
The test was failing because JSON.stringify cannot serialize BigInt values. Added custom serialization/deserialization that encodes BigInt values as objects with a __bigint__ marker. Also removed unused variable in main.cpp to clean up compiler warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Use a table of test cases to reduce duplication - Add INT32_MAX and INT32_MIN boundary tests - Test values just outside int32 range to verify edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
### What does this PR do? - fixes both functions returning false for double-encoded values (even if the numeric value is a valid int32/uint32) - fixes IsUint32() returning false for values that don't fit in int32 - fixes the test from #22462 not testing anything (the native functions were being passed a callback to run garbage collection as the first argument, so it was only ever testing what the type check APIs returned for that function) - extends the test to cover the first edge case above ### How did you verify your code works? The new tests fail without these fixes. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
Summary
This PR implements four V8 C++ API methods for type checking that are commonly used by native Node.js modules:
v8::Value::IsMap()- checks if value is a Mapv8::Value::IsArray()- checks if value is an Arrayv8::Value::IsInt32()- checks if value is a 32-bit integerv8::Value::IsBigInt()- checks if value is a BigIntImplementation Details
The implementation maps V8's type checking APIs to JavaScriptCore's equivalent functionality:
IsMap()uses JSC'sinherits<JSC::JSMap>()checkIsArray()uses JSC'sisArray()function with the global objectIsInt32()uses JSC'sisInt32()methodIsBigInt()uses JSC'sisBigInt()methodChanges
V8Value.hV8Value.cppnapi.zig(both Unix and Windows mangled names)symbols.txtandsymbols.dynv8-module/main.cppandv8.test.tsTesting
The implementation has been verified to:
Tests cover various value types including empty and populated Maps/Arrays, different numeric ranges, BigInts, and other JavaScript types.
🤖 Generated with Claude Code