Repository: mercurius-js/mercurius
Pull request #671 (2021-11-29 11:32:02)

Include the info object in loaders when the loader cache is off
Signed-off-by: Matteo Collina <hello@matteocollina.com>


Patch:
diff --git a/docs/loaders.md b/docs/loaders.md
index 6834196..d84f189 100644
--- a/docs/loaders.md
+++ b/docs/loaders.md
@@ -21,8 +21,8 @@ Example:
 ```js
 const loaders = {
   Dog: {
-    async owner(queries, { reply }) {
-      return queries.map(({ obj }) => owners[obj.name])
+    async owner (queries, { reply }) {
+      return queries.map(({ obj, params }) => owners[obj.name])
     }
   }
 }
@@ -40,8 +40,11 @@ It is also possible disable caching with:
 const loaders = {
   Dog: {
     owner: {
-      async loader(queries, { reply }) {
-        return queries.map(({ obj }) => owners[obj.name])
+      async loader (queries, { reply }) {
+        return queries.map(({ obj, params, info }) => { 
+          // info is available only if the loader is not cached
+          owners[obj.name]
+        })
       },
       opts: {
         cache: false
@@ -57,6 +60,28 @@ app.register(mercurius, {
 })
 ```
 
+Alternatively, globally disabling caching also disable the Loader cache:
+
+```js
+const loaders = {
+  Dog: {
+    async owner (queries, { reply }) {
+      return queries.map(({ obj, params, info }) => { 
+        // info is available only if the loader is not cached
+        owners[obj.name]
+      })
+    }
+  }
+}
+
+app.register(mercurius, {
+  schema,
+  resolvers,
+  loaders,
+  cache: false
+})
+```
+
 Disabling caching has the advantage to avoid the serialization at
 the cost of more objects to fetch in the resolvers.
 
diff --git a/index.js b/index.js
index 17b3db3..74f3f0e 100644
--- a/index.js
+++ b/index.js
@@ -28,6 +28,7 @@ const buildGateway = require('./lib/gateway')
 const mq = require('mqemitter')
 const { PubSub, withFilter } = require('./lib/subscriber')
 const persistedQueryDefaults = require('./lib/persistedQueryDefaults')
