Plugins: no server; interface contravariant; no factory functions

.changeset/wet-kiwis-play.md

@@ -0,0 +1,15 @@
+---
+"@apollo/server-integration-testsuite": patch
+"@apollo/server-plugin-response-cache": patch
+"@apollo/server": patch
+---
+
+Several changes relating to plugins:
+
+- Remove the `server` field on `GraphQLRequestContext` and `GraphQLServerContext` (ie, the arguments to most plugin hook methods). This was added during AS4 development and did not exist in AS3.
+
+- Add `logger` and `cache` fields to `GraphQLRequestContext` and `GraphQLServerContext`. The `logger` fields and `GraphQLRequestContext.cache` existed in AS3 and had been previously removed for redundancy with the `server` field. (Unlike in AS3, `logger` is readonly.)
+
+- `ApolloServerPlugin` is now declared as `<in TContext extends BaseContext = BaseContext>` rather than `<in out TContext>`. This means that you can declare a plugin that doesn't care about `contextValue` to simply implement `ApolloServerPlugin` and it will work with any `ApolloServer<NoMatterWhatContext>`. This should make it easy to write plugins that don't care about context.
+
+- Remove the ability to specify a factory function as an element of the `plugins` list in the `ApolloServer` constructor. (Reducing the number of ways to specify constructor options helps keep type errors simpler.) As far as we know the main use case for this (referring to the `ApolloServer` itself when creating the plugin) can be handled with the new-in-AS4 `ApolloServer.addPlugin` method.

docs/source/migration.mdx

@@ -1383,7 +1383,7 @@ Most plugin API hooks take a `GraphQLRequestContext` object as their first argum
 
 The `context` field has been renamed `contextValue` for consistency with the `graphql-js` API and to help differentiate from the `context` option of integration functions (the *function* which returns a context value).
 
-Apollo Server 4 removes the  `logger` and `cache` fields from `GraphQLRequestContext`. These fields are now available as `public readonly` fields on the `ApolloServer` object.  The `GraphQLRequestContext` object now provides the `ApolloServer` object in a new field named `server`. This means you should replace `requestContext.logger` and `requestContext.cache` with `requestContext.server.logger` and `requestContext.server.cache` respectively.
+The `GraphQLRequestContext.logger` field is now `readonly`. If you depended on the ability to change the `logger`, please file an issue.
 
 Apollo Server 4 removes the `schemaHash` field from `GraphQLRequestContext`. This field is an unstable hash of a JSON encoding resulting from running a GraphQL introspection query against your schema. The `schemaHash` field is not guaranteed to change when the schema changes (e.g., it is not affected by changes to schema directive applications). If you want a schema hash, you can use `graphql-js`'s `printSchema` function on the `schema` field and then hash the output.
 
@@ -1396,8 +1396,6 @@ Apollo Server 4 makes several changes to the `GraphQLServerContext` object.
 
 Apollo Server 4 renames the TypeScript type for the argument to the `serverWillStart` plugin hook from  `GraphQLServiceContext` to `GraphQLServerContext`, for consistency with the hook name.
 
