Update Express v5#1879
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request upgrades the project's dependencies to Express v5 and Chai v5. Key changes include updating package.json and package-lock.json, migrating to @as-integrations/express5, and refactoring tests to use res.sendStatus() instead of passing numeric status codes to res.send(), which is a breaking change in Express v5. Feedback suggests that the sendStatus implementation in the MockResponse helper should be adjusted to send the status code as a string in the response body to accurately reflect Express v5 behavior.
| public sendStatus(code: number) { | ||
| this.status(code); | ||
| this.end(); | ||
| } |
There was a problem hiding this comment.
The sendStatus implementation in MockResponse should ideally send the status code as the response body to match Express v5 behavior, e.g., this.send(code.toString()).
| public sendStatus(code: number) { | |
| this.status(code); | |
| this.end(); | |
| } | |
| public sendStatus(code: number) { | |
| this.status(code); | |
| this.send(code.toString()); | |
| } |
There was a problem hiding this comment.
I'm not really sure about this.
.send shouldn't accept anymore the status in Express v5, so I change the mock as well to complete the request. (or, yes, make the code a string, but I'm not sure it still make sense)
Description
This Pull Request attempts upgrading to Express v5 (#1691).
I run all the tests, linting and build. Everything passed fine.
I cannot test - if test are not available - the integration with Apollo GraphQL.