chore: bumpup napi to v3 #70
Conversation
WalkthroughUpgraded N-API dependencies and build config, migrated package.json N-API fields to new schema, updated dev CLI version, and changed the Rust generator public API to accept owned Strings, with a local conversion to &str for a downstream function. Core generation flow remains the same. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Generator as generator::generate
participant Bindgen as internal bindgen
participant RustToTS as rust_to_ts
Client->>Generator: generate(headers: Vec<String>, dump, clang_args: Option<Vec<String>>, entry_types: Option<Vec<String>>)
activate Generator
note right of Generator: Prepare inputs
Generator->>Bindgen: run(headers: Vec<String>, clang_args: Option<Vec<String>>)
Bindgen-->>Generator: Rust code string
note over Generator,RustToTS: Convert entry_types: Vec<String> -> Vec<&str>
Generator->>RustToTS: rust_to_ts(entry_types: Vec<&str>)
RustToTS-->>Generator: TS defs string
Generator-->>Client: Result<String> (output)
deactivate Generator
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
=======================================
Coverage 86.83% 86.83%
=======================================
Files 9 9
Lines 509 509
Branches 112 112
=======================================
Hits 442 442
Misses 67 67 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/generator/Cargo.toml (1)
22-24: [profile.release] here is ignored in a workspace; move to the root Cargo.toml.The CI warning indicates these settings won’t apply. Define
ltoandstripat the workspace root instead.-# packages/generator/Cargo.toml -[profile.release] -lto = true -strip = "symbols" +# Root Cargo.toml (workspace) +[profile.release] +lto = true +strip = "symbols"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/generator/Cargo.toml(1 hunks)packages/generator/package.json(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/generator/Cargo.toml
[warning] 1-1: Warning: profiles for the non-root package will be ignored; specify profiles at the workspace root.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build aarch64-pc-windows-msvc
- GitHub Check: build / Build aarch64-unknown-linux-musl
- GitHub Check: build / Build i686-pc-windows-msvc
- GitHub Check: build / Build x86_64-pc-windows-msvc
🔇 Additional comments (1)
packages/generator/Cargo.toml (1)
11-12: napi v3 bump — confirm N‑API baseline and CI MSRVFile: packages/generator/Cargo.toml (lines 11–12)
- Verified: napi-rs v3 exposes feature flags napi1..napi10; features = ["napi4"] pins the crate to N‑API v4.
- Verified: napi-rs v3 MSRV = Rust 1.80.0 — ensure CI/toolchains use Rust ≥ 1.80.0 or lower the N‑API baseline if you intend broader runtime support.
| "binaryName": "generator", | ||
| "targets": [ | ||
| "aarch64-apple-darwin", | ||
| "aarch64-unknown-linux-gnu", | ||
| "aarch64-unknown-linux-musl", | ||
| "aarch64-pc-windows-msvc", | ||
| "armv7-unknown-linux-gnueabihf", | ||
| "x86_64-unknown-linux-musl", | ||
| "i686-pc-windows-msvc" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
Targets/optionalDependencies mismatch will skip key artifacts (darwin-x64, linux-x64-gnu, win32-x64).
napi.targets lacks:
- x86_64-apple-darwin
- x86_64-unknown-linux-gnu
- x86_64-pc-windows-msvc
but these platforms exist in optionalDependencies. Add the missing triples or drop the corresponding optional deps to avoid broken installs.
Apply this diff to include the missing targets:
"napi": {
"binaryName": "generator",
"targets": [
+ "x86_64-apple-darwin",
"aarch64-apple-darwin",
"aarch64-unknown-linux-gnu",
"aarch64-unknown-linux-musl",
"aarch64-pc-windows-msvc",
"armv7-unknown-linux-gnueabihf",
+ "x86_64-unknown-linux-gnu",
"x86_64-unknown-linux-musl",
+ "x86_64-pc-windows-msvc",
"i686-pc-windows-msvc"
]
},🏁 Script executed:
#!/bin/bash
# Verify required targets exist for declared optionalDependencies
pkg=packages/generator/package.json
echo "Current targets:"; jq -r '.napi.targets[]' "$pkg" | sort
echo
echo "Checking for missing targets..."
missing=0
for t in x86_64-apple-darwin x86_64-unknown-linux-gnu x86_64-pc-windows-msvc; do
if ! jq -re --arg t "$t" '.napi.targets[] | select(.==$t)' "$pkg" >/dev/null; then
echo "MISSING: $t"
missing=1
fi
done
exit $missingLength of output: 344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pkg=packages/generator/package.json
if [ ! -f "$pkg" ]; then
echo "MISSING FILE: $pkg"
exit 2
fi
echo "optionalDependencies keys:"
jq -r '.optionalDependencies // {} | keys[]' "$pkg" | sort || true
echo
echo "napi.targets:"
jq -r '.napi.targets[]' "$pkg" | sort || true
echo
check_triples=( "x86_64-apple-darwin" "x86_64-unknown-linux-gnu" "x86_64-pc-windows-msvc" )
for t in "${check_triples[@]}"; do
present=$(jq -r --arg t "$t" 'any(.napi.targets[]; . == $t) // false' "$pkg")
if [ "$present" = "true" ]; then
echo "PRESENT: $t"
else
echo "MISSING: $t"
fi
done
echo
echo "Optional deps with platform-like names:"
jq -r '.optionalDependencies // {} | keys[]' "$pkg" | grep -E 'x86_64|darwin|linux|windows|win32' || trueLength of output: 1161
Add missing x86_64 napi.targets to match optionalDependencies
napi.targets is missing x86_64 triples while optionalDependencies include x64 artifacts — installs will skip darwin-x64, linux-x64-gnu, and win32-x64.
"napi": {
"binaryName": "generator",
"targets": [
+ "x86_64-apple-darwin",
"aarch64-apple-darwin",
"aarch64-unknown-linux-gnu",
"aarch64-unknown-linux-musl",
"aarch64-pc-windows-msvc",
"armv7-unknown-linux-gnueabihf",
+ "x86_64-unknown-linux-gnu",
"x86_64-unknown-linux-musl",
+ "x86_64-pc-windows-msvc",
"i686-pc-windows-msvc"
]
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "binaryName": "generator", | |
| "targets": [ | |
| "aarch64-apple-darwin", | |
| "aarch64-unknown-linux-gnu", | |
| "aarch64-unknown-linux-musl", | |
| "aarch64-pc-windows-msvc", | |
| "armv7-unknown-linux-gnueabihf", | |
| "x86_64-unknown-linux-musl", | |
| "i686-pc-windows-msvc" | |
| ] | |
| "binaryName": "generator", | |
| "targets": [ | |
| "x86_64-apple-darwin", | |
| "aarch64-apple-darwin", | |
| "aarch64-unknown-linux-gnu", | |
| "aarch64-unknown-linux-musl", | |
| "aarch64-pc-windows-msvc", | |
| "armv7-unknown-linux-gnueabihf", | |
| "x86_64-unknown-linux-gnu", | |
| "x86_64-unknown-linux-musl", | |
| "x86_64-pc-windows-msvc", | |
| "i686-pc-windows-msvc" | |
| ] |
🤖 Prompt for AI Agents
In packages/generator/package.json around lines 18 to 27, the napi.targets array
omits x86_64 triples so x64 optionalDependencies (darwin-x64, linux-x64-gnu,
win32-x64) are skipped; add the corresponding targets to the array —
x86_64-apple-darwin, x86_64-unknown-linux-gnu, and x86_64-pc-windows-msvc —
alongside the existing arm/aarch64/i686 entries so installs include the x64
artifacts.
| }, | ||
| "devDependencies": { | ||
| "@napi-rs/cli": "^2.18.4" | ||
| "@napi-rs/cli": "^3.2.0" |
There was a problem hiding this comment.
🧩 Analysis chain
CLI v3 upgrade: align Node engines with supported versions.
@napi-rs/cli@^3.2.0 typically requires newer Node; engines.node is still >= 10. Bump engines accordingly after confirming CLI v3 support matrix.
Proposed change (adjust the version per verification):
"engines": {
- "node": ">= 10"
+ "node": ">= 16"
},🌐 Web query:
What Node.js versions are supported by @napi-rs/cli v3.x, and what minimum Node version is recommended?
💡 Result:
- Supported: Node.js 16 and newer (e.g. 16, 18, 20, 22). [1][2]
- Recommended minimum: Node.js >= 16 (v3.x dropped support for older Node versions). [1]
Sources: changelog noting "requires node >= 16" for v3, and napi-rs docs showing Node support matrix. [1][2]
Bump engines.node to >=16 to match @napi-rs/cli v3 requirement.
Confirmed: @napi-rs/cli v3.x requires Node.js >=16 — update packages/generator/package.json (line ~41).
Proposed change:
"engines": {
- "node": ">= 10"
+ "node": ">= 16"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@napi-rs/cli": "^3.2.0" | |
| "engines": { | |
| "node": ">= 16" | |
| }, |
🤖 Prompt for AI Agents
In packages/generator/package.json around line 41, the project currently lists
"@napi-rs/cli": "^3.2.0" which requires Node.js >=16; update the package.json
engines field to require Node >=16 (e.g., "engines": { "node": ">=16" }) or add
an engines entry if missing so the package declares the correct minimum Node
version; ensure the version string follows existing formatting (e.g., ">=16" or
">=16.0.0") and run a quick install/test to confirm no other files need
adjustments.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/generator/Cargo.toml (1)
22-24: Move [profile.release] to workspace root Cargo.tomlFile: packages/generator/Cargo.toml (lines 22–24)
[profile.release] lto = true strip = "symbols"Cargo only honors profiles at the workspace root; this package-level block is ignored — add the same block to the workspace root Cargo.toml (and remove it from the package) so LTO/strip apply to release builds.
🧹 Nitpick comments (4)
packages/generator/package.json (1)
43-45: Consider raising engines.node.If you target current LTS (Node 18/20), set "node": ">=16" or ">=18" to reduce support burden.
packages/generator/src/lib.rs (3)
107-114: Avoid borrowing from a temporary for clarity.Current code is sound but a local binding is clearer and avoids referencing a temporary Vec.
Apply this diff:
- rust_to_ts( - &rust, - entry_types - .unwrap_or_default() - .iter() - .map(|s| s.as_str()) - .collect(), - ) + let entry_types_refs: Vec<&str> = entry_types + .as_ref() + .map(|v| v.iter().map(|s| s.as_str()).collect()) + .unwrap_or_default(); + rust_to_ts(&rust, entry_types_refs)
6-8: Modernize macro import.On edition 2021 you don’t need macro_use; import the proc‑macro path directly.
-#[macro_use] -extern crate napi_derive; +use napi_derive::napi;
286-291: Remove unused local.The sorted Vec isn’t used (you call sort_hashmap! below). Drop it to silence warnings.
- let mut sorted = enum_def - .variants - .iter() - .collect::<Vec<(&String, &String)>>(); - sorted.sort_by(|a, b| a.1.cmp(b.1));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/generator/Cargo.toml(1 hunks)packages/generator/package.json(2 hunks)packages/generator/src/lib.rs(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/generator/Cargo.toml
[warning] 1-1: warning: profiles for the non root package will be ignored, specify profiles at the workspace root
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build / Build i686-pc-windows-msvc
- GitHub Check: build / Build aarch64-pc-windows-msvc
🔇 Additional comments (3)
packages/generator/Cargo.toml (1)
11-12: N-API v3 bump looks good.napi → 3.3.0 and napi-derive → 3.2.5 with default-features=false and ["napi4"] is consistent with a Node‑API v4 floor.
packages/generator/src/lib.rs (1)
77-81: Owned String params: good change.Accepting Vec removes lifetime friction at the N-API boundary.
packages/generator/package.json (1)
18-27: Verify and add missing x86_64 N-API targetsCouldn't verify — the verification script produced no output. Confirm packages/generator/package.json (napi.targets) contains the three x86_64 triples below and add them if missing.
Location: packages/generator/package.json — napi.targets
- x86_64-apple-darwin
- x86_64-unknown-linux-gnu
- x86_64-pc-windows-msvc
Apply this diff:
"napi": { "binaryName": "generator", "targets": [ + "x86_64-apple-darwin", "aarch64-apple-darwin", "aarch64-unknown-linux-gnu", "aarch64-unknown-linux-musl", "aarch64-pc-windows-msvc", "armv7-unknown-linux-gnueabihf", + "x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl", + "x86_64-pc-windows-msvc", "i686-pc-windows-msvc" ] },
| }, | ||
| "devDependencies": { | ||
| "@napi-rs/cli": "^2.18.4" | ||
| "@napi-rs/cli": "^3.2.0" |
There was a problem hiding this comment.
🧩 Analysis chain
CLI v3 bump: double‑check Node engine floor.
@npm-rs/cli v3 may require newer Node than ">=10". Confirm and align engines if needed.
🌐 Web query:
What Node.js versions are supported by @napi-rs/cli v3.x, and are "napi.binaryName"/"napi.targets" the correct config keys in v3?
💡 Result:
Short answer:
- Supported Node.js: napi v3 (used by NAPI‑RS v3 tooling) documents support for Node 12 through Node 22. [1]
- Config keys in v3: yes — use napi.binaryName (replaces napi.name) and napi.targets (replaces napi.triples). See the v2→v3 migration docs and the CLI config schema. [2][3]
Sources:
- Node support matrix (napi / napi-rs docs). [1]
- v2 → v3 migration guide (shows napi.name → napi.binaryName and triples → targets). [2]
- NAPI CLI config schema (shows binaryName and targets). [3]
Bump Node engine floor to >=12 and confirm N-API v3 keys
packages/generator/package.json — change "engines.node" from ">=10" to ">=12" to match @napi-rs/cli v3; verify N-API config uses napi.binaryName and napi.targets (v3 migration).
🤖 Prompt for AI Agents
In packages/generator/package.json around line 41, update the engines.node field
from ">=10" to ">=12" to match the @napi-rs/cli v3 requirement, and verify or
migrate the N-API configuration to use the v3 keys: ensure package.json uses
"napi.binaryName" (not old key names) and "napi.targets" (an array of target
triples) for prebuilds; adjust any existing napi config to these keys, bump any
related docs or CI Node versions, and run a local install/build to confirm
native builds still succeed.
1 participant
Summary by CodeRabbit