-
-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for GraphQL requests with partial results #401
Comments
Or we could introduce a new context method like |
Hey! This is something we need to cover. Perhaps, |
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:
Really eager to hear what others think :) |
I'm not sure exposing the I like the fact that graphql/rest handlers expose only a set of methods that make sense to their respective APIs. As a user, as I suggested before, I would imagine to have a dedicated method for covering the use case of partial results.
Just my 2 cents... |
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. |
This could be a sensible API: res(ctx.data(), ctx.errors()) With those context functions implicitly merging the response body under the hood. |
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 😄. |
Nit: |
My bad. Corrected my prior comment for clarity. |
Wanted to take the change to help out. Hope that is okay. I implemented the I am not against |
Hi guys, I like the idea to |
Thanks for chiming in. |
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 |
I updated the respective pull request to use |
Hi there, Our Graphql schema is set up with instrumentation to provide a response which looks like: {
"data": {
"check": "ok"
},
"extensions": {
"id": "123",
"otherVariables": "etc"
}
} Now, because |
Hey, @OLingard. Thanks for suggesting that! It sounds like a new feature we'd love to work on. Do you mind opening a new feature request and describing how you intend to use it? |
Hey @kettanaito , I've added a pull request (#981) for the potential change |
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
then this query can partially fail. In the example above the
myOrders
could be resolved while themyProfile
can fail and benull
. The resulting response could be something likeI don't see a way to support this case via
msw
and thegraphql
mocking system. Theerrors
context to how I read it's code terminates the request. While thedata
context seems to be a shorthand forjson.data()
. As a result I can't use both as inctx.data
andctx.errors
. Whilectx.json
is not exposed when usinggraphql
. Mocking the request viarest
feels cumbersome as matching the query is hard.Describe the solution you'd like
Either the
errors
onctx
does not terminate the request and allows for combination withdata
orjson
is exposed on the context when usinggraphql
.The text was updated successfully, but these errors were encountered: