From 639b9d8fe9f17f114c50581b6c5af2e14c1e9aec Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 12 Jun 2022 21:07:32 +0300 Subject: [PATCH 1/4] refactor: move `assertValidExecutionArguments` into `buildExecutionContext` `assertValidExecutionArguments` is only called immediately prior to `buildExecutionContext` and `buildExecutionContext` is never called without calling `assertValidExecutionArguments` immediately prior. --- src/execution/execute.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 67905366c1..b72d5ecfa4 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -162,10 +162,7 @@ export interface ExecutionArgs { * a GraphQLError will be thrown immediately explaining the invalid input. */ export function execute(args: ExecutionArgs): PromiseOrValue { - const { schema, document, variableValues, rootValue } = args; - - // If arguments are missing or incorrect, throw an error. - assertValidExecutionArguments(schema, document, variableValues); + const { rootValue } = args; // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. @@ -281,6 +278,9 @@ export function buildExecutionContext( subscribeFieldResolver, } = args; + // If arguments are missing or incorrect, throw an error. + assertValidExecutionArguments(schema, document, rawVariableValues); + let operation: OperationDefinitionNode | undefined; const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { @@ -1117,10 +1117,6 @@ export function createSourceEventStream( subscribeFieldResolver, } = args; - // If arguments are missing or incorrectly typed, this is an internal - // developer mistake which should throw an early error. - assertValidExecutionArguments(schema, document, variableValues); - // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. const exeContext = buildExecutionContext({ From 2332de50be1271085341bbb40aa0ec8ad86d77cc Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 12 Jun 2022 21:11:10 +0300 Subject: [PATCH 2/4] refactor: remove `rootValue` argument from `executeOperation` `executeOperation` is never called with a `rootValue` different from `exeContext.rootValue` and `rootValue` is not utilized in the calling code except to pass its value to `executeOperation`. Rather than using an additional argument to `executeOperation`, the `rootValue`, the `rootValue` can be selected from the `exeContext` within `executeOperation` --- src/execution/execute.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index b72d5ecfa4..3d82908cc6 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -162,8 +162,6 @@ export interface ExecutionArgs { * a GraphQLError will be thrown immediately explaining the invalid input. */ export function execute(args: ExecutionArgs): PromiseOrValue { - const { rootValue } = args; - // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. const exeContext = buildExecutionContext(args); @@ -186,7 +184,7 @@ export function execute(args: ExecutionArgs): PromiseOrValue { // in this case is the entire response. try { const { operation } = exeContext; - const result = executeOperation(exeContext, operation, rootValue); + const result = executeOperation(exeContext, operation); if (isPromise(result)) { return result.then( (data) => buildResponse(data, exeContext.errors), @@ -349,7 +347,6 @@ export function buildExecutionContext( function executeOperation( exeContext: ExecutionContext, operation: OperationDefinitionNode, - rootValue: unknown, ): PromiseOrValue | null> { const rootType = exeContext.schema.getRootType(operation.operation); if (rootType == null) { @@ -368,6 +365,8 @@ function executeOperation( ); const path = undefined; + const { rootValue } = exeContext; + switch (operation.operation) { case OperationTypeNode.QUERY: return executeFields(exeContext, rootType, rootValue, path, rootFields); From 3a174070c06c0da96a56f559c84ead50333a30fe Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 12 Jun 2022 21:14:10 +0300 Subject: [PATCH 3/4] refactor: pass through args to buildExecutionContext --- src/execution/execute.ts | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 3d82908cc6..a310e3869e 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1106,27 +1106,9 @@ function mapSourceToResponse( export function createSourceEventStream( args: ExecutionArgs, ): PromiseOrValue | ExecutionResult> { - const { - schema, - document, - rootValue, - contextValue, - variableValues, - operationName, - subscribeFieldResolver, - } = args; - // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. - const exeContext = buildExecutionContext({ - schema, - document, - rootValue, - contextValue, - variableValues, - operationName, - subscribeFieldResolver, - }); + const exeContext = buildExecutionContext(args); // Return early errors if execution context failed. if (!('schema' in exeContext)) { From 15dafb8370bdd4134124d59ea4be92adeff1867c Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 12 Jun 2022 23:06:12 +0300 Subject: [PATCH 4/4] refactor: remove null from executeOperation return type not actually possible! --- src/execution/execute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a310e3869e..b66b434300 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -347,7 +347,7 @@ export function buildExecutionContext( function executeOperation( exeContext: ExecutionContext, operation: OperationDefinitionNode, -): PromiseOrValue | null> { +): PromiseOrValue> { const rootType = exeContext.schema.getRootType(operation.operation); if (rootType == null) { throw new GraphQLError(