Remove info from ExecutionParams and add operationType (#3166)

* Remove info from ExecutionParams and add operationType * Fix tests * Allow getBatchingExecutor to cache executor by memoizing default executor * Memoize for a single so allow to memoize correctly. * Rename Request to ExecutionRequest * Remove unnecessary diff * Remove unnecessary operationName from DelegationContext * Add `context` in `createRequest` and `createRequestInfo` instead of `delegateToSchema` * Get context from request * Do not export extra types from delegate * Add info back * Fix typings
ardatan · Jul 12, 2021 · c42e811 · c42e811
1 parent a0ff8f3
commit c42e811
Show file tree

Hide file tree

Showing 51 changed files with 266 additions and 219 deletions.
diff --git a/.changeset/lazy-turtles-dress.md b/.changeset/lazy-turtles-dress.md
@@ -0,0 +1,25 @@
+---
+'@graphql-tools/batch-execute': major
+'@graphql-tools/delegate': major
+'@graphql-tools/links': major
+'@graphql-tools/utils': major
+'@graphql-tools/wrap': major
+---
+
+BREAKING CHANGES;
+
+- Rename `Request` to `ExecutionRequest`
+- Drop unnecessary `GraphQLResolveInfo` in `ExecutionRequest`
+- Add required `operationType: OperationTypeNode` field in `ExecutionRequest`
+- Add `context` in `createRequest` and `createRequestInfo` instead of `delegateToSchema`
+
+> It doesn't rely on info.operation.operationType to allow the user to call an operation from different root type.
+And it doesn't call getOperationAST again and again to get operation type from the document/operation because we have it in Request and ExecutionParams
+https://github.com/ardatan/graphql-tools/pull/3166/files#diff-d4824895ea613dcc1f710c3ac82e952fe0ca12391b671f70d9f2d90d5656fdceR38
+
+Improvements;
+- Memoize `defaultExecutor` for a single `GraphQLSchema` so allow `getBatchingExecutor` to memoize `batchingExecutor` correctly.
+- And there is no different `defaultExecutor` is created for `subscription` and other operation types. Only one executor is used.
+
+> Batch executor is memoized by `executor` reference but `createDefaultExecutor` didn't memoize the default executor so this memoization wasn't working correctly on `batch-execute` side.
+https://github.com/ardatan/graphql-tools/blob/remove-info-executor/packages/batch-execute/src/getBatchingExecutor.ts#L9
diff --git a/packages/batch-execute/src/createBatchingExecutor.ts b/packages/batch-execute/src/createBatchingExecutor.ts
@@ -1,10 +1,8 @@
-import { getOperationAST } from 'graphql';
-
 import DataLoader from 'dataloader';
 
 import { ValueOrPromise } from 'value-or-promise';
 
-import { Request, Executor, ExecutionResult } from '@graphql-tools/utils';
+import { ExecutionRequest, Executor, ExecutionResult } from '@graphql-tools/utils';
 
 import { mergeRequests } from './mergeRequests';
 import { splitResult } from './splitResult';
@@ -14,32 +12,30 @@ export function createBatchingExecutor(
   dataLoaderOptions?: DataLoader.Options<any, any, any>,
   extensionsReducer: (
     mergedExtensions: Record<string, any>,
-    request: Request
+    request: ExecutionRequest
   ) => Record<string, any> = defaultExtensionsReducer
 ): Executor {
   const loader = new DataLoader(createLoadFn(executor, extensionsReducer), dataLoaderOptions);
-  return (request: Request) =>
-    request.info?.operation.operation === 'subscription' ? executor(request) : loader.load(request);
+  return (request: ExecutionRequest) => {
+    return request.operationType === 'subscription' ? executor(request) : loader.load(request);
+  };
 }
 
 function createLoadFn(
   executor: Executor,
-  extensionsReducer: (mergedExtensions: Record<string, any>, request: Request) => Record<string, any>
+  extensionsReducer: (mergedExtensions: Record<string, any>, request: ExecutionRequest) => Record<string, any>
 ) {
-  return async (requests: ReadonlyArray<Request>): Promise<Array<ExecutionResult>> => {
-    const execBatches: Array<Array<Request>> = [];
+  return async (requests: ReadonlyArray<ExecutionRequest>): Promise<Array<ExecutionResult>> => {
+    const execBatches: Array<Array<ExecutionRequest>> = [];
     let index = 0;
     const request = requests[index];
-    let currentBatch: Array<Request> = [request];
+    let currentBatch: Array<ExecutionRequest> = [request];
     execBatches.push(currentBatch);
 
-    const operationType = getOperationAST(request.document, undefined)?.operation;
-    if (operationType == null) {
-      throw new Error('Could not identify operation type of document.');
-    }
+    const operationType = request.operationType;
 
     while (++index < requests.length) {
-      const currentOperationType = getOperationAST(requests[index].document, undefined)?.operation;
+      const currentOperationType = requests[index].operationType;
       if (operationType == null) {
         throw new Error('Could not identify operation type of document.');
       }
@@ -68,7 +64,10 @@ function createLoadFn(
   };
 }
 
-function defaultExtensionsReducer(mergedExtensions: Record<string, any>, request: Request): Record<string, any> {
+function defaultExtensionsReducer(
+  mergedExtensions: Record<string, any>,
+  request: ExecutionRequest
+): Record<string, any> {
   const newExtensions = request.extensions;
   if (newExtensions != null) {
     Object.assign(mergedExtensions, newExtensions);

diff --git a/packages/batch-execute/src/getBatchingExecutor.ts b/packages/batch-execute/src/getBatchingExecutor.ts
@@ -1,14 +1,16 @@
 import DataLoader from 'dataloader';
 
-import { Request, Executor } from '@graphql-tools/utils';
+import { ExecutionRequest, Executor } from '@graphql-tools/utils';
 import { createBatchingExecutor } from './createBatchingExecutor';
 import { memoize2of4 } from './memoize';
 
-export const getBatchingExecutor = memoize2of4(function (
+export const getBatchingExecutor = memoize2of4(function getBatchingExecutor(
   _context: Record<string, any>,
   executor: Executor,
   dataLoaderOptions?: DataLoader.Options<any, any, any> | undefined,
-  extensionsReducer?: undefined | ((mergedExtensions: Record<string, any>, request: Request) => Record<string, any>)
+  extensionsReducer?:
+    | undefined
+    | ((mergedExtensions: Record<string, any>, request: ExecutionRequest) => Record<string, any>)
 ): Executor {
   return createBatchingExecutor(executor, dataLoaderOptions, extensionsReducer);
 });
diff --git a/packages/batch-execute/src/mergeRequests.ts b/packages/batch-execute/src/mergeRequests.ts
@@ -15,10 +15,9 @@ import {
   ASTKindToNode,
   InlineFragmentNode,
   FieldNode,
-  OperationTypeNode,
 } from 'graphql';
 
-import { Request, Maybe } from '@graphql-tools/utils';
+import { ExecutionRequest } from '@graphql-tools/utils';
 
 import { createPrefix } from './prefix';
 
@@ -57,24 +56,21 @@ import { createPrefix } from './prefix';
  *   }
  */
 export function mergeRequests(
-  requests: Array<Request>,
-  extensionsReducer: (mergedExtensions: Record<string, any>, request: Request) => Record<string, any>
-): Request {
+  requests: Array<ExecutionRequest>,
+  extensionsReducer: (mergedExtensions: Record<string, any>, request: ExecutionRequest) => Record<string, any>
+): ExecutionRequest {
   const mergedVariables: Record<string, any> = Object.create(null);
   const mergedVariableDefinitions: Array<VariableDefinitionNode> = [];
   const mergedSelections: Array<SelectionNode> = [];
   const mergedFragmentDefinitions: Array<FragmentDefinitionNode> = [];
   let mergedExtensions: Record<string, any> = Object.create(null);
 
-  let operation: Maybe<OperationTypeNode>;
-
   for (const index in requests) {
     const request = requests[index];
     const prefixedRequests = prefixRequest(createPrefix(index), request);
 
     for (const def of prefixedRequests.document.definitions) {
       if (isOperationDefinition(def)) {
-        operation = def.operation;
         mergedSelections.push(...def.selectionSet.selections);
         if (def.variableDefinitions) {
           mergedVariableDefinitions.push(...def.variableDefinitions);
@@ -88,13 +84,9 @@ export function mergeRequests(
     mergedExtensions = extensionsReducer(mergedExtensions, request);
   }
 
-  if (operation == null) {
-    throw new Error('Could not identify operation type. Did the document only include fragment definitions?');
-  }
-
   const mergedOperationDefinition: OperationDefinitionNode = {
     kind: Kind.OPERATION_DEFINITION,
-    operation,
+    operation: requests[0].operationType,
     variableDefinitions: mergedVariableDefinitions,
     selectionSet: {
       kind: Kind.SELECTION_SET,
@@ -111,10 +103,11 @@ export function mergeRequests(
     extensions: mergedExtensions,
     context: requests[0].context,
     info: requests[0].info,
+    operationType: requests[0].operationType,
   };
 }
 
-function prefixRequest(prefix: string, request: Request): Request {
+function prefixRequest(prefix: string, request: ExecutionRequest): ExecutionRequest {
   let document = aliasTopLevelFields(prefix, request.document);
   const executionVariables = request.variables ?? {};
   const variableNames = Object.keys(executionVariables);
@@ -137,6 +130,7 @@ function prefixRequest(prefix: string, request: Request): Request {
   return {
     document,
     variables: prefixedVariables,
+    operationType: request.operationType,
   };
 }
 

diff --git a/packages/delegate/src/Transformer.ts b/packages/delegate/src/Transformer.ts
@@ -1,4 +1,4 @@
-import { Request, ExecutionResult } from '@graphql-tools/utils';
+import { ExecutionRequest, ExecutionResult } from '@graphql-tools/utils';
 
 import { DelegationContext, DelegationBinding, Transform } from './types';
 
@@ -25,9 +25,9 @@ export class Transformer<TContext = Record<string, any>> {
     this.transformations.push({ transform, context });
   }
 
-  public transformRequest(originalRequest: Request) {
+  public transformRequest(originalRequest: ExecutionRequest) {
     return this.transformations.reduce(
-      (request: Request, transformation: Transformation) =>
+      (request: ExecutionRequest, transformation: Transformation) =>
         transformation.transform.transformRequest != null
           ? transformation.transform.transformRequest(request, this.delegationContext, transformation.context)
           : request,

diff --git a/packages/delegate/src/createRequest.ts b/packages/delegate/src/createRequest.ts
@@ -16,7 +16,7 @@ import {
   DocumentNode,
 } from 'graphql';
 
-import { Request, serializeInputValue, updateArgument } from '@graphql-tools/utils';
+import { ExecutionRequest, serializeInputValue, updateArgument } from '@graphql-tools/utils';
 import { ICreateRequestFromInfo, ICreateRequest } from './types';
 
 export function getDelegatingOperation(parentType: GraphQLObjectType, schema: GraphQLSchema): OperationTypeNode {
@@ -37,7 +37,8 @@ export function createRequestFromInfo({
   fieldName = info.fieldName,
   selectionSet,
   fieldNodes = info.fieldNodes,
-}: ICreateRequestFromInfo): Request {
+  context,
+}: ICreateRequestFromInfo): ExecutionRequest {
   return createRequest({
     sourceSchema: info.schema,
     sourceParentType: info.parentType,
@@ -51,6 +52,8 @@ export function createRequestFromInfo({
     targetFieldName: fieldName,
     selectionSet,
     fieldNodes,
+    context,
+    info,
   });
 }
 
@@ -67,7 +70,9 @@ export function createRequest({
   targetFieldName,
   selectionSet,
   fieldNodes,
-}: ICreateRequest): Request {
+  context,
+  info,
+}: ICreateRequest): ExecutionRequest {
   let newSelectionSet: SelectionSetNode | undefined;
   let argumentNodeMap: Record<string, ArgumentNode>;
 
@@ -176,6 +181,9 @@ export function createRequest({
     variables: newVariables,
     rootValue: targetRootValue,
     operationName: targetOperationName,
+    operationType: targetOperation,
+    context,
+    info,
   };
 }