+const stringify = require('safe-stable-stringify')
 const {
   ErrorWithProps,
   defaultErrorFormatter,
@@ -366,9 +367,10 @@ const plugin = fp(async function (app, opts) {
       app.decorate(kSubscriptionFactory, subscriptionFactory)
     }
 
-    function defineLoader (name) {
+    function defineLoader (name, opts) {
       // async needed because of throw
-      return async function (obj, params, { reply }, info) {
+      return async function (obj, params, ctx, info) {
+        const { reply } = ctx
         if (!reply) {
           throw new MER_ERR_INVALID_OPTS('loaders only work via reply.graphql()')
         }
@@ -379,19 +381,33 @@ const plugin = fp(async function (app, opts) {
       }
     }
 
+    function serialize (query) {
+      if (query.info) {
+        return stringify({ obj: query.obj, params: query.params })
+      }
+      return query
+    }
+
     const resolvers = {}
     for (const typeKey of Object.keys(loaders)) {
       const type = loaders[typeKey]
       resolvers[typeKey] = {}
       for (const prop of Object.keys(type)) {
         const name = typeKey + '-' + prop
-        resolvers[typeKey][prop] = defineLoader(name)
+        const toAssign = [{}, type[prop].opts || {}]
+        if (opts.cache === false) {
+          toAssign.push({
+            cache: false
+          })
+        }
+        const factoryOpts = Object.assign(...toAssign)
+        resolvers[typeKey][prop] = defineLoader(name, factoryOpts)
         if (typeof type[prop] === 'function') {
-          factory.add(name, type[prop])
-          subscriptionFactory.add(name, { cache: false }, type[prop])
+          factory.add(name, factoryOpts, type[prop], serialize)
+          subscriptionFactory.add(name, { cache: false }, type[prop], serialize)
         } else {
-          factory.add(name, type[prop].opts, type[prop].loader)
-          subscriptionFactory.add(name, Object.assign({}, type[prop].opts, { cache: false }), type[prop].loader)
+          factory.add(name, factoryOpts, type[prop].loader, serialize)
+          subscriptionFactory.add(name, Object.assign({}, type[prop].opts, { cache: false }), type[prop].loader, serialize)
         }
       }
     }


Test patch:
diff --git a/test/loaders.js b/test/loaders.js
index 28cd24f..530ac11 100644
--- a/test/loaders.js
+++ b/test/loaders.js
@@ -14,7 +14,7 @@ const dogs = [{
   name: 'Buddy'
 }, {
   name: 'Max'
-}]
+}].map(Object.freeze)
 
 const owners = {
   Max: {
@@ -87,7 +87,9 @@ test('loaders create batching resolvers', async (t) => {
           },
           params: {}
         }])
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -142,7 +144,10 @@ test('disable cache for each loader', async (t) => {
       owner: {
         async loader (queries, { reply }) {
           // note that the second entry for max is NOT cached
-          t.same(queries, [{
+          const found = queries.map((q) => {
+            return { obj: q.obj, params: q.params }
+          })
+          t.same(found, [{
             obj: {
               name: 'Max'
             },
@@ -163,7 +168,9 @@ test('disable cache for each loader', async (t) => {
             },
             params: {}
           }])
-          return queries.map(({ obj }) => owners[obj.name])
+          return queries.map(({ obj }) => {
+            return { ...owners[obj.name] }
+          })
         },
         opts: {
           cache: false
@@ -220,7 +227,9 @@ test('defineLoaders method, if factory exists', async (t) => {
   const loaders = {
     Dog: {
       async owner (queries, { reply }) {
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -285,7 +294,9 @@ test('support context in loader', async (t) => {
     Dog: {
       async owner (queries, context) {
         t.equal(context.app, app)
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -440,7 +451,9 @@ test('reply is empty, throw error', async (t) => {
   const loaders = {
     Dog: {
       async owner (queries) {
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -497,7 +510,9 @@ test('loaders support custom context', async (t) => {
           },
           params: {}
         }])
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -565,7 +580,7 @@ test('subscriptions properly execute loaders', t => {
     },
     loaders: {
       Dog: {
-        owner: async () => [owners[dogs[0].name]]
+        owner: async () => [{ ...owners[dogs[0].name] }]
       }
     },
     subscription: {
@@ -608,10 +623,10 @@ test('subscriptions properly execute loaders', t => {
       if (data.type === 'connection_ack') {
         app.graphql.pubsub.publish({
           topic: 'PINGED_DOG',
-          payload: { onPingDog: dogs[0] }
+          payload: { onPingDog: { ...dogs[0] } }
         })
       } else if (data.id === 1) {
-        const expectedDog = dogs[0]
+        const expectedDog = { ...dogs[0] }
         expectedDog.owner = owners[dogs[0].name]
 
         t.same(data.payload.data.onPingDog, expectedDog)
@@ -720,47 +735,57 @@ test('Pass info to loader if cache is disabled', async (t) => {
 
   const loaders = {
     Dog: {
-      async owner (queries, context) {
-        t.equal(context.app, app)
-        return queries.map(({ obj, info }) => {
-          // verify info properties
-          t.equal(info.operation.operation, 'query')
-
-          const resolverOutputParams = info.operation.selectionSet.selections[0].selectionSet.selections
-          t.equal(resolverOutputParams.length, 3)
-          t.equal(resolverOutputParams[0].name.value, 'dogName')
-          t.equal(resolverOutputParams[1].name.value, 'age')
-          t.equal(resolverOutputParams[2].name.value, 'owner')
-
-          const loaderOutputParams = resolverOutputParams[2].selectionSet.selections
-
-          t.equal(loaderOutputParams.length, 2)
-          t.equal(loaderOutputParams[0].name.value, 'nickName')
-          t.equal(loaderOutputParams[1].name.value, 'age')
-
-          return owners[obj.dogName]
-        })
+      owner: {
+        async loader (queries, context) {
+          t.equal(context.app, app)
+          return queries.map(({ obj, info }) => {
+            // verify info properties
+            t.equal(info.operation.operation, 'query')
+
+            const resolverOutputParams = info.operation.selectionSet.selections[0].selectionSet.selections
+            t.equal(resolverOutputParams.length, 3)
+            t.equal(resolverOutputParams[0].name.value, 'dogName')
+            t.equal(resolverOutputParams[1].name.value, 'age')
+            t.equal(resolverOutputParams[2].name.value, 'owner')
+
+            const loaderOutputParams = resolverOutputParams[2].selectionSet.selections
+
+            t.equal(loaderOutputParams.length, 2)
+            t.equal(loaderOutputParams[0].name.value, 'nickName')
+            t.equal(loaderOutputParams[1].name.value, 'age')
+
+            return { ...owners[obj.dogName] }
+          })
+        },
+        opts: {
+          cache: false
+        }
       }
     },
     Cat: {
-      async owner (queries, context) {
-        t.equal(context.app, app)
-        return queries.map(({ obj, info }) => {
-          // verify info properties
-          t.equal(info.operation.operation, 'query')
+      owner: {
+        async loader (queries, context) {
+          t.equal(context.app, app)
+          return queries.map(({ obj, info }) => {
+            // verify info properties
+            t.equal(info.operation.operation, 'query')
 
-          const resolverOutputParams = info.operation.selectionSet.selections[1].selectionSet.selections
-          t.equal(resolverOutputParams.length, 2)
-          t.equal(resolverOutputParams[0].name.value, 'catName')
-          t.equal(resolverOutputParams[1].name.value, 'owner')
+            const resolverOutputParams = info.operation.selectionSet.selections[1].selectionSet.selections
+            t.equal(resolverOutputParams.length, 2)
+            t.equal(resolverOutputParams[0].name.value, 'catName')
+            t.equal(resolverOutputParams[1].name.value, 'owner')
 
-          const loaderOutputParams = resolverOutputParams[1].selectionSet.selections
+            const loaderOutputParams = resolverOutputParams[1].selectionSet.selections
 
-          t.equal(loaderOutputParams.length, 1)
-          t.equal(loaderOutputParams[0].name.value, 'age')
+            t.equal(loaderOutputParams.length, 1)
+            t.equal(loaderOutputParams[0].name.value, 'age')
 
-          return owners[obj.catName]
-        })
+            return { ...owners[obj.catName] }
+          })
+        },
+        opts: {
+          cache: false
+        }
       }
     }
   }
@@ -768,8 +793,7 @@ test('Pass info to loader if cache is disabled', async (t) => {
   app.register(GQL, {
     schema,
     resolvers,
-    loaders,
-    cache: false
+    loaders
   })
 
   await app.ready()
@@ -858,7 +882,9 @@ test('should not pass info to loader if cache is enabled', async (t) => {
     Dog: {
       async owner (queries) {
         t.equal(queries[0].info, undefined)
-        return queries.map(({ obj }) => owners[obj.name])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
       }
     }
   }
@@ -908,3 +934,84 @@ test('should not pass info to loader if cache is enabled', async (t) => {
     }
   })
 })
+
+test('loaders create batching resolvers', { only: true }, async (t) => {
+  const app = Fastify()
+
+  const loaders = {
+    Dog: {
+      async owner (queries, { reply }) {
+        // note that the second entry for max is cached
+        const found = queries.map((q) => {
+          return { obj: q.obj, params: q.params }
+        })
+        t.same(found, [{
+          obj: {
+            name: 'Max'
+          },
+          params: {}
+        }, {
+          obj: {
+            name: 'Charlie'
+          },
+          params: {}
+        }, {
+          obj: {
+            name: 'Buddy'
+          },
+          params: {}
+        }, {
+          obj: {
+            name: 'Max'
+          },
+          params: {}
+        }])
+        return queries.map(({ obj }) => {
+          return { ...owners[obj.name] }
+        })
+      }
+    }
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers,
+    loaders,
+    cache: false
+  })
+
+  const res = await app.inject({
+    method: 'POST',
+    url: '/graphql',
+    body: {
+      query
+    }
+  })
+
+  t.equal(res.statusCode, 200)
+  t.same(JSON.parse(res.body), {
+    data: {
+      dogs: [{
+        name: 'Max',
+        owner: {
+          name: 'Jennifer'
+        }
+      }, {
+        name: 'Charlie',
+        owner: {
+          name: 'Sarah'
+        }
+      }, {
+        name: 'Buddy',
+        owner: {
+          name: 'Tracy'
+        }
+      }, {
+        name: 'Max',
+        owner: {
+          name: 'Jennifer'
+        }
+      }]
+    }
+  })
+})