feat(napi): add node_api_create_object_with_properties support for enum creation#2990
Conversation
How to use the Graphite Merge QueueAdd the label ready-to-merge to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
node_api_create_object_with_properties
Brooooooklyn
force-pushed
the
copilot/fix-34149896-131389850-6e6b5c74-c4e5-4900-b576-f875918ca1ea
branch
from
bacc5f8 to
11abd6a
Compare
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
Brooooooklyn
force-pushed
the
copilot/fix-34149896-131389850-6e6b5c74-c4e5-4900-b576-f875918ca1ea
branch
from
11abd6a to
4e7ffc1
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Brooooooklyn
changed the title
Brooooooklyn
changed the title
Brooooooklyn
deleted the
copilot/fix-34149896-131389850-6e6b5c74-c4e5-4900-b576-f875918ca1ea
branch
…16403) NAPI-RS 3.6.0 makes a change to the ordering of objects. It now puts optional fields last. This made tests in `napi/parser` for module record fail when we tried to update (#16383) because the `module_request` field of `ExportEntry` moves to last which breaks the snapshots. The change in NAPI-RS is unlikely to be reverted, because it's a sizeable perf optimization: napi-rs/napi-rs#2990 To work around this problem: 1. Move the field in the `ExportEntry` intermediate struct in `napi/parser` to last, so NAPI's output matches what you'd expect from the struct definition. 2. Alter the `#[estree]` attr on `ExportEntry` struct in `oxc_syntax` crate to match. Note: The `ExportEntry` struct in `napi/parser` is just an intermediate structure used for serialization. So I think it's fine to fiddle with its field order. The actual `ExportEntry` struct used in the module record is in `oxc_syntax` crate, and it remains unaltered.
…xc-project#16403) NAPI-RS 3.6.0 makes a change to the ordering of objects. It now puts optional fields last. This made tests in `napi/parser` for module record fail when we tried to update (oxc-project#16383) because the `module_request` field of `ExportEntry` moves to last which breaks the snapshots. The change in NAPI-RS is unlikely to be reverted, because it's a sizeable perf optimization: napi-rs/napi-rs#2990 To work around this problem: 1. Move the field in the `ExportEntry` intermediate struct in `napi/parser` to last, so NAPI's output matches what you'd expect from the struct definition. 2. Alter the `#[estree]` attr on `ExportEntry` struct in `oxc_syntax` crate to match. Note: The `ExportEntry` struct in `napi/parser` is just an intermediate structure used for serialization. So I think it's fine to fiddle with its field order. The actual `ExportEntry` struct used in the module record is in `oxc_syntax` crate, and it remains unaltered.
Node.js 22 introduced
node_api_create_object_with_propertieswhich creates objects with properties in a single FFI call instead of N+1 calls (one to create, N to set properties). This reduces overhead when creating enum objects.Changes
FFI Binding (
crates/sys/src/functions.rs)node_api_create_object_with_propertiesdeclaration undernapi10featureRuntime Helper (
crates/napi/src/bindgen_runtime/mod.rs)create_object_with_propertieshelper that uses optimized API when:napi10+node_version_detect+dyn-symbolsfeatures enablednapi_create_object+napi_set_named_propertyotherwisedyn-symbolsto avoid linking against unavailable symbols on older Node.js versionsCode Generation (
crates/backend/src/codegen/enum.rs)Example
For an enum with 3 variants:
Before: 4 FFI calls
After (Node.js 22+): 1 FFI call
Addresses: nodejs/node#45905
Original prompt
node_api_create_object_with_properties#2985💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.