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
Plugins: no server; interface contravariant; no factory functions #6814
Conversation
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f5b4133:
|
b2611d5
to
5015fe0
Compare
Previously on `version-4`, we had made `ApolloServerPlugin<TContext>` "invariant" in `TContext` via the new `in out` annotation. That's because we had added a `server` field to `GraphQLRequestContext` and `GraphQLServerContext`. The issue here is that because you can invoke `server.executeOperation(op, contextValue)`, then a `ApolloServerPlugin<BaseContext>` was not assignable to `ApolloServerPlugin<SomeSpecificContext>`, because the former was able to call `requestContext.executeOperation(op, {})`, and a server whose context has additional required fields could not load this plugin. This essentially meant that you couldn't have a single object (not created by a generic function or generic class) that was a plugin that works with any server, even if it doesn't actually care about context values at all. When trying to dogfood AS4 it became clear that this was annoying. So let's fix it by removing `server` from those objects and making `ApolloServerPlugin` "contravariant" instead. This means that plugins can only *receive* `contextValue`, not *send* it. And so `ApolloServerPlugin<BaseContext>` (a plugin that doesn't bother to read any fields from `contextValue`) can be assigned to `ApolloServerPlugin<SpecificContext>` (a plugin that is permitted to read specific fields on `contextValue`, though it may choose not to). After removing `server`, restore `logger` (now `readonly`) and `cache` to `GraphQLRequestContext` and `GraphQLServerContext` (`cache` is actually new to `GraphQLServerContext` in AS4). They had previously been removed for redundancy. This un-does the fix to #6264. If your plugin needs access to the `server` object, you can just pass it to your plugin's constructor/factory yourself. This wasn't an option in AS3 because there was no `server.addPlugin` method... or actually, you could do it with the plugin factory feature. Note that we do leave `ApolloServer<TContext>` as invariant in `TContext`, for the reasons described in a comment above the class. While we're at it, remove the unnecessary ability to specify plugins as factory functions. As far as we know, the main use case for this feature is to refer to the `ApolloServer` itself in the expression creating the plugin, but the new `addPlugin` method lets you do the same thing. Reducing the number of ways to specify constructor options helps keep type errors simpler.
5015fe0
to
f5b4133
Compare
Going to merge this without full review so that I can make another alpha and move forward, but happy to make big changes or even revert after feedback. |
Previously on
version-4
, we had madeApolloServerPlugin<TContext>
"invariant" in
TContext
via the newin out
annotation. That'sbecause we had added a
server
field toGraphQLRequestContext
andGraphQLServerContext
.The issue here is that because you can invoke
server.executeOperation(op, contextValue)
, then aApolloServerPlugin<BaseContext>
was not assignable toApolloServerPlugin<SomeSpecificContext>
, because the former was ableto call
requestContext.executeOperation(op, {})
, and a server whosecontext has additional required fields could not load this plugin. This
essentially meant that you couldn't have a single object (not created by
a generic function or generic class) that was a plugin that works with
any server, even if it doesn't actually care about context values at
all.
When trying to dogfood AS4 it became clear that this was annoying. So
let's fix it by removing
server
from those objects and makingApolloServerPlugin
"contravariant" instead. This means that pluginscan only receive
contextValue
, not send it. And soApolloServerPlugin<BaseContext>
(a plugin that doesn't bother to readany fields from
contextValue
) can be assigned toApolloServerPlugin<SpecificContext>
(a plugin that is permitted toread specific fields on
contextValue
, though it may choose not to).After removing
server
, restorelogger
(nowreadonly
) andcache
to
GraphQLRequestContext
andGraphQLServerContext
(cache
isactually new to
GraphQLServerContext
in AS4). They had previously beenremoved for redundancy.
This un-does the fix to #6264. If your plugin needs access to the
server
object, you can just pass it to your plugin'sconstructor/factory yourself. This wasn't an option in AS3 because there
was no
server.addPlugin
method... or actually, you could do it withthe plugin factory feature.
Note that we do leave
ApolloServer<TContext>
as invariant inTContext
, for the reasons described in a comment above the class.While we're at it, remove the unnecessary ability to specify plugins as
factory functions. As far as we know, the main use case for this feature
is to refer to the
ApolloServer
itself in the expression creating theplugin, but the new
addPlugin
method lets you do the same thing.Reducing the number of ways to specify constructor options helps keep
type errors simpler.