updated noble/curves dep #612
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the @noble/curves dependency from version 1.9.6 to 2.0.1, which includes breaking API changes that require modifications to how the library is used.
Changes:
- Updated @noble/curves from ^1.9.6 to ^2.0.1 in package.json and package-lock.json
- Updated import paths in src/chat/signature.ts to use .js extensions and adapted function calls to use hexToBytes conversions for the new API
- Changed ns import in src/chat/keys.ts from default import to namespace import
- Added acorn as a production dependency
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| package.json | Updated @noble/curves version and added acorn dependency |
| package-lock.json | Updated lockfile with new dependency versions and Node.js requirements |
| src/chat/signature.ts | Updated imports with .js extensions and adapted schnorr API calls to use byte arrays |
| src/chat/keys.ts | Changed ns import style but missing corresponding .js extension updates for @noble imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/chat/keys.ts
Outdated
| import { schnorr } from '@noble/curves/secp256k1' | ||
| import { bytesToHex } from '@noble/hashes/utils' | ||
| import ns from '../ns' | ||
| import * as ns from '../ns' |
There was a problem hiding this comment.
The import statement for ns should use default import (import ns from) instead of namespace import (import * as ns from), to maintain consistency with the rest of the codebase. All other files in the project use 'import ns from' (e.g., src/acl/access-groups.ts:10, src/chat/chatLogic.js:9, src/utils/keyHelpers/accessData.ts:3, and many others). Since ns.js exports a default export (src/ns.js:6), this should be 'import ns from'.
src/chat/keys.ts
Outdated
| import { schnorr } from '@noble/curves/secp256k1' | ||
| import { bytesToHex } from '@noble/hashes/utils' |
There was a problem hiding this comment.
The imports from @noble/curves and @noble/hashes are missing the .js file extensions that were added in src/chat/signature.ts. For consistency with the changes in signature.ts and to align with @noble/curves v2.0.1 best practices, these imports should be updated to:
- '@noble/curves/secp256k1.js' (line 2)
- '@noble/hashes/utils.js' (line 3)
This ensures consistent import patterns across the codebase when using the @noble libraries.
| import { schnorr } from '@noble/curves/secp256k1' | |
| import { bytesToHex } from '@noble/hashes/utils' | |
| import { schnorr } from '@noble/curves/secp256k1.js' | |
| import { bytesToHex } from '@noble/hashes/utils.js' |
package.json
Outdated
| "@noble/curves": "^1.9.6", | ||
| "@noble/hashes": "^1.8.0", | ||
| "@noble/curves": "^2.0.1", | ||
| "acorn": "^8.15.0", |
There was a problem hiding this comment.
The addition of acorn as a production dependency appears unrelated to the @noble/curves update mentioned in the PR title. Previously, acorn was only used as a dev dependency (via acorn-jsx and acorn-import-phases). If this change is intentional and acorn is now required at runtime, please clarify the reason in the PR description. If it's accidental, this line should be removed.
No description provided.