fix(ext/node): add node_api_create_property_key_(latin1/utf8)#32559
Merged
bartlomieju merged 9 commits intomainfrom Mar 10, 2026
Merged
fix(ext/node): add node_api_create_property_key_(latin1/utf8)#32559bartlomieju merged 9 commits intomainfrom
node_api_create_property_key_(latin1/utf8)#32559bartlomieju merged 9 commits intomainfrom
Conversation
Adds implementations for node_api_create_property_key_latin1 and node_api_create_property_key_utf8 to complement the existing node_api_create_property_key_utf16 function. These functions create internalized V8 strings optimized for use as object property keys. Co-authored-by: bartlomieju <13602871+bartlomieju@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Support node api create property key optimizations
feat: support node_api_create_property_key_(latin1/utf8)
Mar 7, 2026
Member
CI Status SummaryTwo issues remain: 1. Formatting
2. Windows linker failure — unresolved externalsThe Root cause: On Windows, Likely fix: Update There's also a local fix for |
86847c3 to
f645b32
Compare
bartlomieju
added a commit
that referenced
this pull request
Mar 8, 2026
…i_sys crate Fork napi-sys into libs/napi_sys to avoid build-time dependency on Node.js binaries (required by napi-sys v3.x) and allow adding new Node-API symbols directly in-tree. This unblocks PRs that need newer symbols: - #32559 (node_api_create_property_key_latin1/utf8) - #31443 (node_api_create_object_with_properties) All napi version feature gates have been removed — every symbol is always available. The napi-build dependency is also eliminated by inlining the macOS linker flags into tests/napi/build.rs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
bartlomieju
added a commit
that referenced
this pull request
Mar 9, 2026
…#32582) ## Summary - Forks `napi-sys` (v2.2.2) into `libs/napi_sys/` to avoid build-time dependency on Node.js binaries (required by `napi-sys` v3.x) and allow adding new Node-API symbols directly in-tree - Removes all napi version feature gates — every symbol is always available - Eliminates `napi-build` dependency by inlining the macOS linker flags into `tests/napi/build.rs` - Adds new symbols that are needed by blocked PRs: - `node_api_create_property_key_latin1`, `node_api_create_property_key_utf8` (#32559) - `node_api_create_object_with_properties`, `node_api_create_object_with_named_properties` (#31443) - `node_api_symbol_for`, `node_api_create_property_key_utf16` This unblocks #32559 and #31443 which currently can't build because the symbols they need aren't available in `napi-sys` v2.2.2, and upgrading to v3.x introduces a requirement for Node.js binaries during build. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…api-create-property-key
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…1,utf8} Restores the napi_sys function definitions that match the implementations added in this PR. Removes unimplemented symbols (node_api_create_object_with_*) to avoid GetProcAddress failures on Windows debug builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kajukitli
approved these changes
Mar 10, 2026
Contributor
kajukitli
left a comment
There was a problem hiding this comment.
lgtm — follows the same pattern as existing node_api_create_property_key_utf16
latin1usesnew_from_one_byte+Internalizedutf8usesnew_from_utf8+Internalized- both handle
NAPI_AUTO_LENGTHcorrectly - tests verify interoperability: property set with internalized key, retrieved with normal string key
Remove redundant `unsafe extern "C"` declarations for
node_api_create_property_key_{latin1,utf8,utf16} in the test NAPI
module. These symbols are already exported by the napi_sys crate via
the `generate!` macro. The raw extern declarations caused LNK2019
unresolved symbol errors on Windows, where napi_sys uses runtime
loading (libloading/GetProcAddress) rather than static linking.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
node_api_create_property_key_(latin1/utf8)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
node_api_create_property_key_latin1andnode_api_create_property_key_utf8to complement the existingnode_api_create_property_key_utf16. These create internalized V8strings for use as object property keys, enabling deduplication and
faster property lookups compared to
napi_create_string_*(which usev8::NewStringType::Normal).ext/napi/js_native_api.rs— Addednode_api_create_property_key_latin1(
new_from_one_byte+Internalized) andnode_api_create_property_key_utf8(
new_from_utf8+Internalized), following the same pattern as the existing_utf16variantsymbol_exports.jsonand allthree platform
.deffiles (Linux, macOS, Windows)property keys in each encoding, set them on objects, and verify retrieval via
standard string keys
Fixes #31434