fix(napi): napi_typeof returns napi_object for String objects #25365

robobun · 2025-12-06T01:03:42Z

Summary

Fix napi_typeof to return napi_object for boxed String objects (new String("hello")) instead of incorrectly returning napi_string
Add regression test for boxed primitive objects (String, Number, Boolean)

The issue was that StringObjectType and DerivedStringObjectType JSC cell types were falling through to return napi_string, but these represent object wrappers around strings, not primitive strings.

Test plan

bun bd test test/napi/napi.test.ts -t "napi_typeof" passes
Test fails with USE_SYSTEM_BUN=1 (confirming the bug exists in released version)

Fixes #25351

🤖 Generated with Claude Code

napi_typeof was incorrectly returning napi_string for boxed String objects (new String("hello")) when it should return napi_object, matching JavaScript's typeof behavior. The issue was that StringObjectType and DerivedStringObjectType cell types were falling through to return napi_string, but these represent object wrappers around strings, not primitive strings. Fixes #25351 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

robobun · 2025-12-06T01:03:51Z

^{Updated 5:58 PM PT - Dec 5th, 2025}

❌ Your commit d853eb9f has 6 failures in Build #32929 (All Failures):

test/v8/v8.test.ts - 1 failing on 🐧 13 x64-asan
test/integration/next-pages/test/dev-server.test.ts - 1 failing on 🍎 13 aarch64
test/js/node/test/parallel/test-fs-readdir-stack-overflow.js - timeout on 🍎 14 aarch64
test/regression/issue/11297/11297.test.ts - 1 failing on 🪟 2019 x64
test/bake/dev/stress.test.ts - 1 failing on 🪟 2019 x64-baseline
scripts/build.mjs - build failed on 🐧 aarch64 - build-bun

🧪 To try this PR locally:

bunx bun-pr 25365

That installs a local version of the PR into your bun-25365 executable, so you can run:

bun-25365 --bun

coderabbitai · 2025-12-06T01:09:21Z

Walkthrough

This PR fixes napi_typeof to correctly report boxed primitives (String, Number, Boolean objects) as napi_object instead of their primitive types. The fix adds explicit handling in the C++ binding for string object types, and introduces comprehensive tests to validate the behavior.

Changes

Cohort / File(s)	Summary
NAPI typeof type handling `src/bun.js/bindings/napi.cpp`	Added explicit cases for DerivedStringObjectType and StringObjectType in the napi_typeof switch to return napi_object, ensuring boxed strings are classified as objects rather than strings.
Test helper functions `test/napi/napi-app/standalone_tests.cpp`	Added new N-API test function `napi_get_typeof` that wraps napi_typeof calls to expose typeof results as integers for testing purposes.
JavaScript test fixtures `test/napi/napi-app/module.js`	Added `test_napi_typeof_boxed_primitives` function to test boxed primitive type detection; reformatted arrow function signatures; added GC pressure pattern with ArrayBuffer array in existing finalizer test.
Test suite `test/napi/napi.test.ts`	Added regression test under cleanup hooks suite to verify `napi_typeof` correctly identifies boxed primitives (String, Number, Boolean objects) as `napi_object`.

Possibly related PRs

#22459: Refactors napi_typeof and error handling paths in the same napi.cpp file, directly related to typeof behavior fixes.

Suggested reviewers

dylan-conway
Jarred-Sumner
nektro

Pre-merge checks

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and specifically summarizes the main fix: napi_typeof incorrectly returns napi_string for String objects instead of napi_object.
Description check	✅ Passed	The description includes both required sections: 'Summary' explaining what the PR does (the fix for napi_typeof) and 'Test plan' detailing verification steps taken.
Linked Issues check	✅ Passed	The code changes fully address issue #25351: napi_typeof now returns napi_object for boxed String/Number/Boolean primitives, with regression tests confirming correct behavior.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing napi_typeof for boxed primitives: the core fix in napi.cpp, test infrastructure updates, and regression test coverage.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 23383b3 and d853eb9.

📒 Files selected for processing (4)

src/bun.js/bindings/napi.cpp (1 hunks)
test/napi/napi-app/module.js (4 hunks)
test/napi/napi-app/standalone_tests.cpp (2 hunks)
test/napi/napi.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

src/bun.js/bindings/napi.cpp

src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: JavaScript class implementations in C++ should create three classes if there's a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties in C++ JavaScript classes using HashTableValue arrays
Add iso subspaces for C++ JavaScript classes with C++ fields
Cache structures in ZigGlobalObject for C++ JavaScript classes

Files:

src/bun.js/bindings/napi.cpp

test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

test/napi/napi.test.ts
test/napi/napi-app/module.js

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

test/napi/napi.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using -e flag over tempDir
For multi-file tests in Bun test suite, prefer using tempDir and Bun.spawn
Always use port: 0 when spawning servers in tests - do not hardcode ports or use custom random port functions
Use normalizeBunSnapshot to normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Use tempDir from harness to create temporary directories in tests - do not use tmpdirSync or fs.mkdtempSync
In tests, call expect(stdout).toBe(...) before expect(exitCode).toBe(0) when spawning processes for more useful error messages on failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - tests are not valid if they pass with USE_SYSTEM_BUN=1
Avoid shell commands in tests - do not use find or grep; use Bun's Glob and built-in tools instead
Test files must end in .test.ts or .test.tsx and be created in the appropriate test folder structure

Files:

