-
Notifications
You must be signed in to change notification settings - Fork 98
UPDATE: Update nodejs dependencies #476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the napi-rs dependency ecosystem for the Node.js bindings, upgrading from version 2.x to 3.x. The update includes API changes where JsUnknown is replaced with Unknown, lifetime annotations are added to certain methods, and the package.json configuration is updated to align with napi-rs 3.x conventions.
- Updated napi and napi-derive dependencies from 2.x to 3.x
- Replaced
JsUnknowntype withUnknownand added explicit lifetime annotations - Migrated package.json configuration structure to match napi-rs 3.x requirements
Reviewed Changes
Copilot reviewed 19 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| bindings/nodejs/Cargo.toml | Updated napi dependency versions to 3.x |
| bindings/nodejs/src/user_model.rs | Replaced JsUnknown with Unknown type and added lifetime annotations |
| bindings/nodejs/src/model.rs | Replaced JsUnknown with Unknown type and added lifetime annotations |
| bindings/nodejs/package.json | Updated @napi-rs/cli, restructured napi configuration, and added optionalDependencies |
| bindings/nodejs/npm/*/package.json | Bumped version from 0.2.0 to 0.6.0 across all platform-specific packages |
| bindings/nodejs/index.js | Regenerated loader code with improved platform detection and error handling |
Files not reviewed (1)
- bindings/nodejs/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| cause: loadErrors.reduce((err, cur) => { | ||
| cur.cause = err | ||
| return cur | ||
| }), |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reduce operation lacks an initial value, which will cause it to start with the first error. This means the first error won't have its cause set and the chain will be incomplete. Provide an initial value of null or undefined to properly build the error chain.
| }), | |
| }, null), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is generated code.
e6d181a to
0ba7ea9
Compare
0ba7ea9 to
835137f
Compare
napi-rs hase veloved quite a bit in the last year or so and we should update :)