-
Notifications
You must be signed in to change notification settings - Fork 2k
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
chore(deps): update all dependencies #3929
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.
Hi @subfuzion, I'm currently reviewing this pull request and will post my detailed feedback in a few minutes. In the meantime, here's a quick summary for you and other reviewers to get up to speed:
This PR, "chore(deps): update all dependencies", aims to update various dependencies across multiple sample applications. The original PR (#3889) was reverted due to incompatibilities, and this version focuses on updating to compatible versions for chai
, got
, mime
, nanoid
, and eslint
. Specifically, the changes include updating chai
to ^4.5.0, got
to ^11 (though the exact version isn't specified in all files), mime
to ^3, nanoid
to ^3, and eslint
to ^8. Additionally, several Node.js versions are updated to 22.x across various package.json
files, and some other minor dependency version bumps are included (e.g., sinon
, uuid
, @types/express
, gts
). The Dockerfiles for several examples are also updated to use Node.js 22-slim.
If there's any missing context, please let me know. I'll provide a more detailed review shortly.
And now, a little haiku to brighten your day:
Code flows like a stream,
Updates ripple, clean and bright,
Bugs flee in the night.
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.
This pull request updates several dependencies, including a major version bump for Node.js (20.x to 22.x), uuid
(10.0.0 to 11.0.0), sinon
(18.0.0 to 19.0.0), gts
(5.0.0 to 6.0.0), @types/express
(4.17.17 to 5.0.0), and @types/node
(20.0.0 to 22.0.0). It also intentionally keeps some dependencies at older versions due to incompatibilities: chai
at v4, got
at v11, mime
at v3, and eslint
at v8. While updating Node.js and core testing libraries is generally a good practice, it's important to address the stated incompatibilities with other libraries promptly to benefit from the latest features, performance improvements, and security fixes. Consider creating follow-up tasks to track upgrading these as soon as possible.
Thorough testing is crucial after dependency updates. Ensure all existing tests pass and consider adding integration tests to verify interoperability between components. Pay close attention to the breaking changes introduced by @types/express
and ensure your code is compatible with version 5. Review the changelogs for all updated dependencies to identify any potential breaking changes or deprecated features that might affect your code. Documenting these updates in your changelog or release notes is also recommended.
ai-platform/snippets/package.json
Outdated
"uuid": "^11.0.0", | ||
"sinon": "^19.0.0" |
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.
@@ -10,7 +10,7 @@ | |||
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git" | |||
}, | |||
"engines": { | |||
"node": "20.x" | |||
"node": "22.x" |
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.
@@ -14,7 +14,7 @@ | |||
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git" | |||
}, | |||
"engines": { | |||
"node": "20.x.x" | |||
"node": "22.x" |
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.
@@ -14,7 +14,7 @@ | |||
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git" | |||
}, | |||
"engines": { | |||
"node": "20.x.x" | |||
"node": "22.x" |
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.
@@ -25,7 +25,7 @@ | |||
"c8": "^10.0.0", | |||
"mocha": "^10.0.0", | |||
"proxyquire": "^2.1.3", | |||
"sinon": "^18.0.0", | |||
"sinon": "^19.0.0", |
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.
@@ -24,6 +24,6 @@ | |||
"mocha": "^10.2.0", | |||
"proxyquire": "^2.1.3", | |||
"supertest": "^7.0.0", | |||
"uuid": "^10.0.0" | |||
"uuid": "^11.0.0" |
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.
@@ -6,7 +6,7 @@ | |||
"license": "Apache Version 2.0", | |||
"author": "Google Inc.", | |||
"engines": { | |||
"node": "20.x" | |||
"node": "22.x" |
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.
"@types/express": "^5.0.0", | ||
"@types/node": "^22.0.0", | ||
"c8": "^10.0.0", | ||
"chai": "^4.5.0", | ||
"gts": "^5.0.0", | ||
"gts": "^6.0.0", |
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.
Upgrading these dev dependencies is good. However, ensure that your tests and build process still work correctly with the new versions, especially with the breaking change in @types/express
v5. Check its changelog for details.
"@types/express": "^5.0.0", | |
"@types/node": "^22.0.0", | |
"c8": "^10.0.0", | |
"chai": "^4.5.0", | |
"gts": "^5.0.0", | |
"gts": "^6.0.0", | |
"@types/express": "^5.0.0", | |
"@types/node": "^22.0.0", | |
"c8": "^10.0.0", | |
"chai": "^4.5.0", | |
"gts": "^6.0.0" |
asset/snippets/package.json
Outdated
"gts": "^6.0.0", | ||
"mocha": "^10.0.0", | ||
"sinon": "^18.0.0", | ||
"uuid": "^10.0.0" | ||
"sinon": "^19.0.0", | ||
"uuid": "^11.0.0" |
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.
Upgrading gts
, sinon
, and uuid
is good practice for keeping dependencies up-to-date. Ensure your tests and build process still function correctly with the new versions.
"gts": "^6.0.0", | |
"mocha": "^10.0.0", | |
"sinon": "^18.0.0", | |
"uuid": "^10.0.0" | |
"sinon": "^19.0.0", | |
"uuid": "^11.0.0" | |
"gts": "^6.0.0", | |
"mocha": "^10.0.0", | |
"sinon": "^19.0.0", | |
"uuid": "^11.0.0" |
auth/package.json
Outdated
@@ -27,6 +27,6 @@ | |||
"devDependencies": { | |||
"c8": "^10.0.0", | |||
"mocha": "^10.0.0", | |||
"uuid": "^10.0.0" | |||
"uuid": "^11.0.0" |
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.
This is #3889 but submitted from a repo branch instead of a fork. It reverts a number of incompatible upgrades to stay on the following versions for now:
Revert back to CommonJS (pre ESM-only) packages
Revert back to pre eslint v9 flattened config
Ignore renovate-bot PR comments recommending updates to incompatible versions until we're ready. I also recommend that we disable renovate-bot for the repo in the meantime.