test/napi/napi.test.ts

🧠 Learnings (30)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: When fixing a Node.js compatibility bug, first verify the expected behavior works in Node.js before testing against Bun's canary version

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++

📚 Learning: 2025-11-24T18:37:47.899Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-10-01T21:59:54.571Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations

Applied to files:

src/bun.js/bindings/napi.cpp
test/napi/napi-app/standalone_tests.cpp
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : JavaScript class implementations in C++ should create three classes if there's a public constructor: `Foo` (JSDestructibleObject), `FooPrototype` (JSNonFinalObject), and `FooConstructor` (InternalFunction)

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-10-01T21:49:27.862Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : For objects that need internal fields, extend InternalFieldObject class instead of directly extending Data

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method

Applied to files:

src/bun.js/bindings/napi.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/standalone_tests.cpp
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Tests should be located in the `test/` directory, organized by category: `cli/` for CLI tests, `js/` for JavaScript/TypeScript tests (further subdivided into `bun/`, `node/`, `web/`, `third_party/`), `napi/` for N-API tests, `v8/` for V8 C++ API tests, `bundler/` for bundler/transpiler tests, and `regression/issue/[number]` for regression tests

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/standalone_tests.cpp
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/standalone_tests.cpp
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:35:39.205Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality

Applied to files:

test/napi/napi.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

test/napi/napi.test.ts

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: When fixing a Node.js compatibility bug, first verify the expected behavior works in Node.js before testing against Bun's canary version

Applied to files:

test/napi/napi.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/standalone_tests.cpp
test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes

Applied to files:

test/napi/napi.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

test/napi/napi.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

test/napi/napi.test.ts
test/napi/napi-app/module.js

📚 Learning: 2025-10-19T04:55:33.099Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:36:08.558Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : For reference-counted objects, use `.deref()` in finalize instead of `destroy()` to release references to other JS objects

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `Buffer.alloc(count, fill).toString()` instead of `'A'.repeat(count)` to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: See `test/harness.ts` for common test utilities and helpers

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-10-25T17:20:19.041Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

test/napi/napi-app/module.js

📚 Learning: 2025-11-24T18:35:08.612Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

test/napi/napi-app/module.js

🧬 Code graph analysis (1)

test/napi/napi-app/standalone_tests.cpp (1)

src/bun.js/bindings/napi.cpp (2)

napi_typeof (2370-2460)

napi_typeof (2370-2371)

🔇 Additional comments (6)

src/bun.js/bindings/napi.cpp (1)

2405-2407: Boxed String now correctly reported as napi_object

Mapping DerivedStringObjectType / StringObjectType to napi_object fixes napi_typeof(new String("…")) and brings behavior in line with JS typeof / Node-API expectations, without affecting primitive StringType handling.

test/napi/napi.test.ts (1)

759-777: Good regression coverage for napi_typeof on boxed primitives

The new test cleanly reuses checkSameOutput and then asserts the expected napi_valuetype mappings for primitive string vs String/Number/Boolean objects, which should catch regressions against Node’s behavior.

test/napi/napi-app/standalone_tests.cpp (1)

1828-1856: napi_get_typeof helper is well-structured and safely exported

The helper validates its single argument, forwards to napi_typeof, and returns the napi_valuetype as an int32, with clear failure logging and export via REGISTER_FUNCTION; this is a solid foundation for the JS-side boxed primitive tests.

Also applies to: 1890-1890

test/napi/napi-app/module.js (3)

732-748: BigInt literal change for word-count test is safe and clearer

Switching the test BigInt to 0x123456789abcdef0123456789abcdefn keeps it as a multi-word value (2×64‑bit digits) while making the test intent more explicit, and the subsequent expectations on queriedWordCount / actualWordCount remain valid.

752-769: Improved visibility for napi_reference_unref underflow behavior

The enhanced test_ref_unref_underflow now logs both the first unref count and the second unref status, and asserts 0 and 1 (napi_generic_failure) respectively. This aligns with the N-API contract and will make regressions around refcount underflow easy to spot.

848-896: JS-side boxed primitive typeof test is well-aligned with N-API

test_napi_typeof_boxed_primitives:

Uses the new native napi_get_typeof helper to obtain raw napi_valuetype values.

Asserts:

primitive string → napi_string (4),

new String("…") / new Number(…) / new Boolean(…) → napi_object (6),

Logs clear PASS messages that are then checked in test/napi/napi.test.ts.

Relying on the documented numeric values of napi_string and napi_object is reasonable here and keeps the test concise.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions bot added the claude label Dec 6, 2025

Jarred-Sumner merged commit 6ce419d into main Dec 6, 2025
52 of 57 checks passed

Jarred-Sumner deleted the claude/fix-napi-typeof-string-object branch December 6, 2025 02:27

kjvalencik mentioned this pull request Dec 9, 2025

Consider a String object an object instead of string in node-api #25353

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(napi): napi_typeof returns napi_object for String objects #25365

fix(napi): napi_typeof returns napi_object for String objects #25365

robobun commented Dec 6, 2025

Uh oh!

robobun commented Dec 6, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(napi): napi_typeof returns napi_object for String objects #25365

fix(napi): napi_typeof returns napi_object for String objects #25365

Conversation

robobun commented Dec 6, 2025

Summary

Test plan

Uh oh!

robobun commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Dec 6, 2025

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Pre-merge checks

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robobun commented Dec 6, 2025 •

edited

Loading