Skip to content

Conversation

@nhatcher
Copy link
Member

napi-rs hase veloved quite a bit in the last year or so and we should update :)

@nhatcher nhatcher requested a review from Copilot October 21, 2025 20:03
@nhatcher nhatcher self-assigned this Oct 21, 2025
Copy link
Contributor

Copilot AI left a 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 JsUnknown type with Unknown and 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
}),
Copy link

Copilot AI Oct 21, 2025

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.

Suggested change
}),
}, null),

Copilot uses AI. Check for mistakes.
Copy link
Member Author

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.

@nhatcher nhatcher force-pushed the feature/nicolas-update-nodejs branch 5 times, most recently from e6d181a to 0ba7ea9 Compare October 21, 2025 23:01
@nhatcher nhatcher force-pushed the feature/nicolas-update-nodejs branch from 0ba7ea9 to 835137f Compare October 21, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants