Msw: Support for GraphQL requests with partial results

[tdeekens]

Thanks for this great library! It's a pleasure to use. While integrating it I ran into an issue which might be caused by lack of understanding on my side.

Is your feature request related to a problem? Please describe.

When using GraphQL requests which for instance load different parts of data such as

query MyQuery {
   myOrders {
      total
   }
   myProfile {
      name
   }
}

then this query can partially fail. In the example above the myOrders could be resolved while the myProfile can fail and be null. The resulting response could be something like

{
   data: {
     myOrders: {
        total: 42
     },
     myProfile: null
   },
   errors: [{
      path: 'MyQuery.myProfile',
      extensions: { code: 'foo' }
   }]
}

I don't see a way to support this case via msw and the graphql mocking system. The errors context to how I read it's code terminates the request. While the data context seems to be a shorthand for json.data(). As a result I can't use both as in ctx.data and ctx.errors. While ctx.json is not exposed when using graphql. Mocking the request via rest feels cumbersome as matching the query is hard.

Describe the solution you'd like

Either the errors on ctx does not terminate the request and allows for combination with data or json is exposed on the context when using graphql.

[emmenko]

Or we could introduce a new context method like partial({ data, errors }).

[kettanaito]

Hey!
ctx.errors does not terminate the request. The thing is that both ctx.data and ctx.errors use ctx.json internally, which strictly overrides the mocked response body. So the next utility overrides the previous one, making it impossible to combine data and errors.

This is something we need to cover. Perhaps, ctx.json could accept an extend argument internally to extend the JSON body. Needs discussion.

[tdeekens]

ctx.errors does not terminate the request.

Thanks for the clarification. My bad I must've read that wrong somewhere. First time reading the code base. Which is very nice structured by the way.

I assume we have three options at the moment:

1. Exposing json in a graphql context so that both data and errors can be set
2. Allowing json to have a merge option so that errors doesn't overwrite data and vice-versa
3. Have a partial context which feels a bit like 2.

Really eager to hear what others think :)

[emmenko]

I'm not sure exposing the json method is what we want here.

I like the fact that graphql/rest handlers expose only a set of methods that make sense to their respective APIs.
GraphQL has a very well-defined and strict spec for the response format (data and error) and allowing people to just pass any arbitrary JSON feels out of place.

As a user, as I suggested before, I would imagine to have a dedicated method for covering the use case of partial results.
This has some advantages:

• like for data() and errors(), it's very clear what the intent is
• it's easier to type, as using such dedicated method requires you to define both data and errors objects
• it fits with the GraphQL format, as partial results is something specified in the spec

Just my 2 cents...

[tdeekens]

Thanks for clarifying. Just listed options prior. Didn’t intend to pick much of a side. Sorry if it felt that way.

I agree that GraphQL is a well defined, rather small spec, and we should open the pandora’s box by exposing the json context.

I think what was suggested before, if I got it right, is that error and data would „behind the scenes“ use a some sort of ‚merge‘ option of json so that both of them can be called in sequence while not overwriting each other. Just to clarify.

It is true that a ‚partial‘ would be easier to type and gives the thing a name which is close to the GraphQL spec. I am thinking while writing how partial would work compared to json. I assume it would only accept data and errors and omit anything else being set. Would subsequent calls overwrite the prior or merge into it?

From a „users standpoint“ I just also got confused by essentially calling data and then errors overwrites by the latter. I found that rather confusing subjectively. On the other hand, is data and errors „merge“ via json it would also open the case of multiple calls to data. So calling data.foo and then data.bar would mean that both are set due to the merging. Not sure if that is intended or not. Just wanted to mention that.

[kettanaito]

This could be a sensible API:

res(ctx.data(), ctx.errors())

With those context functions implicitly merging the response body under the hood.

[tdeekens]

Thanks. If they merge under the good we would support

res(
  ctx.data({ myProfile: null }),
  ctx.data({ myOrders: { total: 2 } ),
  ctx.errors([{ path: 'myProfile', extensions: [{ code: 'foo' }] }])
)

Is that intended? I assume we only perform shallow merges, no deep merges in any case?

Contrasting that with @emmenko which would allow:

res(
  ctx.partial({ 
    data: {
      myProfile: null,
      myOrders: { total: 2 },
    },
    errors: [{ path: 'myProfile', extensions: [{ code: 'foo' }] 
  })
)

Do you have a favorite in regards to those options? Asking cause you reacted with 👍 to @emmenko's comment while suggesting the the alternative 😄.

[emmenko]

Nit: myProfile and myOrders are nested under data

[tdeekens]

Nit: myProfile and myOrders are nested under data

My bad. Corrected my prior comment for clarity.

[tdeekens]

Wanted to take the change to help out. Hope that is okay. I implemented the { merge: true} approach discussed here over there https://github.com/mswjs/msw/pull/403#pullrequestreview-497109326.

I am not against partial. Quite the contrary just wanted to explore one option discussed here.

[marcosvega91]

Hi guys,

I like the idea to merge json under the hood. I think that we can support this feature using the this util https://github.com/mswjs/msw/blob/master/src/utils/internal/mergeRight.ts

[tdeekens]

I like the idea to merge json under the hood. I think that we can support this feature using the this util https://github.com/mswjs/msw/blob/master/src/utils/internal/mergeRight.ts

Thanks for chiming in. mergeRight would perform a deep merge, right? I think we still need to see if that's intended?

[marcosvega91]

Mergeright Will be useful from top to bottom in this case. It makes sense I think and it will be used only on certain cases like the one of this issue

[tdeekens]

I updated the respective pull request to use mergeRight.

[kettanaito]

This has been implemented in https://github.com/mswjs/msw/pull/403. Will be released in the next minor version of msw.

Thank you, @tdeekens, for a superb work on this! 🎉