Skip to content
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

Remove info from ExecutionParams and add operationType #3166

Merged
merged 12 commits into from
Jul 12, 2021
25 changes: 25 additions & 0 deletions .changeset/lazy-turtles-dress.md
Original file line number Diff line number Diff line change
@@ -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
31 changes: 15 additions & 16 deletions packages/batch-execute/src/createBatchingExecutor.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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.');
}
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 5 additions & 3 deletions packages/batch-execute/src/getBatchingExecutor.ts
Original file line number Diff line number Diff line change
@@ -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);
});
22 changes: 8 additions & 14 deletions packages/batch-execute/src/mergeRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -137,6 +130,7 @@ function prefixRequest(prefix: string, request: Request): Request {
return {
document,
variables: prefixedVariables,
operationType: request.operationType,
};
}

Expand Down
6 changes: 3 additions & 3 deletions packages/delegate/src/Transformer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Request, ExecutionResult } from '@graphql-tools/utils';
import { ExecutionRequest, ExecutionResult } from '@graphql-tools/utils';

import { DelegationContext, DelegationBinding, Transform } from './types';

Expand All @@ -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,
Expand Down
14 changes: 11 additions & 3 deletions packages/delegate/src/createRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -51,6 +52,8 @@ export function createRequestFromInfo({
targetFieldName: fieldName,
selectionSet,
fieldNodes,
context,
info,
});
}

Expand All @@ -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>;

Expand Down Expand Up @@ -176,6 +181,9 @@ export function createRequest({
variables: newVariables,
rootValue: targetRootValue,
operationName: targetOperationName,
operationType: targetOperation,
context,
info,
};
}

Expand Down
Loading