From 344ae03290bb51597c7f66a6396e6705db80f53e Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 18 May 2020 20:20:17 +0000 Subject: [PATCH] Add deprecation warnings for `GraphQLExtension` usage. This introduces a deprecation warning which will be emitted once per extension which is defined in the `extensions` parameter of an Apollo Server configuration. With the introduction of the last missing integration point (`willResolveField`) via the recently landed https://github.com/apollographql/apollo-server/pull/3988, the new [[Plugin API]] should now have all of the abilities (a super-set, in fact) of the prior `graphql-extensions` API. Furthermore, rather than being undocumented, rather untested and largely experimental, the new plugin API is considered stable and well-understood in terms of what it aims to support. Its documentation and API are now considered part of the Apollo Server API and we will do our best to maintain first-class support for it, including the addition of new functionality as deemed necessary. As of this commit, we will now print deprecation warnings one time per server start-up for _each_ legacy extension. Since extensions were often defined as factory functions which were invoked on each request - and expected to return an instance of the extension by calling `new` on it - we limit this deprecation warning to once per start-up by attaching a `Symbol` to the `constructor` and skipping the warning when the `Symbol` is already present. An alternative design might use a `Map` to track the `constructor`s at the module-level within `requestPipeline.ts`, but I believe this should be functionally the same. [Plugin API]: https://www.apollographql.com/docs/apollo-server/integrations/plugins/ --- .../src/__tests__/runQuery.test.ts | 83 +++++++++++++++++++ .../apollo-server-core/src/requestPipeline.ts | 46 ++++++++++ 2 files changed, 129 insertions(+) diff --git a/packages/apollo-server-core/src/__tests__/runQuery.test.ts b/packages/apollo-server-core/src/__tests__/runQuery.test.ts index 69f50590d3f..ec6d83477dc 100644 --- a/packages/apollo-server-core/src/__tests__/runQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runQuery.test.ts @@ -31,6 +31,7 @@ import { } from 'apollo-server-plugin-base'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { generateSchemaHash } from "../utils/schemaHash"; +import { Logger } from "apollo-server-types"; // This is a temporary kludge to ensure we preserve runQuery behavior with the // GraphQLRequestProcessor refactoring. @@ -399,6 +400,88 @@ describe('runQuery', () => { } } + describe('deprecation warnings', () => { + const queryString = `{ testString }`; + async function runWithExtAndReturnLogger( + extensions: QueryOptions['extensions'], + ): Promise { + const logger = { + warn: jest.fn(() => {}), + info: console.info, + debug: console.debug, + error: console.error, + }; + + await runQuery( + { + schema, + queryString, + extensions, + request: new MockReq(), + }, + { + logger, + }, + ); + + return logger; + } + + it('warns about named extensions', async () => { + const logger = await runWithExtAndReturnLogger([ + () => new (class NamedExtension implements GraphQLExtension {})(), + ]); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "NamedExtension" was/)); + }); + + it('warns about anonymous extensions', async () => { + const logger = await runWithExtAndReturnLogger([ + () => new (class implements GraphQLExtension {})(), + ]); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] An anonymous extension was/)); + }); + + it('warns about anonymous class expressions', async () => { + // In other words, when the name is the name of the variable. + const anon = class implements GraphQLExtension {}; + const logger = await runWithExtAndReturnLogger([ + () => new anon(), + ]); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "anon" was/)); + }); + + it('warns for multiple extensions', async () => { + const logger = await runWithExtAndReturnLogger([ + () => new (class Name1Ext implements GraphQLExtension {})(), + () => new (class Name2Ext implements GraphQLExtension {})(), + ]); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "Name1Ext" was/)); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "Name2Ext" was/)); + }); + + it('warns only once', async () => { + // Will use the same extension across two invocations. + class NameExt implements GraphQLExtension {}; + + const logger1 = await runWithExtAndReturnLogger([ + () => new NameExt, + ]); + expect(logger1.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "NameExt" was/)); + + const logger2 = await runWithExtAndReturnLogger([ + () => new NameExt, + ]); + expect(logger2.warn).not.toHaveBeenCalledWith( + expect.stringMatching(/^\[deprecated\] A "NameExt" was/)); + }); + }); + it('creates the extension stack', async () => { const queryString = `{ testString }`; const extensions = [() => new CustomExtension()]; diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 09d4d4517fe..60962277b0c 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -113,6 +113,14 @@ export type DataSources = { type Mutable = { -readonly [P in keyof T]: T[P] }; +/** + * We attach this symbol to the constructor of extensions to mark that we've + * already warned about the deprecation of the `graphql-extensions` API for that + * particular definition. + */ +const symbolExtensionDeprecationDone = + Symbol("apolloServerExtensionDeprecationDone"); + export async function processGraphQLRequest( config: GraphQLRequestPipelineConfig, requestContext: Mutable>, @@ -634,6 +642,44 @@ export async function processGraphQLRequest( // objects. const extensions = config.extensions ? config.extensions.map(f => f()) : []; + // Warn about usage of (deprecated) `graphql-extensions` implementations. + // Since extensions are often provided as factory functions which + // instantiate an extension on each request, we'll attach a symbol to the + // constructor after we've warned to ensure that we don't do it on each + // request. Another option here might be to keep a `Map` of constructor + // instances within this module, but I hope this will do the trick. + const hasOwn = Object.prototype.hasOwnProperty; + extensions.forEach((extension) => { + // Using `hasOwn` just in case there is a user-land `hasOwnProperty` + // defined on the `constructor` object. + if ( + !extension.constructor || + hasOwn.call(extension.constructor, symbolExtensionDeprecationDone) + ) { + return; + } + + Object.defineProperty( + extension.constructor, + symbolExtensionDeprecationDone, + { value: true } + ); + + const extensionName = extension.constructor.name; + logger.warn( + '[deprecated] ' + + (extensionName + ? 'A "' + extensionName + '" ' + : 'An anonymous extension ') + + 'was defined within the "extensions" configuration for ' + + 'Apollo Server. The API on which this extension is built ' + + '("graphql-extensions") is being deprecated in the next major ' + + 'version of Apollo Server in favor of the new plugin API. See ' + + 'https://go.apollo.dev/s/plugins for the documentation on how ' + + 'these plugins are to be defined and used.', + ); + }); + return new GraphQLExtensionStack(extensions); }