Merge pull request #85 from teamstarter/develop

vincentdesmares · web-flow · commit ed0e36c55c62 · 2024-03-08T13:38:01.000+01:00
feat(dataloader): Optimize all list calls by removing useless where and include properties to allow batching. Small clean on internal types.
diff --git a/lib/createListResolver.js b/lib/createListResolver.js
@@ -75,7 +75,7 @@ function allowOrderOnAssociations(findOptions, model) {
         // By default we take the direction detected by GraphQL-sequelize
         // It will be 'ASC' if 'reverse:' was not specified.
         // But this will only work for the first field.
-        var direction = index === 0 ? findOptions.order[0][1] : 'ASC';
+        var direction = index === 0 && findOptions.order ? findOptions.order[0][1] : 'ASC';
         // When reverse is not already removed by graphql-sequelize
         // we try to detect it ourselves. Happens for multiple fields sort.
         if (singleOrder.search('reverse:') === 0) {
@@ -92,7 +92,8 @@ function allowOrderOnAssociations(findOptions, model) {
             if (typeof model.associations[associationName] === 'undefined') {
                 throw new Error("Association ".concat(associationName, " unknown on model ").concat(model.name, " order"));
             }
-            if (typeof findOptions.include === 'undefined') {
+            if (typeof findOptions.include === 'undefined' ||
+                typeof findOptions.include === 'string') {
                 findOptions.include = [];
             }
             var modelInclude = {
@@ -101,7 +102,10 @@ function allowOrderOnAssociations(findOptions, model) {
             if (model.associations[associationName].as) {
                 modelInclude.as = model.associations[associationName].as;
             }
-            findOptions.include.push(modelInclude);
+            // Type assertion to specify the type of findOptions.include as an array
+            if ('push' in findOptions.include) {
+                findOptions.include.push(modelInclude);
+            }
             var modelSort = {
                 model: model.associations[associationName].target
             };
@@ -139,17 +143,19 @@ function allowOrderOnAssociations(findOptions, model) {
      * to
      * order = [['id', 'ASC'], ['fullname', 'DESC']
      */
-    findOptions.order.map(function (order) {
-        // Handle multiple sort fields.
-        if (order[0].search(',') === -1) {
-            checkForAssociationSort(order[0], 0);
-            return;
-        }
-        var multipleOrder = order[0].split(',');
-        for (var index in multipleOrder) {
-            checkForAssociationSort(multipleOrder[index], parseInt(index));
-        }
-    });
+    if ('map' in findOptions.order) {
+        findOptions.order.map(function (order) {
+            // Handle multiple sort fields.
+            if (order[0].search(',') === -1) {
+                checkForAssociationSort(order[0], 0);
+                return;
+            }
+            var multipleOrder = order[0].split(',');
+            for (var index in multipleOrder) {
+                checkForAssociationSort(multipleOrder[index], parseInt(index));
+            }
+        });
+    }
     findOptions.order = processedOrder;
     return findOptions;
 }
@@ -175,6 +181,22 @@ function trimAndOptimizeFindOptions(_a) {
                         graphqlTypeDeclaration.list.removeUnusedAttributes === false
                         ? findOptions
                         : (0, removeUnusedAttributes_1["default"])(findOptions, info, graphqlTypeDeclaration.model, models);
+                    // As sequelize-dataloader does not support the include option, we have to remove it.
+                    // It does not differenciate between an empty include and an include with models so we have to remove it.
+                    if (trimedFindOptions.include &&
+                        typeof trimedFindOptions.include === 'object' &&
+                        'length' in trimedFindOptions.include &&
+                        trimedFindOptions.include.length === 0) {
+                        delete trimedFindOptions.include;
+                    }
+                    // As sequelize-dataloader does not support the where option, we have to remove it.
+                    // It does not differenciate between an empty where and a where with properties so we have to remove it.
+                    if (trimedFindOptions.where &&
+                        // Symbols like [Op.and] are not returned by Object.keys and must be handled separately.
+                        Object.getOwnPropertySymbols(trimedFindOptions.where).length === 0 &&
+                        Object.keys(trimedFindOptions.where).length === 0) {
+                        delete trimedFindOptions.where;
+                    }
                     if (!
                     // If we have a list with a limit and an offset
                     (trimedFindOptions.limit &&
diff --git a/src/createListResolver.ts b/src/createListResolver.ts
@@ -1,9 +1,9 @@
 import { resolver } from 'graphql-sequelize'
-import { Model } from 'sequelize'
+import { FindOptions, Model } from 'sequelize'
 import removeUnusedAttributes from './removeUnusedAttributes'
 import { GlobalBeforeHook, ModelDeclarationType } from './types/types'
 
-function allowOrderOnAssociations(findOptions: any, model: any) {
+function allowOrderOnAssociations(findOptions: FindOptions<any>, model: any) {
   if (typeof findOptions.order === 'undefined') {
     return findOptions
   }
@@ -16,7 +16,8 @@ function allowOrderOnAssociations(findOptions: any, model: any) {
     // By default we take the direction detected by GraphQL-sequelize
     // It will be 'ASC' if 'reverse:' was not specified.
     // But this will only work for the first field.
-    let direction = index === 0 ? findOptions.order[0][1] : 'ASC'
+    let direction =
+      index === 0 && findOptions.order ? findOptions.order[0][1] : 'ASC'
     // When reverse is not already removed by graphql-sequelize
     // we try to detect it ourselves. Happens for multiple fields sort.
     if (singleOrder.search('reverse:') === 0) {
@@ -35,7 +36,10 @@ function allowOrderOnAssociations(findOptions: any, model: any) {
           `Association ${associationName} unknown on model ${model.name} order`
         )
       }
-      if (typeof findOptions.include === 'undefined') {
+      if (
+        typeof findOptions.include === 'undefined' ||
+        typeof findOptions.include === 'string'
+      ) {
         findOptions.include = []
       }
 
@@ -47,7 +51,10 @@ function allowOrderOnAssociations(findOptions: any, model: any) {
         modelInclude.as = model.associations[associationName].as
       }
 
-      findOptions.include.push(modelInclude)
+      // Type assertion to specify the type of findOptions.include as an array
+      if ('push' in findOptions.include) {
+        findOptions.include.push(modelInclude)
+      }
 
       const modelSort: any = {
         model: model.associations[associationName].target,
@@ -89,23 +96,25 @@ function allowOrderOnAssociations(findOptions: any, model: any) {
    * to
    * order = [['id', 'ASC'], ['fullname', 'DESC']
    */
-  findOptions.order.map((order: any) => {
-    // Handle multiple sort fields.
-    if (order[0].search(',') === -1) {
-      checkForAssociationSort(order[0], 0)
-      return
-    }
-    const multipleOrder = order[0].split(',')
-    for (const index in multipleOrder) {
-      checkForAssociationSort(multipleOrder[index], parseInt(index))
-    }
-  })
+  if ('map' in findOptions.order) {
+    findOptions.order.map((order: any) => {
+      // Handle multiple sort fields.
+      if (order[0].search(',') === -1) {
+        checkForAssociationSort(order[0], 0)
+        return
+      }
+      const multipleOrder = order[0].split(',')
+      for (const index in multipleOrder) {
+        checkForAssociationSort(multipleOrder[index], parseInt(index))
+      }
+    })
+  }
   findOptions.order = processedOrder
   return findOptions
 }
 
 const argsAdvancedProcessing = (
-  findOptions: any,
+  findOptions: FindOptions<any>,
   args: any,
   context: any,
   info: any,
@@ -135,7 +144,7 @@ async function trimAndOptimizeFindOptions({
   info,
   models,
 }: {
-  findOptions: any
+  findOptions: FindOptions<any>
   graphqlTypeDeclaration: any
   info: any
   models: any
@@ -151,6 +160,28 @@ async function trimAndOptimizeFindOptions({
           models
         )
 
+  // As sequelize-dataloader does not support the include option, we have to remove it.
+  // It does not differenciate between an empty include and an include with models so we have to remove it.
+  if (
+    trimedFindOptions.include &&
+    typeof trimedFindOptions.include === 'object' &&
+    'length' in trimedFindOptions.include &&
+    trimedFindOptions.include.length === 0
+  ) {
+    delete trimedFindOptions.include
+  }
+
+  // As sequelize-dataloader does not support the where option, we have to remove it.
+  // It does not differenciate between an empty where and a where with properties so we have to remove it.
+  if (
+    trimedFindOptions.where &&
+    // Symbols like [Op.and] are not returned by Object.keys and must be handled separately.
+    Object.getOwnPropertySymbols(trimedFindOptions.where).length === 0 &&
+    Object.keys(trimedFindOptions.where).length === 0
+  ) {
+    delete trimedFindOptions.where
+  }
+
   if (
     // If we have a list with a limit and an offset
     trimedFindOptions.limit &&
@@ -230,7 +261,7 @@ export default function createListResolver(
       ? graphqlTypeDeclaration.list.contextToOptions
       : undefined,
     before: async (findOptions: any, args: any, context: any, info: any) => {
-      let processedFindOptions = argsAdvancedProcessing(
+      let processedFindOptions: FindOptions<any> = argsAdvancedProcessing(
         findOptions,
         args,
         context,
diff --git a/src/types/types.d.ts b/src/types/types.d.ts
@@ -10,21 +10,12 @@ import {
 } from 'graphql'
 import {
   Association,
-  Attributes,
-  FindOptions as FO,
-  Includeable,
+  FindOptions,
   Model,
   ModelStatic,
   Sequelize,
 } from 'sequelize/types'
 
-// graphql-sequelize does not have typescript support. So we have to reproduce what it is based on
-// the sequelize implementation.
-export type FindOptions<M extends Model> = Omit<
-  FO<Attributes<M>>,
-  'include'
-> & { include: Includeable[] }
-
 export type Action = 'list' | 'create' | 'delete' | 'update' | 'count'
 export type ActionList = Array<Action>
 
