From 28c7524e006a0d5178b5eb80bdac38f246e56cc7 Mon Sep 17 00:00:00 2001 From: bquinn Date: Mon, 28 Sep 2020 08:45:34 -0700 Subject: [PATCH] consider the so-far-traversed path (instead of individual segments) when checking for null when merging errors --- lib/delegate/__tests__.js | 72 ++++++++++++++++++++++++++++++++++++++- lib/delegate/index.js | 13 +++---- package.json | 1 + 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/lib/delegate/__tests__.js b/lib/delegate/__tests__.js index e902dbf..8205d1d 100644 --- a/lib/delegate/__tests__.js +++ b/lib/delegate/__tests__.js @@ -1364,7 +1364,7 @@ Test('delegateToComponent - target field non-nullable arg is not passed', async t.end(); }); -Test('delegateToComponent - with errors', async (t) => { +Test('delegateToComponent - with errors (selection set order doesnt matter)', async (t) => { const primitive = new GraphQLComponent({ types: ` type Query { @@ -1509,6 +1509,76 @@ Test('delegateToComponent - with errors - delegate graphql data result is comple t.equal(result.data.bar, null, 'query resolves as expected'); t.equal(result.errors.length, 1, '1 error returned'); t.equal(result.errors[0].message, 'Cannot return null for non-nullable field Foo.b.', 'expected error is propagated regardless of completely null delegate result'); + t.deepEqual(result.errors[0].path, ['foo', 'b']); t.end(); }); +Test('delegateToComponent - errors merged as expected for non-nullable list that allows nullable items', async (t) => { + const primitive = new GraphQLComponent({ + types: ` + type Query { + foos: [Foo]! + } + + type Foo { + a: String! + } + `, + resolvers: { + Query: { + foos() { + return [ { a: 'bar'} , {}, { a: 'baz'} ]; + } + } + } + }); + + const composite = new GraphQLComponent({ + types: ` + type Query { + bar: Bar + } + + type Bar { + foos: [Foo]! + } + `, + resolvers: { + Query: { + async bar(_root, _args, context, info) { + const foos = await GraphQLComponent.delegateToComponent(primitive, { + info, + contextValue: context, + targetRootField: 'foos', + subPath: 'foos' + }); + return { foos }; + } + } + }, + imports: [primitive] + }); + + const document = gql` + query { + bar { + foos { + a + } + } + } + `; + + const result = await graphql.execute({ + document, + schema: composite.schema, + contextValue: {} + }); + + t.deepEqual(result.data.bar.foos[0], { a: 'bar' }, 'first item of list resolved as expected'); + t.deepEqual(result.data.bar.foos[2], { a: 'baz' }, 'third item of list resolved as expected'); + t.equal(result.errors.length, 1, 'one error returned'); + t.equal(result.errors[0].message, 'Cannot return null for non-nullable field Foo.a.'); + t.deepEqual(result.errors[0].path, ['foos', 1, 'a']); + t.end(); +}); \ No newline at end of file diff --git a/lib/delegate/index.js b/lib/delegate/index.js index 02cb9aa..4a92906 100644 --- a/lib/delegate/index.js +++ b/lib/delegate/index.js @@ -12,7 +12,8 @@ const { visit, visitWithTypeInfo } = require('graphql'); -const deepSet = require('lodash.set'); +const set = require('lodash.set'); +const get = require('lodash.get'); const debug = require('debug')('graphql-component:delegate'); /** @@ -195,14 +196,14 @@ const createSubOperationDocument = function (component, targetRootField, args, s const mergeErrors = function (data, errors) { for (const error of errors) { const { path } = error; - const mergePath = []; - for (const segment of path) { - mergePath.push(segment); - if (!data[segment]) { + let depth = 1; + while (depth <= path.length) { + if (!get(data, path.slice(0, depth))) { break; } + depth++; } - deepSet(data, mergePath, error); + set(data, path.slice(0, depth), error); } } diff --git a/package.json b/package.json index 772bda8..01737fd 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "cuid": "^2.1.8", "debug": "^4.1.1", "graphql-tools": "^6.0.10", + "lodash.get": "^4.4.2", "lodash.set": "^4.3.2" }, "peerDependencies": {