-
Notifications
You must be signed in to change notification settings - Fork 421
Support Apollo v5 #3669
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
base: master
Are you sure you want to change the base?
Support Apollo v5 #3669
Conversation
@kamilmysliwiec is there anything I can do to help move this forward? I'd love to start using this in our project 🙌 |
any updates ? |
I’d like to start using this in a project as well. Are there any updates? |
It will support fastify as well? |
any updates on this? |
@kamilmysliwiec awaiting your response - any comments on this PR? |
|
It makes a lot of sense to stop using @as-integrations/express5 and create middleware within the package. middleware.ts import type { WithRequired } from '@apollo/utils.withrequired'
import type express from 'express'
import type {
ApolloServer,
BaseContext,
ContextFunction,
HTTPGraphQLRequest
} from '@apollo/server'
import { HeaderMap } from '@apollo/server'
export interface ExpressContextFunctionArgument {
request: express.Request
response: express.Response
}
export interface ExpressMiddlewareOptions<TContext extends BaseContext> {
context?: ContextFunction<[ExpressContextFunctionArgument], TContext>
}
export function expressMiddleware(
server: ApolloServer<BaseContext>,
options?: ExpressMiddlewareOptions<BaseContext>
): express.RequestHandler
export function expressMiddleware<TContext extends BaseContext>(
server: ApolloServer<TContext>,
options: WithRequired<ExpressMiddlewareOptions<TContext>, 'context'>
): express.RequestHandler
export function expressMiddleware<TContext extends BaseContext>(
server: ApolloServer<TContext>,
options?: ExpressMiddlewareOptions<TContext>
): express.RequestHandler {
server.assertStarted('expressMiddleware()')
// This `any` is safe because the overload above shows that context can
// only be left out if you're using BaseContext as your context, and {} is a
// valid BaseContext.
const defaultContext: ContextFunction<
[ExpressContextFunctionArgument],
// eslint-disable-next-line @typescript-eslint/no-explicit-any
any
> = async () => ({})
const context: ContextFunction<[ExpressContextFunctionArgument], TContext> =
options?.context ?? defaultContext
return async (request, response) => {
if (!('body' in request)) {
// The json body-parser *always* initializes the `body` field on requests
// when it runs. (body-parser@1 (included in Express v4 as
// `express.json()`) sets it to `{}` by default, and body-parser@2
// (included in Express v5 as `express.json()`) sets to to `undefined` by
// default.)
//
// So if the field is *completely* missing (not merely set to undefined,
// but actually not there), you probably forgot to set up body-parser. We
// send a nice error in this case to help with debugging.
return void response
.status(500)
.send(
'`req.body` is not set; this probably means you forgot to set up the ' +
'`json` middleware before the Apollo Server middleware.'
)
}
const headers = new HeaderMap()
for (const [key, value] of Object.entries(request.headers)) {
if (value !== undefined) {
// Node/Express headers can be an array or a single value. We join
// multi-valued headers with `, ` just like the Fetch API's `Headers`
// does. We assume that keys are already lower-cased (as per the Node
// docs on IncomingMessage.headers) and so we don't bother to lower-case
// them or combine across multiple keys that would lower-case to the
// same value.
headers.set(key, Array.isArray(value) ? value.join(', ') : value)
}
}
const httpGraphQLRequest: HTTPGraphQLRequest = {
method: request.method.toUpperCase(),
headers,
search: new URL(request.url).search ?? '',
body: request.body
}
const httpGraphQLResponse = await server.executeHTTPGraphQLRequest({
httpGraphQLRequest,
context: () => context({ req: request, res: response })
})
for (const [key, value] of httpGraphQLResponse.headers) {
response.setHeader(key, value)
}
response.statusCode = httpGraphQLResponse.status || 200
if (httpGraphQLResponse.body.kind === 'complete') {
return void response.send(httpGraphQLResponse.body.string)
}
for await (const chunk of httpGraphQLResponse.body.asyncIterator) {
response.write(chunk)
// Express/Node doesn't define a way of saying "it's time to send this
// data over the wire"... but the popular `compression` middleware
// (which implements `accept-encoding: gzip` and friends) does, by
// monkey-patching a `flush` method onto the response. So we call it
// if it's there.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
if (typeof (response as any).flush === 'function') {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(response as any).flush()
}
}
return void response.end()
}
} |
Alternative above removes unnecessary dependencies like for url parsing, now uses new URL() constructor |
Is this production ready yet? |
|
Apollo Server 4 will reach EOL on January 26, 2026 https://www.apollographql.com/docs/apollo-server/previous-versions |
Can I help with something or is there anything missing to implement it? |
@tinezmatias there hasn't been any feedback from the maintainer(s). I asked for feedback on Discord as well, but no response as of yet. I don't feel like spamming and tagging every week if I'm ending up being ignored. For now, I've packaged this as a tgz archive in our project, and it's been running perfectly fine in production for a few weeks. |
@kamilmysliwiec It would be great if you could look at merging this PR. Especially in regards to Apollo Server 4 reaching its EOL really soon!! |
Much needed feature. |
@gmenih If you can put the code above in the file, you don't need to let the guy always download the dependency. |
@luas10c I won't do that. My PR only adds v5 support as documented in Migrating from Apollo Server 4. If you wish to change the middleware, feel free to open your own PR. Also, if you're referring to the |
We'll get this shipped prob next month, no worries |
I say this because I helped contribute to v4, and whenever there is a new version of Apollo(express) it is necessary to install a new lib @as-integrations/express-v4 or @as-integrations/apollo-v5 it was just a suggestion to have the middleware that this lib implements directly in the @nestjs/graphql module, whenever there was a change in Express we would not be dependent on libs. But no problem, rest assured. |
![]() In the node js documentation it says that it supports new URL, using url.parse is the legacy way that is not as performant as new URL |
I actually agree with @luas10c on this - might be easier to just place it in the package given how small it is |
Alright, I can add it later today. Will also check if its doable for Fastify as well. |
A simple version upgrade will be easier to accept by the maintainers. A more complex contribution can delay the acceptance. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
@apollo/server@v5
is now supported, along with asexpress@v5
Does this PR introduce a breaking change?
An additional dependency is introduced -
@as-integrations/express5
; not an API breaking change, but projects using this package will have to install the new dependency. I'm happy to update the docs if this gets approved.Other information
Technically both express@v4 and express@v5 are supported by
@apollo/server@v5
, but they require separate dependencies - either@as-integrations/express4
or@as-integrations/express5
.Considering that
@nestjs/platform-express
is on v5, I used v5 here as well.