Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Commit

Permalink
fix(stitching): do not make sync delegation async
Browse files Browse the repository at this point in the history
delegateToSchema now returns a synchronous result when possible rather
than always returning a promise.
merging types is also performed synchronously when possible.

Fixes #36.
  • Loading branch information
yaacovCR committed Jan 16, 2020
1 parent d859ac1 commit dcfea0c
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 30 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"apollo-upload-client": "^12.1.0",
"body-parser": "^1.19.0",
"chai": "^4.2.0",
"dataloader": "^2.0.0",
"dateformat": "^3.0.3",
"express": "^4.17.1",
"express-graphql": "^0.9.0",
Expand Down
27 changes: 20 additions & 7 deletions src/stitching/checkResultAndHandleErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ function handleListMember(
}
}

async function mergeFields(
function mergeFields(
type: GraphQLCompositeType,
object: any,
subschemas: Array<GraphQLSchema | SubschemaConfig>,
context: Record<string, any>,
info: IGraphQLToolsResolveInfo,
): Promise<any> {
): any {
let typeName: string;
if (isAbstractType(type)) {
typeName = info.schema.getTypeMap()[resolveFromParentTypename(object)].name;
Expand All @@ -174,17 +174,30 @@ async function mergeFields(
subschema => !subschemas.includes(subschema)
);
if (remainingSubschemas.length) {
const results = await Promise.all(remainingSubschemas.map(subschema => {
const resolver = subschema.mergedTypeConfigs[typeName].mergedTypeResolver;
return resolver(subschema, object, context, {
const maybePromises = remainingSubschemas.map(subschema => {
return subschema.mergedTypeConfigs[typeName].mergedTypeResolver(subschema, object, context, {
...info,
mergeInfo: {
...info.mergeInfo,
mergedTypes: {},
},
});
}));
object = results.reduce((acc: any, r: ExecutionResult) => mergeDeep(acc, r), object);
});

let containsPromises = false; {
for (const maybePromise of maybePromises) {
if (maybePromise instanceof Promise) {
containsPromises = true;
break;
}
}
}
if (containsPromises) {
return Promise.all(maybePromises).
then(results => results.reduce((acc: any, r: ExecutionResult) => mergeDeep(acc, r), object));
} else {
return maybePromises.reduce((acc: any, r: ExecutionResult) => mergeDeep(acc, r), object);
}
}
}

Expand Down
54 changes: 31 additions & 23 deletions src/stitching/delegateToSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ import linkToFetcher from './linkToFetcher';
import { observableToAsyncIterable } from './observableToAsyncIterable';
import { AddMergedTypeFragments } from '../transforms';

import { isAsyncIterable } from 'iterall';

export default function delegateToSchema(
options: IDelegateToSchemaOptions | GraphQLSchema,
...args: any[]
): Promise<any> {
): any {
if (options instanceof GraphQLSchema) {
throw new Error(
'Passing positional arguments to delegateToSchema is a deprecated. ' +
Expand All @@ -57,7 +59,7 @@ export default function delegateToSchema(
return delegateToSchemaImplementation(options);
}

async function delegateToSchemaImplementation({
function delegateToSchemaImplementation({
schema: subschema,
rootValue,
info,
Expand All @@ -68,7 +70,7 @@ async function delegateToSchemaImplementation({
transforms = [],
skipValidation,
}: IDelegateToSchemaOptions,
): Promise<any> {
): any {
let targetSchema: GraphQLSchema;
let subschemaConfig: SubschemaConfig;

Expand Down Expand Up @@ -138,33 +140,39 @@ async function delegateToSchemaImplementation({
if (operation === 'query' || operation === 'mutation') {
const executor = createExecutor(targetSchema, rootValue, subschemaConfig);

return applyResultTransforms(
await executor({
document: processedRequest.document,
context,
variables: processedRequest.variables
}),
transforms,
);
const executionResult: ExecutionResult | Promise<ExecutionResult> = executor({
document: processedRequest.document,
context,
variables: processedRequest.variables
});

if (executionResult instanceof Promise) {
return executionResult.then((originalResult: any) => applyResultTransforms(originalResult, transforms));
} else {
return applyResultTransforms(executionResult, transforms);
}
} else if (operation === 'subscription') {
const subscriber = createSubscriber(targetSchema, rootValue, subschemaConfig);

const originalAsyncIterator = (await subscriber({
return subscriber({
document: processedRequest.document,
context,
variables: processedRequest.variables,
})) as AsyncIterator<ExecutionResult>;

// "subscribe" to the subscription result and map the result through the transforms
return mapAsyncIterator<ExecutionResult, any>(originalAsyncIterator, result => {
const transformedResult = applyResultTransforms(result, transforms);

// wrap with fieldName to return for an additional round of resolutioon
// with payload as rootValue
return {
[info.fieldName]: transformedResult,
};
}).then((subscriptionResult: AsyncIterableIterator<ExecutionResult> | ExecutionResult) => {
if (isAsyncIterable(subscriptionResult)) {
// "subscribe" to the subscription result and map the result through the transforms
return mapAsyncIterator<ExecutionResult, any>(subscriptionResult, result => {
const transformedResult = applyResultTransforms(result, transforms);

// wrap with fieldName to return for an additional round of resolutioon
// with payload as rootValue
return {
[info.fieldName]: transformedResult,
};
});
} else {
return applyResultTransforms(subscriptionResult, transforms);
}
});
}
}
Expand Down
125 changes: 125 additions & 0 deletions src/test/testDataloader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/* tslint:disable:no-unused-expression */

import { expect } from 'chai';

import { makeExecutableSchema } from '../makeExecutableSchema';
import {
mergeSchemas,
delegateToSchema
} from '../stitching';
import { graphql, GraphQLList } from 'graphql';
import DataLoader from 'dataloader';
import { IGraphQLToolsResolveInfo } from '../Interfaces';

describe('dataloader', () => {
it('should work', async () => {
const taskSchema = makeExecutableSchema({
typeDefs: `
type Task {
id: ID!
text: String
userId: ID!
}
type Query {
task(id: ID!): Task
}
`,
resolvers: {
Query: {
task: (root, { id }) => ({ id, text: `task ${id}`, userId: id }),
}
},
});

const userSchema = makeExecutableSchema({
typeDefs: `
type User {
id: ID!
email: String!
}
type Query {
usersByIds(ids: [ID!]!): [User]!
}
`,
resolvers: {
Query: {
usersByIds: (root, { ids }) => {
return ids.map((id: string) => ({ id, email: `${id}@tasks.com` }));
},
}
},
});

const schema = mergeSchemas({
schemas: [
taskSchema,
userSchema
],
typeDefs: `
extend type Task {
user: User!
}
`,
resolvers: {
Task: {
user: {
fragment: `... on Task { userId }`,
resolve(task, args, context, info) {
return context.usersLoader.load({ id: task.userId, info });
}
}
},
}
});

const usersLoader = new DataLoader(async (keys: Array<{ id: any, info: IGraphQLToolsResolveInfo }>) => {
const users = delegateToSchema({
schema: userSchema,
operation: 'query',
fieldName: 'usersByIds',
args: {
ids: keys.map((k: { id: any }) => k.id)
},
context: null,
info: {
...keys[0].info,
returnType: new GraphQLList(keys[0].info.returnType),
}
});

expect(users).to.deep.equal([{
id: '1',
email: '1@tasks.com',
}]);

return users;
});

const query = `{
task(id: "1") {
id
text
user {
id
email
}
}
}`;

const result = await graphql(schema, query, null, { usersLoader });

expect(result).to.deep.equal({
data:
{
task: {
id: '1',
text: 'task 1',
user: {
id: '1',
email: '1@tasks.com',
}
}
}
});
});
});
1 change: 1 addition & 0 deletions src/test/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ require('source-map-support').install();

import './testAlternateMergeSchemas';
import './testDelegateToSchema';
import './testDataloader';
import './testDirectives';
import './testErrors';
import './testFragmentsAreNotDuplicated';
Expand Down

0 comments on commit dcfea0c

Please sign in to comment.