Skip to content

Commit

Permalink
test
Browse files Browse the repository at this point in the history
these changes purposely introduce unnecessary promise resolution into
key calculation.

the goal is to make sure that subfields of fields from one subschema
are not held up by another subschema. the previous lazy-query-planner
approach was to return a promise for each field that resolved to a
merged parent from the correct stage of the delegation plan.

A different approach would be to just request from the merged parent
the required fields, and return a promise that will resolve to the
value whenever it is set.

the goal in this experiment is to see if all the  promise creation
required has the same performance penalties of the lazy-query planner
approach

in some examples, serial awaits are used rather than Promise.all. In
theory, a method should be created to easily rquest multiple keys
from the store manually.
  • Loading branch information
yaacovCR committed Sep 27, 2021
1 parent b151843 commit 90f016b
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 69 deletions.
10 changes: 5 additions & 5 deletions packages/batch-delegate/tests/typeMerging.example.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ describe('merging using type merging', () => {
User: {
selectionSet: '{ id }',
fieldName: '_userById',
args: ({ id }) => ({ id })
args: async ({ id }) => ({ id: await id })
}
},
batch: true,
Expand All @@ -242,7 +242,7 @@ describe('merging using type merging', () => {
},
},
fieldName: '_products',
key: ({ upc, weight, price }) => ({ upc, weight, price }),
key: async ({ upc, weight, price }) => ({ upc: await upc, weight: await weight, price: await price }),
argsFromKeys: (representations) => ({ representations }),
}
},
Expand All @@ -254,7 +254,7 @@ describe('merging using type merging', () => {
Product: {
selectionSet: '{ upc }',
fieldName: '_productByUpc',
args: ({ upc }) => ({ upc }),
args: async ({ upc }) => ({ upc: await upc }),
}
},
batch: true,
Expand All @@ -266,12 +266,12 @@ describe('merging using type merging', () => {
selectionSet: '{ id }',
fieldName: '_usersById',
argsFromKeys: (ids) => ({ ids }),
key: ({ id }) => id,
key: async ({ id }) => id,
},
Product: {
selectionSet: '{ upc }',
fieldName: '_productByUpc',
args: ({ upc }) => ({ upc }),
args: async ({ upc }) => ({ upc: await upc }),
},
},
batch: true,
Expand Down
4 changes: 2 additions & 2 deletions packages/delegate/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,13 @@ export interface MergedTypeConfig<K = any, V = any, TContext = Record<string, an
export interface MergedTypeEntryPoint<K = any, V = any, TContext = Record<string, any>>
extends MergedTypeResolverOptions<K, V> {
selectionSet?: string;
key?: (originalResult: any) => K;
key?: (originalResult: any) => Promise<K>;
resolve?: MergedTypeResolver<TContext>;
}

export interface MergedTypeResolverOptions<K = any, V = any> {
fieldName?: string;
args?: (originalResult: any) => Record<string, any>;
args?: (originalResult: any) => Promise<Record<string, any>>;
argsFromKeys?: (keys: ReadonlyArray<K>) => Record<string, any>;
valuesFromResults?: (results: any, keys: ReadonlyArray<K>) => Array<V>;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/stitch/src/createMergedTypeResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ export function createMergedTypeResolver<TContext = any>(
}

if (args != null) {
return function mergedTypeResolver(originalResult, context, info, subschema, selectionSet) {
return async function mergedTypeResolver(originalResult, context, info, subschema, selectionSet) {
return delegateToSchema({
schema: subschema,
operation: 'query',
fieldName,
returnType: getNamedType(
info.schema.getType(originalResult.__typename) ?? info.returnType
) as GraphQLOutputType,
args: args(originalResult),
args: await args(originalResult),
selectionSet,
context,
info,
Expand Down
4 changes: 2 additions & 2 deletions packages/stitch/src/stitchingInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ function createMergedTypes<TContext = Record<string, any>>(
resolvers.set(
subschema,
keyFn
? (originalResult, context, info, subschema, selectionSet) => {
const key = keyFn(originalResult);
? async (originalResult, context, info, subschema, selectionSet) => {
const key = await keyFn(originalResult);
return resolver(originalResult, context, info, subschema, selectionSet, key);
}
: resolver
Expand Down
24 changes: 13 additions & 11 deletions packages/stitching-directives/src/properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,26 @@ export function getProperty(object: Record<string, any>, path: Array<string>): a
return getProperty(prop, newPath);
}

export function getProperties(object: Record<string, any>, propertyTree: PropertyTree): any {
// c.f. https://github.com/graphql/graphql-js/blob/main/src/jsutils/promiseForObject.ts
export async function getPropertiesAsync(object: Record<string, any>, propertyTree: PropertyTree): Promise<any> {
if (object == null) {
return object;
}

const newObject = Object.create(null);
const keys = Object.keys(propertyTree);

for (const key in propertyTree) {
const subKey = propertyTree[key];
const values = await Promise.all(keys.map(key => object[key]));

const newObject = Object.create(null);
for (const [i, key] of keys.entries()) {
const subKey = propertyTree[key];
if (subKey == null) {
newObject[key] = object[key];
newObject[key] = values[i];
continue;
}

const prop = object[key];

newObject[key] = deepMap(prop, function deepMapFn(item) {
return getProperties(item, subKey);
newObject[key] = await deepMapAsync(values[i], function deepMapFn(item) {
return getPropertiesAsync(item, subKey);
});
}

Expand All @@ -70,9 +71,10 @@ export function propertyTreeFromPaths(paths: Array<Array<string>>): PropertyTree
return propertyTree;
}

function deepMap(arrayOrItem: any, fn: (item: any) => any): any {
async function deepMapAsync(arrayOrItem: any, fn: (item: any) => Promise<any>): Promise<any> {
if (Array.isArray(arrayOrItem)) {
return arrayOrItem.map(nestedArrayOrItem => deepMap(nestedArrayOrItem, fn));
const arr = await Promise.all(arrayOrItem);
return Promise.all(arr.map(nestedArrayOrItem => deepMapAsync(nestedArrayOrItem, fn)));
}

return fn(arrayOrItem);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { MergedTypeResolverInfo, StitchingDirectivesOptions } from './types';

import { defaultStitchingDirectiveOptions } from './defaultStitchingDirectiveOptions';
import { parseMergeArgsExpr } from './parseMergeArgsExpr';
import { addProperty, getProperty, getProperties } from './properties';
import { addProperty, getProperty, getPropertiesAsync } from './properties';
import { stitchingDirectivesValidator } from './stitchingDirectivesValidator';

export function stitchingDirectivesTransformer(
Expand Down Expand Up @@ -464,9 +464,9 @@ function forEachConcreteType(
}
}

function generateKeyFn(mergedTypeResolverInfo: MergedTypeResolverInfo): (originalResult: any) => any {
return function keyFn(originalResult: any) {
return getProperties(originalResult, mergedTypeResolverInfo.usedProperties);
function generateKeyFn(mergedTypeResolverInfo: MergedTypeResolverInfo): (originalResult: any) => Promise<any> {
return function keyFn(originalResult: any): Promise<any> {
return getPropertiesAsync(originalResult, mergedTypeResolverInfo.usedProperties);
};
}

Expand Down Expand Up @@ -498,12 +498,14 @@ function generateArgsFromKeysFn(
};
}

function generateArgsFn(mergedTypeResolverInfo: MergedTypeResolverInfo): (originalResult: any) => Record<string, any> {
function generateArgsFn(
mergedTypeResolverInfo: MergedTypeResolverInfo
): (originalResult: any) => Promise<Record<string, any>> {
const { mappingInstructions, args, usedProperties } = mergedTypeResolverInfo;

return function generateArgs(originalResult: any): Record<string, any> {
return async function generateArgs(originalResult: any): Promise<Record<string, any>> {
const newArgs = mergeDeep([{}, args]);
const filteredResult = getProperties(originalResult, usedProperties);
const filteredResult = await getPropertiesAsync(originalResult, usedProperties);
if (mappingInstructions) {
for (const mappingInstruction of mappingInstructions) {
const { destinationPath, sourcePath } = mappingInstruction;
Expand Down
16 changes: 8 additions & 8 deletions packages/stitching-directives/tests/properties.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addProperty, getProperties } from "../src/properties";
import { addProperty, getPropertiesAsync } from "../src/properties";

describe('addProperty', () => {
test('can add a key to an object', () => {
Expand Down Expand Up @@ -29,16 +29,16 @@ describe('addProperty', () => {
});

describe('getProperties', () => {
test('can getProperties', () => {
test('can getProperties', async () => {
const object = {
field1: 'value1',
field2: {
subfieldA: 'valueA',
subfieldB: 'valueB',
},
field1: Promise.resolve('value1'),
field2: Promise.resolve({
subfieldA: Promise.resolve('valueA'),
subfieldB: Promise.resolve('valueB'),
}),
}

const extracted = getProperties(object, {
const extracted = await getPropertiesAsync(object, {
field1: null,
field2: {
subfieldA: null,
Expand Down
Loading

0 comments on commit 90f016b

Please sign in to comment.