federation icon indicating copy to clipboard operation
federation copied to clipboard

[version-0.x] Fix TS issues with gateway 0.29+ in AS2

Open smyrick opened this issue 2 years ago • 6 comments

If you use gateway version 0.29.0+ with Apollo Server 2 you will start relying on apollo-server-types both ^0.9.0 OR ^3.0.0. See code

[email protected] -> AS2 [email protected] -> AS3

https://github.com/apollographql/federation/blob/7f5565aedefa05f0453985336a24d3e3d8aebd2f/gateway-js/package.json#L41

Normally in dependencies this would be ok, that we can use either version, however the underlying dependency apollo-server-env also has a major version change. We updated the GraphQLRequest.headers to instead defined in apollo-server-env to be the Headers from node-fetch. This was an unintentional "breaking change" that got populated. It appears to have no affect on runtime of most users but what it does break is Typescript compile steps. So if users have a tsc step they can't even use these versions together.

My fix is to instead have npm pull in the dependency of one or the other, we pull in both libs and define in the typed source code that the types themselves could be one or the other.

I believe we should also apply this fix to main if we want @apollo/gateway v2 to still work with AS2.

smyrick avatar Jul 06 '22 19:07 smyrick

Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name Link
Latest commit e1540c92b5a40ab98dc09878b882f4650cb682ea

netlify[bot] avatar Jul 06 '22 19:07 netlify[bot]

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.

codesandbox-ci[bot] avatar Jul 06 '22 19:07 codesandbox-ci[bot]

I could use some other eyes on this because I am not sure if this is still considered a major change since I did update the typing info in a lot of places

smyrick avatar Jul 06 '22 20:07 smyrick

Here's the diff of this code vs @apollo/[email protected]. It seems like mostly types, other than a few JS changes (which were changed in the typescript code, so they make sense).

Mind double-checking to make sure I didn't miss anything?

diff -bur --exclude '*.map' dist/config.d.ts dist-shane/config.d.ts
--- dist/config.d.ts	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/config.d.ts	2022-07-07 19:01:24.000000000 -0500
@@ -1,9 +1,8 @@
 import { GraphQLError, GraphQLSchema } from 'graphql';
 import { HeadersInit } from 'node-fetch';
-import { GraphQLRequestContextExecutionDidStart } from 'apollo-server-types';
 import type { Logger } from '@apollo/utils.logger';
 import { ServiceDefinition } from '@apollo/federation';
-import { GraphQLDataSource } from './datasources/types';
+import { GraphQLDataSource, GraphQLRequestContextExecutionDidStart } from './datasources/types';
 import { QueryPlan } from '@apollo/query-planner';
 import { OperationContext } from './operationContext';
 import { ServiceMap } from './executeQueryPlan';
diff -bur --exclude '*.map' dist/datasources/RemoteGraphQLDataSource.d.ts dist-shane/datasources/RemoteGraphQLDataSource.d.ts
--- dist/datasources/RemoteGraphQLDataSource.d.ts	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/datasources/RemoteGraphQLDataSource.d.ts	2022-07-07 19:01:24.000000000 -0500
@@ -1,8 +1,13 @@
-import { GraphQLRequestContext, GraphQLResponse, ValueOrPromise } from 'apollo-server-types';
+import { ValueOrPromise } from 'apollo-server-types';
 import { ApolloError } from 'apollo-server-errors';
-import { GraphQLDataSource, GraphQLDataSourceProcessOptions } from './types';
+import { GraphQLDataSource, GraphQLDataSourceProcessOptions, GraphQLRequest, GraphQLResponse } from './types';
 import { Request as NodeFetchRequest } from 'node-fetch';
 import { Fetcher, FetcherResponse } from '@apollo/utils.fetcher';
