Skip to content

Commit

Permalink
Remove support for positional args in graphql/execute/subscribe func (g…
Browse files Browse the repository at this point in the history 
…raphql#2904)

Closes graphql#2301
  • Loading branch information
@IvanGoncharov
IvanGoncharov committed Feb 20, 2021
1 parent ca589b6 commit f9534f3
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 260 deletions.
24 changes: 1 addition & 23 deletions src/__tests__/starWarsQuery-test.js
View file
@@ -1,7 +1,7 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { graphql, graphqlSync } from '../graphql';
import { graphql } from '../graphql';

import { StarWarsSchema as schema } from './starWarsSchema';

Expand All @@ -26,28 +26,6 @@ describe('Star Wars Query Tests', () => {
});
});

it('Accepts positional arguments to graphql()', async () => {
const source = `
query HeroNameQuery {
hero {
name
}
}
`;

const result = await graphql(schema, source);
expect(result).to.deep.equal({
data: {
hero: {
name: 'R2-D2',
},
},
});

const syncResult = graphqlSync(schema, source);
expect(syncResult).to.deep.equal(result);
});

it('Allows us to query for the ID and friends of R2-D2', async () => {
const source = `
query HeroNameAndFriendsQuery {
Expand Down
24 changes: 0 additions & 24 deletions src/execution/__tests__/executor-test.js
View file
Expand Up @@ -69,30 +69,6 @@ describe('Execute: Handles basic execution tasks', () => {
);
});

it('accepts positional arguments', () => {
const schema = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Type',
fields: {
a: {
type: GraphQLString,
resolve(rootValue) {
return rootValue;
},
},
},
}),
});

const document = parse('{ a }');
const rootValue = 'rootValue';
const result = execute(schema, document, rootValue);

expect(result).to.deep.equal({
data: { a: 'rootValue' },
});
});

it('executes arbitrary code', async () => {
const data = {
a: () => 'Apple',
Expand Down
77 changes: 17 additions & 60 deletions src/execution/execute.js
View file
Expand Up @@ -146,67 +146,8 @@ export type ExecutionArgs = {|
*
* If the arguments to this function do not result in a legal execution context,
* a GraphQLError will be thrown immediately explaining the invalid input.
*
* Accepts either an object with named arguments, or individual arguments.
*/
declare function execute(
ExecutionArgs,
..._: []
): PromiseOrValue<ExecutionResult>;
/* eslint-disable no-redeclare */
declare function execute(
schema: GraphQLSchema,
document: DocumentNode,
rootValue?: mixed,
contextValue?: mixed,
variableValues?: ?{ +[variable: string]: mixed, ... },
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
typeResolver?: ?GraphQLTypeResolver<any, any>,
): PromiseOrValue<ExecutionResult>;
export function execute(
argsOrSchema,
document,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
) {
/* eslint-enable no-redeclare */
// Extract arguments from object args if provided.
return arguments.length === 1
? executeImpl(argsOrSchema)
: executeImpl({
schema: argsOrSchema,
document,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
});
}

/**
* Also implements the "Evaluating requests" section of the GraphQL specification.
* However, it guarantees to complete synchronously (or throw an error) assuming
* that all field resolvers are also synchronous.
*/
export function executeSync(args: ExecutionArgs): ExecutionResult {
const result = executeImpl(args);

// Assert that the execution was synchronous.
if (isPromise(result)) {
throw new Error('GraphQL execution failed to complete synchronously.');
}

return result;
}

function executeImpl(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
const {
schema,
document,
Expand Down Expand Up @@ -250,6 +191,22 @@ function executeImpl(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
return buildResponse(exeContext, data);
}

/**
* Also implements the "Evaluating requests" section of the GraphQL specification.
* However, it guarantees to complete synchronously (or throw an error) assuming
* that all field resolvers are also synchronous.
*/
export function executeSync(args: ExecutionArgs): ExecutionResult {
const result = execute(args);

// Assert that the execution was synchronous.
if (isPromise(result)) {
throw new Error('GraphQL execution failed to complete synchronously.');
}

return result;
}

/**
* Given a completed execution context and data, build the { errors, data }
* response defined by the "Response" section of the GraphQL specification.
Expand Down
82 changes: 5 additions & 77 deletions src/graphql.js
View file
Expand Up @@ -65,47 +65,10 @@ export type GraphQLArgs = {|
fieldResolver?: ?GraphQLFieldResolver<any, any>,
typeResolver?: ?GraphQLTypeResolver<any, any>,
|};
declare function graphql(GraphQLArgs, ..._: []): Promise<ExecutionResult>;
/* eslint-disable no-redeclare */
declare function graphql(
schema: GraphQLSchema,
source: Source | string,
rootValue?: mixed,
contextValue?: mixed,
variableValues?: ?{ +[variable: string]: mixed, ... },
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
typeResolver?: ?GraphQLTypeResolver<any, any>,
): Promise<ExecutionResult>;
export function graphql(
argsOrSchema,
source,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
) {
/* eslint-enable no-redeclare */

export function graphql(args: GraphQLArgs): Promise<ExecutionResult> {
// Always return a Promise for a consistent API.
return new Promise((resolve) =>
resolve(
// Extract arguments from object args if provided.
arguments.length === 1
? graphqlImpl(argsOrSchema)
: graphqlImpl({
schema: argsOrSchema,
source,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
}),
),
);
return new Promise((resolve) => resolve(graphqlImpl(args)));
}

/**
Expand All @@ -114,43 +77,8 @@ export function graphql(
* However, it guarantees to complete synchronously (or throw an error) assuming
* that all field resolvers are also synchronous.
*/
declare function graphqlSync(GraphQLArgs, ..._: []): ExecutionResult;
/* eslint-disable no-redeclare */
declare function graphqlSync(
schema: GraphQLSchema,
source: Source | string,
rootValue?: mixed,
contextValue?: mixed,
variableValues?: ?{ +[variable: string]: mixed, ... },
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
typeResolver?: ?GraphQLTypeResolver<any, any>,
): ExecutionResult;
export function graphqlSync(
argsOrSchema,
source,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
) {
/* eslint-enable no-redeclare */
// Extract arguments from object args if provided.
const result =
arguments.length === 1
? graphqlImpl(argsOrSchema)
: graphqlImpl({
schema: argsOrSchema,
source,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
typeResolver,
});
export function graphqlSync(args: GraphQLArgs): ExecutionResult {
const result = graphqlImpl(args);

// Assert that the execution was synchronous.
if (isPromise(result)) {
Expand Down
26 changes: 3 additions & 23 deletions src/subscription/__tests__/subscribe-test.js
View file
Expand Up @@ -152,26 +152,6 @@ async function expectPromiseToThrow(

// Check all error cases when initializing the subscription.
describe('Subscription Initialization Phase', () => {
it('accepts positional arguments', async () => {
const document = parse(`
subscription {
importantEmail
}
`);

async function* emptyAsyncIterator() {
// Empty
}

// $FlowFixMe[incompatible-call]
const ai = await subscribe(emailSchema, document, {
importantEmail: emptyAsyncIterator,
});

ai.next();
ai.return();
});

it('accepts multiple subscription fields defined in schema', async () => {
const pubsub = new SimplePubSub();
const SubscriptionTypeMultiple = new GraphQLObjectType({
Expand Down Expand Up @@ -328,7 +308,7 @@ describe('Subscription Initialization Phase', () => {
);

await expectPromiseToThrow(
// $FlowExpectedError[incompatible-call]
// $FlowExpectedError[prop-missing]
() => subscribe({ document }),
'Expected undefined to be a GraphQL schema.',
);
Expand All @@ -342,7 +322,7 @@ describe('Subscription Initialization Phase', () => {
);

await expectPromiseToThrow(
// $FlowExpectedError[incompatible-call]
// $FlowExpectedError[prop-missing]
() => subscribe({ schema: emailSchema }),
'Must provide document.',
);
Expand Down Expand Up @@ -371,7 +351,7 @@ describe('Subscription Initialization Phase', () => {
it('should pass through unexpected errors thrown in subscribe', async () => {
let expectedError;
try {
// $FlowExpectedError[incompatible-call]
// $FlowExpectedError[prop-missing]
await subscribe({ schema: emailSchema, document: {} });
} catch (error) {
expectedError = error;
Expand Down
65 changes: 12 additions & 53 deletions src/subscription/subscribe.js
View file
Expand Up @@ -57,60 +57,7 @@ export type SubscriptionArgs = {|
*
* Accepts either an object with named arguments, or individual arguments.
*/
declare function subscribe(
SubscriptionArgs,
..._: []
): Promise<AsyncGenerator<ExecutionResult, void, void> | ExecutionResult>;
/* eslint-disable no-redeclare */
declare function subscribe(
schema: GraphQLSchema,
document: DocumentNode,
rootValue?: mixed,
contextValue?: mixed,
variableValues?: ?{ +[variable: string]: mixed, ... },
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
subscribeFieldResolver?: ?GraphQLFieldResolver<any, any>,
): Promise<AsyncIterator<ExecutionResult> | ExecutionResult>;
export function subscribe(
argsOrSchema,
document,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
subscribeFieldResolver,
) {
/* eslint-enable no-redeclare */
// Extract arguments from object args if provided.
return arguments.length === 1
? subscribeImpl(argsOrSchema)
: subscribeImpl({
schema: argsOrSchema,
document,
rootValue,
contextValue,
variableValues,
operationName,
fieldResolver,
subscribeFieldResolver,
});
}

/**
* This function checks if the error is a GraphQLError. If it is, report it as
* an ExecutionResult, containing only errors and no data. Otherwise treat the
* error as a system-class error and re-throw it.
*/
function reportGraphQLError(error: mixed): ExecutionResult {
if (error instanceof GraphQLError) {
return { errors: [error] };
}
throw error;
}

function subscribeImpl(
args: SubscriptionArgs,
): Promise<AsyncGenerator<ExecutionResult, void, void> | ExecutionResult> {
const {
Expand Down Expand Up @@ -165,6 +112,18 @@ function subscribeImpl(
);
}

/**
* This function checks if the error is a GraphQLError. If it is, report it as
* an ExecutionResult, containing only errors and no data. Otherwise treat the
* error as a system-class error and re-throw it.
*/
function reportGraphQLError(error: mixed): ExecutionResult {
if (error instanceof GraphQLError) {
return { errors: [error] };
}
throw error;
}

/**
* Implements the "CreateSourceEventStream" algorithm described in the
* GraphQL specification, resolving the subscription source event stream.
Expand Down

0 comments on commit f9534f3

Please sign in to comment.