Skip to content

Commit

Permalink
Use an object, rather than positional params for willResolveField.
Browse files Browse the repository at this point in the history
This should prove to be more ergonomic, and also allow us to attach other
useful properties to this object (for either external or internal use) in
the future.

Also, adds a test to make sure we're passing _something_!

Ref: #3998 (comment)
  • Loading branch information
abernix committed May 8, 2020
1 parent 1121785 commit a926b7e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 3 deletions.
37 changes: 37 additions & 0 deletions packages/apollo-server-core/src/__tests__/runQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,43 @@ describe('runQuery', () => {
expect(executionDidEnd).toHaveBeenCalledTimes(1);
});

it('receives an object with the resolver arguments', async () => {
const willResolveField = jest.fn();
const executionDidEnd = jest.fn();
const executionDidStart = jest.fn(
(): GraphQLRequestExecutionListener => ({
willResolveField,
executionDidEnd,
}),
);

await runQuery({
schema,
context: { ourSpecialContext: true },
queryString: '{ testString }',
plugins: [
{
requestDidStart() {
return {
executionDidStart,
};
},
},
],
request: new MockReq(),
});

expect(executionDidStart).toHaveBeenCalledTimes(1);
expect(willResolveField).toHaveBeenCalledTimes(1);
expect(willResolveField).toHaveBeenNthCalledWith(1, {
source: undefined,

This comment has been minimized.

Copy link
@glasser

glasser May 8, 2020

Member

probably worth making a slightly more complex query so we can have a non-undefined source and args?

This comment has been minimized.

Copy link
@abernix

abernix May 11, 2020

Author Member

Done with 41b103e

args: {},
context: expect.objectContaining({ ourSpecialContext: true }),
info: expect.objectContaining({ fieldName: 'testString' }),
});
expect(executionDidEnd).toHaveBeenCalledTimes(1);
});

it('calls the end handler', async () => {
const didResolveField: GraphQLRequestListenerDidResolveField =
jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function wrapField(field: GraphQLField<any, any>): void {
// resolution is complete.
const didResolveField =
typeof willResolveField === 'function' &&
willResolveField(source, args, context, info);
willResolveField({ source, args, context, info });

const resolveObject: GraphQLObjectResolver<
any,
Expand Down
5 changes: 3 additions & 2 deletions packages/apollo-server-plugin-base/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
GraphQLResponse,
ValueOrPromise,
WithRequired,
GraphQLFieldResolverParams,
GraphQLRequestContextDidResolveSource,
GraphQLRequestContextParsingDidStart,
GraphQLRequestContextValidationDidStart,
Expand All @@ -16,7 +17,6 @@ import {
GraphQLRequestContextExecutionDidStart,
GraphQLRequestContextWillSendResponse,
} from 'apollo-server-types';
import { GraphQLFieldResolver } from "graphql";

// We re-export all of these so plugin authors only need to depend on a single
// package. The overall concept of `apollo-server-types` and this package
Expand All @@ -34,6 +34,7 @@ export {
GraphQLResponse,
ValueOrPromise,
WithRequired,
GraphQLFieldResolverParams,
GraphQLRequestContextDidResolveSource,
GraphQLRequestContextParsingDidStart,
GraphQLRequestContextValidationDidStart,
Expand Down Expand Up @@ -102,6 +103,6 @@ export interface GraphQLRequestExecutionListener<
> extends AnyFunctionMap {
executionDidEnd?: GraphQLRequestListenerExecutionDidEnd;
willResolveField?(
...fieldResolverArgs: Parameters<GraphQLFieldResolver<any, TContext>>
fieldResolverParams: GraphQLFieldResolverParams<any, TContext>
): GraphQLRequestListenerDidResolveField | void;
}
22 changes: 22 additions & 0 deletions packages/apollo-server-types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
OperationDefinitionNode,
DocumentNode,
GraphQLError,
GraphQLResolveInfo,
} from 'graphql';

// This seems like it could live in this package too.
Expand Down Expand Up @@ -147,6 +148,27 @@ export type Logger = {
error(message?: any): void;
}

/**
* This is an object form of the parameters received by typical
* `graphql-js` resolvers. The function type is `GraphQLFieldResolver`
* and normally uses positional parameters. In order to facilitate better
* ergonomics in the Apollo Server plugin API, these have been converted to
* named properties on the object using their names from the upstream
* `GraphQLFieldResolver` type signature. Ergonomic wins, in this case,
* include not needing to have three unused variables in scope just because
* there was a need to access the `info` property in a wrapped plugin.
*/
export type GraphQLFieldResolverParams<
TSource,
TContext,
TArgs = { [argName: string]: any }
> = {
source: TSource;
args: TArgs;
context: TContext;
info: GraphQLResolveInfo;
};

export type GraphQLRequestContextDidResolveSource<TContext> =
WithRequired<GraphQLRequestContext<TContext>,
| 'metrics'
Expand Down

0 comments on commit a926b7e

Please sign in to comment.