+export declare type RequiredGraphQLRequestContext<TContext> = {
+    request: GraphQLRequest;
+    response: GraphQLResponse;
+    context: TContext;
+};
 export declare class RemoteGraphQLDataSource<TContext extends Record<string, any> = Record<string, any>> implements GraphQLDataSource<TContext> {
     fetcher: Fetcher;
     constructor(config?: Partial<RemoteGraphQLDataSource<TContext>> & object & ThisType<RemoteGraphQLDataSource<TContext>>);
@@ -13,7 +18,7 @@
     private sendRequest;
     willSendRequest?(options: GraphQLDataSourceProcessOptions<TContext>): ValueOrPromise<void>;
     private respond;
-    didReceiveResponse?(requestContext: Required<Pick<GraphQLRequestContext<TContext>, 'request' | 'response' | 'context'>>): ValueOrPromise<GraphQLResponse>;
+    didReceiveResponse?(requestContext: RequiredGraphQLRequestContext<TContext>): ValueOrPromise<GraphQLResponse>;
     didEncounterError(error: Error, _fetchRequest: NodeFetchRequest, _fetchResponse?: FetcherResponse, _context?: TContext): void;
     parseBody(fetchResponse: FetcherResponse, _fetchRequest?: NodeFetchRequest, _context?: TContext): Promise<object | string>;
     errorFromResponse(response: FetcherResponse): Promise<ApolloError>;
diff -bur --exclude '*.map' dist/datasources/RemoteGraphQLDataSource.js dist-shane/datasources/RemoteGraphQLDataSource.js
--- dist/datasources/RemoteGraphQLDataSource.js	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/datasources/RemoteGraphQLDataSource.js	2022-07-07 19:01:24.000000000 -0500
@@ -4,7 +4,7 @@
 };
 Object.defineProperty(exports, "__esModule", { value: true });
 exports.RemoteGraphQLDataSource = void 0;
-const apollo_server_types_1 = require("apollo-server-types");
+const apollo_server_types_3_1 = require("apollo-server-types-3");
 const apollo_server_errors_1 = require("apollo-server-errors");
 const predicates_1 = require("../utilities/predicates");
 const types_1 = require("./types");
@@ -47,10 +47,11 @@
             throw new Error('Missing query');
         }
         const { query, ...requestWithoutQuery } = request;
+        const cachePolicy = (_b = options.incomingRequestContext) === null || _b === void 0 ? void 0 : _b.overallCachePolicy;
         const overallCachePolicy = this.honorSubgraphCacheControlHeader &&
             options.kind === types_1.GraphQLDataSourceRequestKind.INCOMING_OPERATION &&
-            ((_b = options.incomingRequestContext.overallCachePolicy) === null || _b === void 0 ? void 0 : _b.restrict)
-            ? options.incomingRequestContext.overallCachePolicy
+            (cachePolicy === null || cachePolicy === void 0 ? void 0 : cachePolicy.restrict)
+            ? cachePolicy
             : null;
         if (this.apq) {
             const apqHash = (0, utils_createhash_1.createHash)('sha256').update(request.query).digest('hex');
@@ -129,10 +130,10 @@
                 hint.maxAge = +maxAge;
             }
             if (parsed['private'] === true) {
-                hint.scope = apollo_server_types_1.CacheScope.Private;
+                hint.scope = apollo_server_types_3_1.CacheScope.Private;
             }
             if (parsed['public'] === true) {
-                hint.scope = apollo_server_types_1.CacheScope.Public;
+                hint.scope = apollo_server_types_3_1.CacheScope.Public;
             }
             overallCachePolicy.restrict(hint);
         }
diff -bur --exclude '*.map' dist/datasources/types.d.ts dist-shane/datasources/types.d.ts
--- dist/datasources/types.d.ts	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/datasources/types.d.ts	2022-07-07 19:01:24.000000000 -0500
@@ -1,4 +1,9 @@
-import { GraphQLResponse, GraphQLRequestContext } from 'apollo-server-types';
+import { GraphQLRequest as GraphQLRequest1, GraphQLResponse as GraphQLResponse1, GraphQLRequestContext as GraphQLRequestContext1, GraphQLRequestContextExecutionDidStart as GraphQLRequestContextExecutionDidStart1 } from 'apollo-server-types';
+import { GraphQLRequest as GraphQLRequest3, GraphQLResponse as GraphQLResponse3, GraphQLRequestContext as GraphQLRequestContext3, GraphQLRequestContextExecutionDidStart as GraphQLRequestContextExecutionDidStart3 } from 'apollo-server-types-3';
+export declare type GraphQLRequest = GraphQLRequest1 | GraphQLRequest3;
+export declare type GraphQLResponse = GraphQLResponse1 | GraphQLResponse3;
+export declare type GraphQLRequestContext<TContext> = GraphQLRequestContext1<TContext> | GraphQLRequestContext3<TContext>;
+export declare type GraphQLRequestContextExecutionDidStart<TContext> = GraphQLRequestContextExecutionDidStart1<TContext> | GraphQLRequestContextExecutionDidStart3<TContext>;
 export interface GraphQLDataSource<TContext extends Record<string, any> = Record<string, any>> {
     process(options: GraphQLDataSourceProcessOptions<TContext>): Promise<GraphQLResponse>;
 }
