Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request upgrades lru-cache to version 11. The code changes correctly reflect the API updates from the new version across all relevant files, including source code, documentation, and configuration. I've found one issue regarding Node.js engine compatibility in the tools package that should be addressed, aligning with the repository's Node.js version policy.
| "firebase-admin": "^11.4.1", | ||
| "lodash": "^4.17.20", | ||
| "lru-cache": "^6.0.0", | ||
| "lru-cache": "^11.2.6", |
There was a problem hiding this comment.
Upgrading lru-cache to version 11 introduces a new Node.js version requirement (20 || >=22). However, the engines.node field for this package is still set to >=12.x (on line 11), which is now incompatible. Please update the engines field to align with the new dependency requirements and the repository's policy to target the oldest active Long-Term Support (LTS) version, for example to >=20.x.
References
- The required Node.js version in
package.jsonshould target the oldest active Long-Term Support (LTS) version.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd6c1b2cea
ℹ️ 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".
| "firebase-admin": "^11.4.1", | ||
| "lodash": "^4.17.20", | ||
| "lru-cache": "^6.0.0", | ||
| "lru-cache": "^11.2.6", |
There was a problem hiding this comment.
Align tools runtime floor with lru-cache v11 requirement
Upgrading firecrypt-tools to lru-cache@^11.2.6 raises the effective Node.js minimum to 20, but this package still declares "engines": {"node": ">=12.x"}; the updated lockfile also records lru-cache as "node": "20 || >=22" (tools/package-lock.json), so installs/runs on Node 12/14/16/18 (currently advertised as supported) will now fail or become unsupported. Please either keep a Node-12-compatible cache dependency or update the tools package engine constraint to match the new requirement.
Useful? React with 👍 / 👎.
cd6c1b2 to
c6e328e
Compare
pkaminski
left a comment
There was a problem hiding this comment.
@pkaminski resolved 2 discussions.
Reviewable status: 0 of 15 files reviewed, all discussions resolved (waiting on @snoack).
snoack
left a comment
There was a problem hiding this comment.
@snoack reviewed 15 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @pkaminski).
This change is