-
Notifications
You must be signed in to change notification settings - Fork 136
ci(deps): Pin third party node dependencies #11965
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?
ci(deps): Pin third party node dependencies #11965
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Signed-off-by: Roger Barker <[email protected]>
4e1d4ff to
fc3ac0a
Compare
Signed-off-by: Roger Barker <[email protected]>
Signed-off-by: Roger Barker <[email protected]>
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.
We have other NPM packages in the tools/ folder. Shouldn't we handle those as well?
| "peerDependencies": { | ||
| "ansi-styles": "6.2.3", | ||
| "ansi-regex": "6.2.2", | ||
| "strip-ansi": "7.1.2", | ||
| "debug": "4.4.1", | ||
| "chalk": "5.6.2", | ||
| "wrap-ansi": "9.0.2", | ||
| "is-arrayish": "0.3.2" |
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.
Do we really need to put these here if we don't even use most of them? Shouldn't we just put the actual transitive packages we use? Same comment for other package.jsons
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.
Each of these were present in the lock files when we reviewed them. That's why we've added these in here.
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.
Not sure what you mean. On main, rest/monitoring/package.json only uses debug out of the above list. So shouldn't peerDependencies only contain debug instead of a bunch of unused and irrelevant dependencies?
Signed-off-by: Andrew Brandt <[email protected]>
|
@steven-sheehy please re-review when you get a chance, thank you! |
|
So we're not going to update the tools folder as well? Or will that be a separate PR? |
| "ansi-styles": "6.2.3", | ||
| "ansi-regex": "6.2.2", | ||
| "strip-ansi": "7.1.2", | ||
| "debug": "4.4.1", | ||
| "chalk": "5.6.2", | ||
| "wrap-ansi": "9.0.2", | ||
| "is-arrayish": "0.3.2" |
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.
On main, only debug and chalk are in this package-lock.json
| "peerDependencies": { | ||
| "ansi-styles": "6.2.3", | ||
| "ansi-regex": "6.2.2", | ||
| "strip-ansi": "7.1.2", | ||
| "debug": "4.4.1", | ||
| "chalk": "5.6.2", | ||
| "wrap-ansi": "9.0.2", | ||
| "is-arrayish": "0.3.2" |
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.
Not sure what you mean. On main, rest/monitoring/package.json only uses debug out of the above list. So shouldn't peerDependencies only contain debug instead of a bunch of unused and irrelevant dependencies?
Description
This pull request makes dependency management more consistent and predictable across the
rest/check-state-proofandrest/monitoringpackages by enforcing exact versioning for dependencies and updating several dependency versions and peer dependencies. The changes help ensure reproducible builds and reduce the risk of unexpected issues due to unintentional dependency upgrades.Dependency management improvements:
save-exact=trueto.npmrcfiles inrest/,rest/check-state-proof/, andrest/monitoring/to enforce exact dependency versioning for future installs.package.jsonandpackage-lock.jsonfor bothrest/check-state-proofandrest/monitoringto use exact versions instead of version ranges (e.g.,"^1.2.3"→"1.2.3"). [1] [2] [3]peerDependenciesin both packages to explicitly require certain versions of shared libraries (such aschalk,ansi-styles,debug,is-arrayish,strip-ansi,wrap-ansi, etc.), improving compatibility and clarity for consumers. [1] [2] [3]Dependency version updates and cleanups:
package-lock.json, including upgrades and removals (e.g.,wrap-ansi,is-arrayish,protobufjs, and related dependencies), and removed redundant or outdated nested dependencies. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]These changes together make dependency management more robust and predictable for these packages.
Related issue(s)
Fixes #11964
Related Testing