-Apollo Server 4  removes the `logger` field from the `GraphQLServerContext` object. This field is now available as a `public readonly` field on the `ApolloServer` object, which `GraphQLServerContext` provides via a new field named `server`. This means `serviceContext.logger` should be replaced with `serverContext.server.logger`.
-
 Apollo Server 4 removes the `schemaHash` field (see the [previous section](#fields-on-graphqlrequestcontext) for details).
 
 Apollo Server 4 removes the `persistedQueries` field from `GraphQLServerContext`. We don't have a reason for providing this particular configuration to plugins, but if having this field is important for you, please file a GitHub issue.
@@ -1431,6 +1429,44 @@ The value of `http.headers` is now a `Map` (with lower-case keys) rather than a
 
 > We plan to implement experimental support for incremental delivery (`@defer`/`@stream`) before the v4.0.0 release and expect this to change the structure of `GraphQLResponse` further.
 
+### `plugins` constructor argument does not take factory functions
+
+In Apollo Server 3, each element of the `plugins` array provided to `new ApolloServer` could either be an `ApolloServerPlugin` (ie, an object with fields like `requestDidStart`) or a zero-argument "factory" function returning an `ApolloServerPlugin`.
+
+In Apollo Server 4, each element must be an `ApolloServerPlugin`. If you used a factory function in order to refer to the `ApolloServer` object itself when setting up your plugin, you may want to use the new `ApolloServer.addPlugin` method which you may call on your `ApolloServer` before you call `start` or `startStandaloneServer`.
+
+For example, if your Apollo Server 3 code looked like this:
+
+<MultiCodeBlock>
+
+```ts
+const server = new ApolloServer({
+  typeDefs,
+  plugins: [
+    makeFirstPlugin,
+    () => makeSecondPlugin(server),
+  ],
+});
+```
+
+</MultiCodeBlock>
+
+then your Apollo Server 4 code can look like this:
+
+<MultiCodeBlock>
+
+```ts
+const server = new ApolloServer({
+  typeDefs,
+  plugins: [
+    makeFirstPlugin(),
+  ],
+});
+server.addPlugin(makeSecondPlugin(server));
+```
+
+</MultiCodeBlock>
+
 ### Changes to plugin semantics
 
 In Apollo Server 4, `requestDidStart` hooks are called in parallel rather than in series.

packages/integration-testsuite/src/apolloServerTests.ts

@@ -33,7 +33,7 @@ import type {
   ApolloServerOptions,
   ApolloServer,
   BaseContext,
-  PluginDefinition,
+  ApolloServerPlugin,
 } from '@apollo/server';
 import fetch from 'node-fetch';
 
@@ -541,7 +541,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
       let serverInstance: ApolloServer<BaseContext>;
 
       const setupApolloServerAndFetchPairForPlugins = async (
-        plugins: PluginDefinition<BaseContext>[] = [],
+        plugins: ApolloServerPlugin<BaseContext>[] = [],
       ) => {
         const { server, url } = await createServer(
           {
@@ -878,7 +878,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
               ApolloServerPluginUsageReportingOptions<any>
             > = {},
             constructorOptions: Partial<CreateServerForIntegrationTests> = {},
-            plugins: PluginDefinition<BaseContext>[] = [],
+            plugins: ApolloServerPlugin<BaseContext>[] = [],
           ) => {
             const uri = await createServerAndGetUrl({
               typeDefs: gql`

packages/plugin-response-cache/src/ApolloServerPluginResponseCache.ts

@@ -174,7 +174,7 @@ export default function plugin<TContext extends BaseContext>(
       outerRequestContext: GraphQLRequestContext<any>,
     ): Promise<GraphQLRequestListener<any>> {
       const cache = new PrefixingKeyValueCache(
-        options.cache ?? outerRequestContext.server.cache,
+        options.cache ?? outerRequestContext.cache,
         'fqc:',
       );
 
@@ -267,7 +267,7 @@ export default function plugin<TContext extends BaseContext>(
         },
 
         async willSendResponse(requestContext) {
-          const logger = requestContext.server.logger || console;
+          const logger = requestContext.logger || console;
 
           if (!isGraphQLQuery(requestContext)) {
             return;

packages/server/src/ApolloServer.ts

@@ -42,6 +42,7 @@ import type {
   PersistedQueryOptions,
   HTTPGraphQLHead,
   ContextThunk,
+  GraphQLRequestContext,
 } from './externalTypes';
 import { runPotentiallyBatchedHttpQuery } from './httpBatching.js';
 import { InternalPluginId, pluginIsInternal } from './internalPlugin.js';
@@ -202,20 +203,6 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
 
     const isDev = nodeEnv !== 'production';
 
-    // Plugins can be (for some reason) provided as a function, which we have to
-    // call first to get the actual plugin. Note that more plugins can be added
-    // before `start()` with `addPlugin()` (eg, plugins that want to take this
-    // ApolloServer as an argument), and `start()` will call
-    // `ensurePluginInstantiation` to add default plugins.
-    const plugins: ApolloServerPlugin<TContext>[] = (config.plugins ?? []).map(
-      (plugin) => {
-        if (typeof plugin === 'function') {
-          return plugin();
-        }
-        return plugin;
-      },
-    );
-
     const state: ServerState = config.gateway
       ? // ApolloServer has been initialized but we have not yet tried to load the
         // schema from the gateway. That will wait until `start()` or
@@ -293,7 +280,10 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
       nodeEnv,
       allowBatchedHttpRequests: config.allowBatchedHttpRequests ?? false,
       apolloConfig,
-      plugins,
+      // Note that more plugins can be added before `start()` with `addPlugin()`
+      // (eg, plugins that want to take this ApolloServer as an argument), and
+      // `start()` will call `addDefaultPlugins` to add default plugins.
+      plugins: config.plugins ?? [],
       parseOptions: config.parseOptions ?? {},
       state,
       stopOnTerminationSignals: config.stopOnTerminationSignals,
@@ -377,8 +367,9 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
       });
 
       const schemaDerivedData = schemaManager.getSchemaDerivedData();
-      const service: GraphQLServerContext<TContext> = {
-        server: this,
+      const service: GraphQLServerContext = {
+        logger: this.logger,
+        cache: this.cache,
         schema: schemaDerivedData.schema,
         apollo: this.internals.apolloConfig,
         startedInBackground,
@@ -1154,8 +1145,9 @@ export async function internalExecuteOperation<TContext extends BaseContext>({
   const httpGraphQLHead = newHTTPGraphQLHead();
   httpGraphQLHead.headers.set('content-type', 'application/json');
 
-  const requestContext = {
-    server,
+  const requestContext: GraphQLRequestContext<TContext> = {
+    logger: server.logger,
+    cache: server.cache,
     schema: schemaDerivedData.schema,
     request: graphQLRequest,
     response: { result: {}, http: httpGraphQLHead },

packages/server/src/__tests__/ApolloServer.test.ts

@@ -489,19 +489,27 @@ describe('ApolloServer executeOperation', () => {
         },
       };
 
+      // A plugin that expects specific fields to be set is not a plugin that
+      // doesn't promise to set any fields.
       // @ts-expect-error
       takesPlugin<BaseContext>(specificPlugin);
-      // @ts-expect-error
+      // This is OK: plugins only get to read context, not write it, so a plugin
+      // that reads no interesting fields can be used as a plugin that is
+      // hypothetically allowed to read some interesting fields but chooses not
+      // to.
       takesPlugin<SpecificContext>(basePlugin);
 
+      // You can't use a plugin that expects specific fields to exist with a
+      // server that doesn't require them to be set when executing operations.
       new ApolloServer<BaseContext>({
         typeDefs: 'type Query { x: ID }',
         // @ts-expect-error
         plugins: [specificPlugin],
       });
+      // A plugin that doesn't expect any fields to be set works fine with a
+      // server that sets some fields when executing operations.
       new ApolloServer<SpecificContext>({
         typeDefs: 'type Query { x: ID }',
-        // @ts-expect-error
         plugins: [basePlugin],
       });
     });

packages/server/src/__tests__/logger.test.ts

@@ -15,7 +15,7 @@ describe('logger', () => {
       `,
       plugins: [
         {
-          async serverWillStart({ server: { logger } }) {
+          async serverWillStart({ logger }) {
             logger.debug(KNOWN_DEBUG_MESSAGE);
           },
         },
@@ -42,7 +42,7 @@ describe('logger', () => {
       `,
       plugins: [
         {
-          async serverWillStart({ server: { logger } }) {
+          async serverWillStart({ logger }) {
             logger.debug(KNOWN_DEBUG_MESSAGE);
           },
         },

packages/server/src/externalTypes/constructor.ts

@@ -11,7 +11,7 @@ import type {
 import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
 import type { GatewayInterface } from '@apollo/server-gateway-interface';
 import type { BaseContext } from '.';
-import type { PluginDefinition } from './plugins';
+import type { ApolloServerPlugin } from './plugins';
 
 export type DocumentStore = KeyValueCache<DocumentNode>;
 
@@ -86,7 +86,7 @@ interface ApolloServerOptionsBase<TContext extends BaseContext> {
   allowBatchedHttpRequests?: boolean;
 
   introspection?: boolean;
-  plugins?: PluginDefinition<TContext>[];
+  plugins?: ApolloServerPlugin<TContext>[];
   persistedQueries?: PersistedQueryOptions | false;
   stopOnTerminationSignals?: boolean;
   apollo?: ApolloConfigInput;

packages/server/src/externalTypes/graphql.ts

@@ -9,7 +9,8 @@ import type {
 import type { CachePolicy } from '@apollo/cache-control-types';
 import type { BaseContext } from './context';
 import type { HTTPGraphQLHead, HTTPGraphQLRequest } from './http';
-import type { ApolloServer } from '../ApolloServer';
+import type { Logger } from '@apollo/utils.logger';
+import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
 
 export interface GraphQLRequest {
   query?: string;
@@ -44,7 +45,8 @@ export interface GraphQLRequestMetrics {
 }
 
 export interface GraphQLRequestContext<TContext extends BaseContext> {
-  readonly server: ApolloServer<TContext>;
+  readonly logger: Logger;
+  readonly cache: KeyValueCache<string>;
 
   readonly request: GraphQLRequest;
   readonly response: GraphQLResponse;

packages/server/src/externalTypes/index.ts

@@ -29,7 +29,6 @@ export type {
   GraphQLServerListener,
   GraphQLServerContext,
   LandingPage,
-  PluginDefinition,
 } from './plugins.js';
 export type {
   GraphQLRequestContextDidEncounterErrors,

packages/server/src/externalTypes/plugins.ts

@@ -1,5 +1,6 @@
+import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
+import type { Logger } from '@apollo/utils.logger';
 import type { GraphQLResolveInfo, GraphQLSchema } from 'graphql';
-import type { ApolloServer } from '../ApolloServer';
 import type { ApolloConfig } from './constructor';
 import type { BaseContext } from './context';
 import type { GraphQLRequestContext, GraphQLResponse } from './graphql';
@@ -14,8 +15,10 @@ import type {
   GraphQLRequestContextWillSendResponse,
 } from './requestPipeline';
 
-export interface GraphQLServerContext<TContext extends BaseContext> {
-  server: ApolloServer<TContext>;
+export interface GraphQLServerContext {
+  readonly logger: Logger;
+  readonly cache: KeyValueCache<string>;
+
   schema: GraphQLSchema;
   apollo: ApolloConfig;
   // TODO(AS4): Make sure we document that we removed `persistedQueries`.
@@ -28,15 +31,11 @@ export interface GraphQLSchemaContext {
   coreSupergraphSdl?: string;
 }
 
-// A plugin can return an interface that matches `ApolloServerPlugin`, or a
-// factory function that returns `ApolloServerPlugin`.
-export type PluginDefinition<TContext extends BaseContext> =
-  | ApolloServerPlugin<TContext>
-  | (() => ApolloServerPlugin<TContext>);
-
-export interface ApolloServerPlugin<in out TContext extends BaseContext> {
+export interface ApolloServerPlugin<
+  in TContext extends BaseContext = BaseContext,
+> {
   serverWillStart?(
-    service: GraphQLServerContext<TContext>,
+    service: GraphQLServerContext,
   ): Promise<GraphQLServerListener | void>;
 
   requestDidStart?(

packages/server/src/plugin/cacheControl/index.ts

@@ -1,4 +1,4 @@
-import type { ApolloServerPlugin, BaseContext } from '../../externalTypes';
+import type { ApolloServerPlugin } from '../../externalTypes';
 import {
   DirectiveNode,
   getNamedType,
@@ -47,9 +47,9 @@ export interface ApolloServerPluginCacheControlOptions {
   __testing__cacheHints?: Map<string, CacheHint>;
 }
 
-export function ApolloServerPluginCacheControl<TContext extends BaseContext>(
+export function ApolloServerPluginCacheControl(
   options: ApolloServerPluginCacheControlOptions = Object.create(null),
-): ApolloServerPlugin<TContext> {
+): ApolloServerPlugin {
   let typeAnnotationCache: LRUCache<GraphQLCompositeType, CacheAnnotation>;
 
   let fieldAnnotationCache: LRUCache<

packages/server/src/plugin/disabled/index.ts

@@ -6,37 +6,27 @@ import type {
   InternalPluginId,
 } from '../../internalPlugin';
 
-function disabledPlugin<TContext extends BaseContext>(
-  id: InternalPluginId,
-): ApolloServerPlugin<TContext> {
-  const plugin: InternalApolloServerPlugin<TContext> = {
+function disabledPlugin(id: InternalPluginId): ApolloServerPlugin {
+  const plugin: InternalApolloServerPlugin<BaseContext> = {
     __internal_plugin_id__() {
       return id;
     },
   };
   return plugin;
 }
 
-export function ApolloServerPluginCacheControlDisabled<
-  TContext extends BaseContext,
->(): ApolloServerPlugin<TContext> {
+export function ApolloServerPluginCacheControlDisabled(): ApolloServerPlugin<BaseContext> {
   return disabledPlugin('CacheControl');
 }
 
-export function ApolloServerPluginInlineTraceDisabled<
-  TContext extends BaseContext,
->(): ApolloServerPlugin<TContext> {
+export function ApolloServerPluginInlineTraceDisabled(): ApolloServerPlugin<BaseContext> {
   return disabledPlugin('InlineTrace');
 }
 
-export function ApolloServerPluginLandingPageDisabled<
-  TContext extends BaseContext,
->(): ApolloServerPlugin<TContext> {
+export function ApolloServerPluginLandingPageDisabled(): ApolloServerPlugin<BaseContext> {
   return disabledPlugin('LandingPageDisabled');
 }
 
-export function ApolloServerPluginUsageReportingDisabled<
-  TContext extends BaseContext,
->(): ApolloServerPlugin<TContext> {
+export function ApolloServerPluginUsageReportingDisabled(): ApolloServerPlugin<BaseContext> {
   return disabledPlugin('UsageReporting');
 }