From 00c5c5863644899e98d4de14824fce0e46828c39 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 3 Aug 2021 14:16:43 -0700 Subject: [PATCH 1/3] separate getPropertyBatchLoader --- src/implementation.ts | 346 ++++++++++++++++++++++++------------------ 1 file changed, 201 insertions(+), 145 deletions(-) diff --git a/src/implementation.ts b/src/implementation.ts index fef9086..69c2f77 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -80,120 +80,8 @@ function callResource(resourceConfig: ResourceConfig, resourcePath: ReadonlyArra `; } -function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { - assert( - resourceConfig.isBatchResource === true, - `${errorPrefix(resourcePath)} Expected getBatchLoader to be called with a batch resource config`, - ); - assert( - typeof resourceConfig.batchKey === 'string' && typeof resourceConfig.newKey === 'string', - `${errorPrefix(resourcePath)} Expected both batchKey and newKey for a batch resource`, - ); - // The reference at runtime to where the underlying resource lives - const resourceReference = ['resources', ...resourcePath].join('.'); - - return `\ - new DataLoader< - ${getLoaderTypeKey(resourceConfig, resourcePath)}, - ${getLoaderTypeVal(resourceConfig, resourcePath)}, - // This third argument is the cache key type. Since we use objectHash in cacheKeyOptions, this is "string". - string, - >(${getLoaderComment(resourceConfig, resourcePath)} async (keys) => { - invariant(typeof ${resourceReference} === 'function', [ - '${errorPrefix(resourcePath)} ${resourceReference} is not a function.', - 'Did you pass in an instance of ${resourcePath.join('.')} to "getLoaders"?', - ].join(' ')); - - /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * \`\`\`js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * \`\`\` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * \`\`\`js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * \`\`\` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * \`\`\`js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * \`\`\` - * - * Returns: - * \`[ [ 0, 2 ], [ 1 ] ]\` - * - * We could also have more than one batch key. - * - * Example: - * - * \`\`\`js - * partitionItems(['bar_id', 'properties'], [ - * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, - * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, - * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, - * ]) - * \`\`\` - * - * Returns: - * \`[ [ 0, 2 ], [ 1 ] ]\` - * - * We'll refer to each element in the group as a "request ID". - */ - let requestGroups; - - if (${typeof resourceConfig.propertyBatchKey === 'string'}) { - requestGroups = partitionItems([ - '${resourceConfig.newKey}', - '${resourceConfig.propertyBatchKey}' - ], keys); - } else { - requestGroups = partitionItems('${resourceConfig.newKey}', keys); - } - +function getBatchResourceCommonLoader(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { + return ` // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all(requestGroups.map(async requestIDs => { /** @@ -206,7 +94,7 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado const requests = requestIDs.map(id => keys[id]); ${(() => { - const { batchKey, newKey, commaSeparatedBatchKey, propertyBatchKey } = resourceConfig; + const { batchKey, newKey, commaSeparatedBatchKey } = resourceConfig; let batchKeyParam = `['${batchKey}']: requests.map(k => k['${newKey}'])`; if (commaSeparatedBatchKey === true) { @@ -418,39 +306,203 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado return ''; } })()} + `; +} + +function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { + assert( + resourceConfig.isBatchResource === true, + `${errorPrefix(resourcePath)} Expected getBatchLoader to be called with a batch resource config`, + ); + assert( + typeof resourceConfig.batchKey === 'string' && typeof resourceConfig.newKey === 'string', + `${errorPrefix(resourcePath)} Expected both batchKey and newKey for a batch resource`, + ); + // The reference at runtime to where the underlying resource lives + const resourceReference = ['resources', ...resourcePath].join('.'); + + return `\ + new DataLoader< + ${getLoaderTypeKey(resourceConfig, resourcePath)}, + ${getLoaderTypeVal(resourceConfig, resourcePath)}, + // This third argument is the cache key type. Since we use objectHash in cacheKeyOptions, this is "string". + string, + >(${getLoaderComment(resourceConfig, resourcePath)} async (keys) => { + invariant(typeof ${resourceReference} === 'function', [ + '${errorPrefix(resourcePath)} ${resourceReference} is not a function.', + 'Did you pass in an instance of ${resourcePath.join('.')} to "getLoaders"?', + ].join(' ')); + + /** + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * \`\`\`js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * \`\`\` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * \`\`\`js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * \`\`\` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * \`\`\`js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * \`\`\` + * + * Returns: + * \`[ [ 0, 2 ], [ 1 ] ]\` + * + * We'll refer to each element in the group as a "request ID". + */ + const requestGroups = partitionItems('${resourceConfig.newKey}', keys); + + ${getBatchResourceCommonLoader(resourceConfig, resourcePath)} + + return response; + })) + + return unPartitionResults(requestGroups, groupedResults); + }, + { + ${ + /** + * TODO: Figure out why directly passing `cacheKeyOptions` causes + * flow errors :( + */ '' + } + ...cacheKeyOptions + } + )`; +} + +function getPropertyBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { + assert( + resourceConfig.isBatchResource === true, + `${errorPrefix(resourcePath)} Expected getBatchLoader to be called with a batch resource config`, + ); + assert( + typeof resourceConfig.batchKey === 'string' && typeof resourceConfig.newKey === 'string', + `${errorPrefix(resourcePath)} Expected both batchKey and newKey for a batch resource`, + ); + assert( + typeof resourceConfig.propertyBatchKey === 'string' && typeof resourceConfig.responseKey === 'string', + `${errorPrefix(resourcePath)} Expected propertyBatchKey and responseKey for a property batch resource`, + ); + // The reference at runtime to where the underlying resource lives + const resourceReference = ['resources', ...resourcePath].join('.'); + + return `\ + new DataLoader< + ${getLoaderTypeKey(resourceConfig, resourcePath)}, + ${getLoaderTypeVal(resourceConfig, resourcePath)}, + // This third argument is the cache key type. Since we use objectHash in cacheKeyOptions, this is "string". + string, + >(${getLoaderComment(resourceConfig, resourcePath)} async (keys) => { + invariant(typeof ${resourceReference} === 'function', [ + '${errorPrefix(resourcePath)} ${resourceReference} is not a function.', + 'Did you pass in an instance of ${resourcePath.join('.')} to "getLoaders"?', + ].join(' ')); + + /** + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * \`\`\`js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) + * \`\`\` + * + * Returns: + * \`[ [ 0, 2 ], [ 1 ] ]\` + * + * We'll refer to each element in the group as a "request ID". + */ + const requestGroups = partitionItems([ + '${resourceConfig.newKey}', + '${resourceConfig.propertyBatchKey}' + ], keys); + + ${getBatchResourceCommonLoader(resourceConfig, resourcePath)} return response; })) /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * The resource might contain less number of items that we requested. * We need the value of batchKey and propertyBatchKey in requests group to help us split the results * back up into the order that they were requested. */ - if (${typeof resourceConfig.propertyBatchKey === 'string'}) { - const batchKeyPartition = getBatchKeysForPartitionItems( - '${resourceConfig.newKey}', - ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], - keys - ); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - '${resourceConfig.propertyBatchKey}', - ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], - keys - ); - return unPartitionResultsByBatchKeyPartition( - '${resourceConfig.newKey}', - '${resourceConfig.propertyBatchKey}', - '${resourceConfig.responseKey}', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + const batchKeyPartition = getBatchKeysForPartitionItems( + '${resourceConfig.newKey}', + ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], + keys + ); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + '${resourceConfig.propertyBatchKey}', + ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], + keys + ); + return unPartitionResultsByBatchKeyPartition( + '${resourceConfig.newKey}', + '${resourceConfig.propertyBatchKey}', + '${resourceConfig.responseKey}', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults + ); }, { ${ @@ -501,9 +553,13 @@ function getNonBatchLoader(resourceConfig: NonBatchResourceConfig, resourcePath: } export default function getLoaderImplementation(resourceConfig: ResourceConfig, resourcePath: ReadonlyArray) { - const loader = resourceConfig.isBatchResource - ? getBatchLoader(resourceConfig, resourcePath) - : getNonBatchLoader(resourceConfig, resourcePath); - - return loader; + if (resourceConfig.isBatchResource) { + if (typeof resourceConfig.propertyBatchKey === 'string') { + return getPropertyBatchLoader(resourceConfig, resourcePath); + } else { + return getBatchLoader(resourceConfig, resourcePath); + } + } else { + return getNonBatchLoader(resourceConfig, resourcePath); + } } From f3649a170a72e6c540eb99e90183b495253fe038 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 3 Aug 2021 15:04:50 -0700 Subject: [PATCH 2/3] test on swapi --- examples/swapi/swapi-loaders.js | 309 +++----------------------------- 1 file changed, 25 insertions(+), 284 deletions(-) diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index 5b7204f..85e475b 100644 --- a/examples/swapi/swapi-loaders.js +++ b/examples/swapi/swapi-loaders.js @@ -337,30 +337,9 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Returns: * `[ [ 0, 2 ], [ 1 ] ]` * - * We could also have more than one batch key. - * - * Example: - * - * ```js - * partitionItems(['bar_id', 'properties'], [ - * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, - * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, - * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, - * ]) - * ``` - * - * Returns: - * `[ [ 0, 2 ], [ 1 ] ]` - * * We'll refer to each element in the group as a "request ID". */ - let requestGroups; - - if (false) { - requestGroups = partitionItems(['planet_id', 'undefined'], keys); - } else { - requestGroups = partitionItems('planet_id', keys); - } + const requestGroups = partitionItems('planet_id', keys); // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -510,35 +489,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. - * We need the value of batchKey and propertyBatchKey in requests group to help us split the results - * back up into the order that they were requested. - */ - if (false) { - const batchKeyPartition = getBatchKeysForPartitionItems( - 'planet_id', - ['planet_id', 'undefined'], - keys, - ); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - 'undefined', - ['planet_id', 'undefined'], - keys, - ); - return unPartitionResultsByBatchKeyPartition( - 'planet_id', - 'undefined', - 'undefined', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults, - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + return unPartitionResults(requestGroups, groupedResults); }, { ...cacheKeyOptions, @@ -659,30 +610,9 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Returns: * `[ [ 0, 2 ], [ 1 ] ]` * - * We could also have more than one batch key. - * - * Example: - * - * ```js - * partitionItems(['bar_id', 'properties'], [ - * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, - * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, - * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, - * ]) - * ``` - * - * Returns: - * `[ [ 0, 2 ], [ 1 ] ]` - * * We'll refer to each element in the group as a "request ID". */ - let requestGroups; - - if (false) { - requestGroups = partitionItems(['person_id', 'undefined'], keys); - } else { - requestGroups = partitionItems('person_id', keys); - } + const requestGroups = partitionItems('person_id', keys); // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -829,35 +759,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. - * We need the value of batchKey and propertyBatchKey in requests group to help us split the results - * back up into the order that they were requested. - */ - if (false) { - const batchKeyPartition = getBatchKeysForPartitionItems( - 'person_id', - ['person_id', 'undefined'], - keys, - ); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - 'undefined', - ['person_id', 'undefined'], - keys, - ); - return unPartitionResultsByBatchKeyPartition( - 'person_id', - 'undefined', - 'undefined', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults, - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + return unPartitionResults(requestGroups, groupedResults); }, { ...cacheKeyOptions, @@ -978,30 +880,9 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Returns: * `[ [ 0, 2 ], [ 1 ] ]` * - * We could also have more than one batch key. - * - * Example: - * - * ```js - * partitionItems(['bar_id', 'properties'], [ - * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, - * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, - * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, - * ]) - * ``` - * - * Returns: - * `[ [ 0, 2 ], [ 1 ] ]` - * * We'll refer to each element in the group as a "request ID". */ - let requestGroups; - - if (false) { - requestGroups = partitionItems(['vehicle_id', 'undefined'], keys); - } else { - requestGroups = partitionItems('vehicle_id', keys); - } + const requestGroups = partitionItems('vehicle_id', keys); // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1151,35 +1032,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. - * We need the value of batchKey and propertyBatchKey in requests group to help us split the results - * back up into the order that they were requested. - */ - if (false) { - const batchKeyPartition = getBatchKeysForPartitionItems( - 'vehicle_id', - ['vehicle_id', 'undefined'], - keys, - ); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - 'undefined', - ['vehicle_id', 'undefined'], - keys, - ); - return unPartitionResultsByBatchKeyPartition( - 'vehicle_id', - 'undefined', - 'undefined', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults, - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + return unPartitionResults(requestGroups, groupedResults); }, { ...cacheKeyOptions, @@ -1309,30 +1162,9 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Returns: * `[ [ 0, 2 ], [ 1 ] ]` * - * We could also have more than one batch key. - * - * Example: - * - * ```js - * partitionItems(['bar_id', 'properties'], [ - * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, - * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, - * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, - * ]) - * ``` - * - * Returns: - * `[ [ 0, 2 ], [ 1 ] ]` - * * We'll refer to each element in the group as a "request ID". */ - let requestGroups; - - if (false) { - requestGroups = partitionItems(['film_id', 'undefined'], keys); - } else { - requestGroups = partitionItems('film_id', keys); - } + const requestGroups = partitionItems('film_id', keys); // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1477,31 +1309,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. - * We need the value of batchKey and propertyBatchKey in requests group to help us split the results - * back up into the order that they were requested. - */ - if (false) { - const batchKeyPartition = getBatchKeysForPartitionItems('film_id', ['film_id', 'undefined'], keys); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - 'undefined', - ['film_id', 'undefined'], - keys, - ); - return unPartitionResultsByBatchKeyPartition( - 'film_id', - 'undefined', - 'undefined', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults, - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + return unPartitionResults(requestGroups, groupedResults); }, { ...cacheKeyOptions, @@ -1567,47 +1375,6 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * * When we have calls to .load with differing non batch key args, * we'll need to send multiple requests to the underlying * resource to make sure we get the right results back. @@ -1618,21 +1385,6 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - * Returns: - * `[ [ 0, 2 ], [ 1 ] ]` - * - * We could also have more than one batch key. - * - * Example: - * - * ```js * partitionItems(['bar_id', 'properties'], [ * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, @@ -1645,13 +1397,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - let requestGroups; - - if (true) { - requestGroups = partitionItems(['film_id', 'properties'], keys); - } else { - requestGroups = partitionItems('film_id', keys); - } + const requestGroups = partitionItems(['film_id', 'properties'], keys); // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1810,30 +1556,25 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * The resource might contain less number of items that we requested. * We need the value of batchKey and propertyBatchKey in requests group to help us split the results * back up into the order that they were requested. */ - if (true) { - const batchKeyPartition = getBatchKeysForPartitionItems('film_id', ['film_id', 'properties'], keys); - const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( - 'properties', - ['film_id', 'properties'], - keys, - ); - return unPartitionResultsByBatchKeyPartition( - 'film_id', - 'properties', - 'id', - batchKeyPartition, - propertyBatchKeyPartiion, - requestGroups, - groupedResults, - ); - } else { - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); - } + const batchKeyPartition = getBatchKeysForPartitionItems('film_id', ['film_id', 'properties'], keys); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'properties', + ['film_id', 'properties'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'film_id', + 'properties', + 'id', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); }, { ...cacheKeyOptions, From 59a9dd88761cdf8955da12639098f750e01a5c0d Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 3 Aug 2021 16:52:19 -0700 Subject: [PATCH 3/3] change funciton name to batchLoaderLogic --- src/implementation.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/implementation.ts b/src/implementation.ts index 69c2f77..43287ef 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -80,7 +80,10 @@ function callResource(resourceConfig: ResourceConfig, resourcePath: ReadonlyArra `; } -function getBatchResourceCommonLoader(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { +/** + * This is a helper to implement the batch logic, shared between getBatchLoader and getPropertyBatchLoader + */ +function batchLoaderLogic(resourceConfig: BatchResourceConfig, resourcePath: ReadonlyArray) { return ` // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all(requestGroups.map(async requestIDs => { @@ -399,7 +402,7 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado */ const requestGroups = partitionItems('${resourceConfig.newKey}', keys); - ${getBatchResourceCommonLoader(resourceConfig, resourcePath)} + ${batchLoaderLogic(resourceConfig, resourcePath)} return response; })) @@ -474,7 +477,7 @@ function getPropertyBatchLoader(resourceConfig: BatchResourceConfig, resourcePat '${resourceConfig.propertyBatchKey}' ], keys); - ${getBatchResourceCommonLoader(resourceConfig, resourcePath)} + ${batchLoaderLogic(resourceConfig, resourcePath)} return response; }))