@@ -8,7 +13,7 @@
     LOADING_SCHEMA = "loading schema"
 }
 export declare type GraphQLDataSourceProcessOptions<TContext extends Record<string, any> = Record<string, any>> = {
-    request: GraphQLRequestContext<TContext>['request'];
+    request: GraphQLRequest;
 } & ({
     kind: GraphQLDataSourceRequestKind.INCOMING_OPERATION;
     incomingRequestContext: GraphQLRequestContext<TContext>;
diff -bur --exclude '*.map' dist/executeQueryPlan.d.ts dist-shane/executeQueryPlan.d.ts
--- dist/executeQueryPlan.d.ts	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/executeQueryPlan.d.ts	2022-07-07 19:01:24.000000000 -0500
@@ -1,6 +1,6 @@
-import { GraphQLExecutionResult, GraphQLRequestContext, VariableValues } from 'apollo-server-types';
+import { GraphQLExecutionResult, VariableValues } from 'apollo-server-types';
 import { GraphQLFieldResolver } from 'graphql';
-import { GraphQLDataSource } from './datasources/types';
+import { GraphQLDataSource, GraphQLRequestContext } from './datasources/types';
 import { OperationContext } from './operationContext';
 import { FetchNode, QueryPlan } from '@apollo/query-planner';
 export declare type ServiceMap = {
diff -bur --exclude '*.map' dist/index.d.ts dist-shane/index.d.ts
--- dist/index.d.ts	1985-10-26 03:15:00.000000000 -0500
+++ dist-shane/index.d.ts	2022-07-07 19:01:24.000000000 -0500
@@ -1,9 +1,9 @@
 import { GraphQLService, Unsubscriber } from 'apollo-server-core';
-import { GraphQLExecutionResult, GraphQLRequestContextExecutionDidStart } from 'apollo-server-types';
+import { GraphQLExecutionResult } from 'apollo-server-types';
 import { GraphQLSchema } from 'graphql';
 import { buildOperationContext } from './operationContext';
 import { executeQueryPlan, ServiceMap } from './executeQueryPlan';
-import { GraphQLDataSource } from './datasources/types';
+import { GraphQLDataSource, GraphQLRequestContextExecutionDidStart } from './datasources/types';
 import { ServiceEndpointDefinition, Experimental_DidFailCompositionCallback, Experimental_DidResolveQueryPlanCallback, Experimental_DidUpdateSupergraphCallback, Experimental_UpdateComposition, CompositionInfo, GatewayConfig } from './config';
 import { IntrospectAndCompose, LocalCompose } from './supergraphManagers';
 declare type DataSourceMap = {
@@ -81,7 +81,7 @@
     private updateWithSchemaAndNotify;
     serviceHealthCheck(serviceMap?: DataSourceMap): Promise<{
         name: string;
-        response: import("apollo-server-types").GraphQLResponse;
+        response: import("./datasources/types").GraphQLResponse;
     }[]>;
     private serviceListFromSupergraphSdl;
     private serviceListFromComposedSchema;

benweatherman avatar Jul 08 '22 00:07 benweatherman

You can reproduce the error here running npm run compile

https://stackblitz.com/edit/apolloserver-2-with-gateway-45?file=README.md

I am pulling this setup and this PR package to test locally

smyrick avatar Jul 12 '22 20:07 smyrick

I had to fix the types in make-fetch-happen first: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/61233

smyrick avatar Jul 19 '22 18:07 smyrick

This PR has drastically been affected by this PR: https://github.com/apollographql/federation/pull/2050

While using gateway 0.52.0 does not solve the TS issues with Apollo Server 2 the changes have become so big that I am going to completely open a new branch. The changes will be here for reference though

smyrick avatar Aug 15 '22 19:08 smyrick