Skip to content
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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

subfuzion
Copy link
Member

@subfuzion subfuzion commented Dec 4, 2024

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

  • chai@4
  • got@11
  • mime@3
  • nanoid@3
  • node-fetch@2
  • uuid@10

Revert back to pre eslint v9 flattened config

  • eslint@8

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.

@subfuzion subfuzion requested review from a team as code owners December 4, 2024 19:00
@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: aiplatform Issues related to the AI Platform API. api: appengine Issues related to the App Engine Admin API API. api: auth api: automl Issues related to the AutoML API. api: cloudasset Issues related to the Cloud Asset Inventory API. api: cloudsql Issues related to the Cloud SQL for MySQL API. api: composer Issues related to the Cloud Composer API. labels Dec 4, 2024
@product-auto-label product-auto-label bot added api: compute Issues related to the Compute Engine API. api: contactcenterinsights Issues related to the Contact Center AI Insights API. api: containeranalysis Issues related to the Container Analysis API. api: datacatalog Issues related to the Data Catalog API. api: language Issues related to the Cloud Natural Language API API. api: tasks Issues related to the Tasks API API. asset: pattern DEE Asset tagging - Pattern. labels Dec 4, 2024
Copy link
Contributor

@code-review-assist-experimental code-review-assist-experimental bot left a 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.

Copy link
Contributor

@code-review-assist-experimental code-review-assist-experimental bot left a 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.

Comment on lines 24 to 25
"uuid": "^11.0.0",
"sinon": "^19.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Upgrading uuid and sinon is good. Ensure your tests still pass with these new versions.

Suggested change
"uuid": "^11.0.0",
"sinon": "^19.0.0"
"uuid": "^11.0.0",
"sinon": "^19.0.0"

@@ -10,7 +10,7 @@
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git"
},
"engines": {
"node": "20.x"
"node": "22.x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Updating to Node.js 22.x is a good move, but ensure thorough testing of your application with this new version.

Suggested change
"node": "22.x"
"node": "22.x"

@@ -14,7 +14,7 @@
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git"
},
"engines": {
"node": "20.x.x"
"node": "22.x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Updating to Node.js 22.x is recommended. Test your application thoroughly after this change.

Suggested change
"node": "22.x"
"node": "22.x"

@@ -14,7 +14,7 @@
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git"
},
"engines": {
"node": "20.x.x"
"node": "22.x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Good to see the Node.js version updated. Ensure all tests pass after this change.

Suggested change
"node": "22.x"
"node": "22.x"

@@ -25,7 +25,7 @@
"c8": "^10.0.0",
"mocha": "^10.0.0",
"proxyquire": "^2.1.3",
"sinon": "^18.0.0",
"sinon": "^19.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Upgrading sinon is a good practice. Test thoroughly after the update.

Suggested change
"sinon": "^19.0.0",
"sinon": "^19.0.0"

@@ -24,6 +24,6 @@
"mocha": "^10.2.0",
"proxyquire": "^2.1.3",
"supertest": "^7.0.0",
"uuid": "^10.0.0"
"uuid": "^11.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Upgrading uuid is a good practice. Make sure your tests still pass after the update.

Suggested change
"uuid": "^11.0.0"
"uuid": "^11.0.0"

@@ -6,7 +6,7 @@
"license": "Apache Version 2.0",
"author": "Google Inc.",
"engines": {
"node": "20.x"
"node": "22.x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Node.js version update looks good. Test your application after the update.

Suggested change
"node": "22.x"
"node": "22.x"

Comment on lines +29 to +33
"@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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
"@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"

Comment on lines 28 to 31
"gts": "^6.0.0",
"mocha": "^10.0.0",
"sinon": "^18.0.0",
"uuid": "^10.0.0"
"sinon": "^19.0.0",
"uuid": "^11.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

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.

Suggested change
"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"

@@ -27,6 +27,6 @@
"devDependencies": {
"c8": "^10.0.0",
"mocha": "^10.0.0",
"uuid": "^10.0.0"
"uuid": "^11.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Upgrading uuid is good. Make sure your tests still pass.

Suggested change
"uuid": "^11.0.0"
"uuid": "^11.0.0"

@subfuzion subfuzion requested review from a team as code owners December 5, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: aiplatform Issues related to the AI Platform API. api: appengine Issues related to the App Engine Admin API API. api: auth api: automl Issues related to the AutoML API. api: cloudasset Issues related to the Cloud Asset Inventory API. api: cloudsql Issues related to the Cloud SQL for MySQL API. api: composer Issues related to the Cloud Composer API. api: compute Issues related to the Compute Engine API. api: contactcenterinsights Issues related to the Contact Center AI Insights API. api: containeranalysis Issues related to the Container Analysis API. api: datacatalog Issues related to the Data Catalog API. api: language Issues related to the Cloud Natural Language API API. api: tasks Issues related to the Tasks API API. asset: pattern DEE Asset tagging - Pattern. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants