Skip to content

Update Express v5#1879

Open
alexandercerutti wants to merge 3 commits intofirebase:masterfrom
alexandercerutti:master
Open

Update Express v5#1879
alexandercerutti wants to merge 3 commits intofirebase:masterfrom
alexandercerutti:master

Conversation

@alexandercerutti
Copy link
Copy Markdown

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.

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 5, 2026

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.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread spec/helper.ts
Comment on lines +78 to +81
public sendStatus(code: number) {
this.status(code);
this.end();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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()).

Suggested change
public sendStatus(code: number) {
this.status(code);
this.end();
}
public sendStatus(code: number) {
this.status(code);
this.send(code.toString());
}

Copy link
Copy Markdown
Author

@alexandercerutti alexandercerutti May 5, 2026

Choose a reason for hiding this comment

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

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants