Make express a peer dependency of @apollo/server

[jekh]

I use fastify rather than express, and would like to avoid pulling all of express into my production dependencies. (Express itself has 31 direct dependencies, too.) Making it a peer dependency would allow package managers to enforce compatibility without forcing a production dependency on it. It would also preserve the new user happy path experience, since the npm default behavior is to auto-install peer dependencies.

[trevor-scheer]

Hmm, I think I'm in favor of this but I have a couple concerns (and questions).

1. This is a breaking change. One we can probably stomach without a major version, given that AS4 is still very early...though I would've much preferred to get this sorted in alpha/rc.
2. I don't love depending on the package manager's opinions to do what we hope (i.e. our guidance should be updated to include express in your project if we make this change). I don't think yarn auto-installs peer deps, nor pnpm?

Would your suggestion also include making the peer dep optional via peerDependenciesMeta? Does using ESM solve your woes? I suppose that only becomes helpful if you're bundling and tree shaking?

[glasser]

So here's what we were thinking when we chose to do this. (tl;dr: avoiding the dependencies does make using this "new user happy path" legitimately harder in common circumstances, and the downside is only a 17% bloat and you can shelter your production deploys from that bloat (and much more) if you use a build tool.)

The dependency on express (as well as body-parser and cors) is used only by the standalone server. (The Express middleware does depend on @types/express as part of defining its API but does not use any of these packages at runtime.)

So question one was: should we put the standalone server in the main Apollo Server package (@apollo/server) or give it its own package (@apollo/server-standalone?). The AS2/AS3 model was for the core code to be in apollo-server-core and the web framework integrations to be in other packages; confusingly, apollo-server was the equivalent of AS4's startStandaloneServer despite its unassuming name which might make you think it was the package everyone should use for everything. We generally found the profusion of packages in AS2/3 to lead to a lot of confusion, often ending up with multiple copies of packages in projects at slightly different versions, or version skew between packages, etc. Ensuring that the most common use cases just required a single package seemed like an advantage here, rather than having the "getting started" experience involve installing two separate AS packages.

So that's why we decided that the standalone server should be in the same npm package as the core code. The next question is, how should the dependencies on express, cors, and body-parser work? I'm not sure that your suggestion of a peer dependency actually helps, unless you make it an optional peer dependency. We definitely don't want to require users to install express, cors, and body-parser manually (eg, what if we need a fourth package later? what if we want to swap out express for something simpler later?)

But relying on particular package manager settings to allow a simple getting started experience doesn't sound good either. For example, let's say you want to experiment with running a prerelease version of graphql (graphql-js). This is what you have to do right now in order to try out our experimental support for incremental delivery (defer/stream). The peer dep from @apollo/server on graphql doesn't allow for graphql prereleases (and even if we changed that, you're probably using other npm packages with graphql peer deps that don't allow for it). So to get that to work with npm right now, as far as I know you have to do npm i --legacy-peer-deps. And that will turn off the automatic installation of peer deps, which would break the user experience.

So it really does seem like the best user experience is going to come from direct dependencies on the code required by the standalone server, in the same package as everything else. But you're right: there certainly are costs to this.

My understanding is that the biggest concern here is just code size. (There are other concerns like "what if these unused dependencies have CVEs which will give false positives in my scanner" but hopefully that will be alleviated just by us being on top of upgrading dependencies.)

If we're looking literally at the amount of code size on a developer machine, it looks like npm i express cors body-parser takes about 17% of the space of npm i @apollo/server (whether using du to count disk space rounding small files up, or just raw bytes). So the total gain we could get by dropping these dependencies is not completely trivial but is still relatively small.

Our assumption has been that optimizing for developer machine disk space is not the highest priority for server developers. Certainly there are folks who care deeply about the size of their server-size code (especially folks using serverless platforms where container start times matter a lot). But in that case, it's probably best to not just send raw npm packages to your servers at all, but to instead use a JS build tool like rollup. And in fact, one of the biggest subprojects for AS4 was ensuring that our package was available as an ESM package, and that optional parts of the codebase like the standalone server could be avoided by putting them in deep imports. We even have an explicit test that if you use rollup and don't use startStandaloneServer, you won't be linking express into your app. The size savings by using rollup are huge: a trivial server turns into ~4MB (if using startStandaloneServer) or ~2MB (if not using startStandaloneServer), vs the 18MB for npm i @apollo/server: the amount of space savings you get by using a build tool dwarfs what you'd get by avoiding installing express in the first place.

(And as part of making that big benefit available, we did have to make an explicit tradeoff against "developer machine" disk space: because we still need to support CJS users, our packages ship with both CJS and ESM code (not to mention the original TS source files for convenience). This also makes the developer machine disk usage larger! But it enables much more effective bundle size compression.)

Finally, we have considered avoiding express specifically, since it is in fact on the big-ish size for what it does. We have explicitly documented that users of startStandaloneServer should not assume it's built on top of Express (the types available to context functions just use the core Node request/response typings instead of Express's typings) and perhaps we could strip it down. We definitely don't want to reimplement body-parser ourselves. We perhaps could reimplement cors (since we only provide its "default configuration") but even that is slightly non-trivial as we would have to write the code to echo access-control-request-headers, etc). Finally, we could possibly replace express with a manual implementation of middleware composition (or a smaller package like compose-middleware) plus a manual implementation of an error handling final handler (or directly use the package finalhandler like express does). But it feels like that might be a lot of potentially error-prone code just to avoid using one of the most popular packages on npm.