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

[breaking] Consider dropping built-in integration with Express v4 and replacing it with Express v5 (or having no built-in integrations other than startStandaloneServer) #7928

Open
joergbaier opened this issue Sep 13, 2024 · 10 comments
Labels
🍳 breaking-change Needs to wait for a major release.

Comments

@joergbaier
Copy link

@apollo/server has express v4 as a dependency. After several years, express is moving forward with releasing v5 (currently published as next tag). Having support for v5 here would unblock and encourage people to upgrade.

https://www.npmjs.com/package/express?activeTab=versions
https://github.com/expressjs/express/releases/tag/v5.0.0

At this time, I have not checked for breaking changes in this repo, but wanted to open this issue to get things rolling. Any changes could be published under the next tag as well.

@joergbaier
Copy link
Author

Typing updates for @types/express & @types/express-serve-static-core
DefinitelyTyped/DefinitelyTyped#70563

@RobinTail
Copy link

Breaking changes are listed in the migration guide, which is not yet up to date. Besides the listed changes there is also Node.js min supported version: 18.
The primary feature is proper handling of asynchronous handlers.

@glasser
Copy link
Member

glasser commented Sep 16, 2024

I think it would be a good start if community members wanted to publish Express 5 integrations separately (perhaps under the @as-integrations npm org / apollo-server-integrations GitHub org)! While we could add a @apollo/server/express5 directory directly to Apollo Server, it might be challenging to do so in a way that uses different dependencies for two different parts of the same npm module, so getting this in core might be more likely a thing for the next major version of AS, which is not currently scheduled.

@RobinTail
Copy link

I made types. Enjoy :)

image

@P4sca1
Copy link

P4sca1 commented Oct 5, 2024

I am using express5 beta with apollo server 4 for the past year without any problems.
After upgrading @types/express and @types/express-serve-static-core to v5, I needed to override the version in apollo server to avoid type mismatches. e.g. for pnpm:

{
        "pnpm": {
                "overrides": {
                        "@types/express": "5.0.0",
                        "@types/express-serve-static-core": "5.0.0"
                }
        }
}

@AndKiel
Copy link

AndKiel commented Oct 28, 2024

I tried upgrading to express v5 and it looks like apollo-server is incompatible. This check:

is invalid because of the following breaking change:

The req.body property returns undefined when the body has not been parsed. In Express 4, it returns {} by default.

EDIT: I mainly wrote this comment because the one above said that the upgrade to beta worked without any problems.

@glasser
Copy link
Member

glasser commented Oct 28, 2024

I don't think it should be a big surprise that the code in the express4 directory does not work with Express 5.

The main interface for Apollo Server is the generic interface that works with any web server if you write a short adapter. Apollo Server does ship with one "first party" binding for the most popular and stable web framework, Express v4. But it's an explicit goal of the AS4 rewrite that it is not the job of Apollo Server itself to provide bindings to every web framework out there. That's how things worked in AS3 — the core project needed to have direct integrations to all supported web frameworks and there was no extensibility. In AS4 we provide a built-in integration with the most popular framework (Express v4) and community packages can provide integrations (typically about a page of code) with other frameworks.

It is likely that at some point in the future, Express v5 may be the most popular and stable web framework, but for today, that's still Express v4. So as mentioned above in #7928 (comment) I'd encourage folks on the cutting edge to put together a community package for @as-integrations/express5.

Perhaps Apollo Server v5 would incorporate that as its default and shift Express v4 to "community/external". But for now I would assume the place to start is with an integration.

@glasser glasser changed the title express v5 Consider adding built-in support for Express v5 Oct 28, 2024
@glasser glasser changed the title Consider adding built-in support for Express v5 Consider adding built-in integration with Express v5 Oct 28, 2024
@glasser glasser added the 🍳 breaking-change Needs to wait for a major release. label Oct 28, 2024
@glasser glasser changed the title Consider adding built-in integration with Express v5 Consider dropping built-in integration with Express v4 and replacing it with Express v5 Oct 28, 2024
@glasser glasser changed the title Consider dropping built-in integration with Express v4 and replacing it with Express v5 [breaking] Consider dropping built-in integration with Express v4 and replacing it with Express v5 Oct 28, 2024
glasser added a commit that referenced this issue Oct 29, 2024
v5 isn't "latest" yet for express itself but it will be soon and it is
already for the DefinitelyTyped packages.

See #7928.
glasser added a commit that referenced this issue Oct 29, 2024
v5 isn't "latest" yet for express itself but it will be soon and it is
already for the DefinitelyTyped packages.

See #7928.
@cupofjoakim
Copy link

Apollo Server does ship with one "first party" binding for the most popular and stable web framework, Express v4.
...
It is likely that at some point in the future, Express v5 may be the most popular and stable web framework, but for today, that's still Express v4.

@glasser Is this the perspective of apollo for node versions too? To only support them once it's the most popular version? Express 4 and 5 are not different frameworks, they are just different versions.

Does this mean you're closed for contributions of an express 5 adapter too?

@glasser
Copy link
Member

glasser commented Dec 12, 2024

A library that runs on 0 versions of the runtime is not useful. So we will continue to track versions of the runtime that we support. (Of course, we have other issues like "we are overdue to bumping our major version number and dropping old Node version support" too...)

A library that has zero built-directly-into-the-core-package implementations of the interface it exports is still useful. Express 4 is an outlier here in that it is very slightly easier to use than all of the other frameworks.

The versions of Express have different API, so building against them require linking in multiple versions of the same library or its DefinitelyTyped libraries. This makes the build more complex and hard to evaluate.

We went through all of this previously with Fastify v2 to v3. It's a lot easier to ship entirely separate adapter packages for different incompatible versions of a framework than to try to make one package that magically works with multiple versions.

The users of Fastify, Hapi, Koa, Next.js, and h3 are fine installing an integration package. It will work fine for Express v5 too. And now that we know that Express is a moving target rather than something that hasn't made an incompatible change in a decade, it probably makes sense to put it in on equal footing with all the other web frameworks and make the next major version of @apollo/server a low-dependency pure implementation of its interface instead of anointing a single version of a single framework as special.

@glasser glasser changed the title [breaking] Consider dropping built-in integration with Express v4 and replacing it with Express v5 [breaking] Consider dropping built-in integration with Express v4 and replacing it with Express v5 (or having no built-in integrations other than startStandaloneServer) Dec 12, 2024
@trevor-scheer
Copy link
Member

@cupofjoakim (or anyone else interested), I'd be happy to add an express repo to the integrations org if someone is looking to implement and maintain the integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍳 breaking-change Needs to wait for a major release.
Projects
None yet
Development

No branches or pull requests

7 participants