[klarstrup]

Background: I have to work against an API where certain DELETE requests will respond happily with that empty {} object, and RestLink asplodes.

This PR makes it so that a {} 200 API response succesfully becomes a null when no fields have been requested, as there is no such thing as an empty object in GraphQL.

Not sure whether this would be considered a bugfix or a new feature, I couldn't find any prior discussion about behavior for this scenario.

[apollo-cla]

@klarstrup: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[abrahamcuenca]

@klarstrup looks like there was a failing test at src/__tests__/restLink.ts Is this related to the changes or was this already failing before?

[fbartho]

@klarstrup various GraphQL schema parsers throw linting or parsing errors when you don't select fields for every type of Query.

(We had a workaround/convention of typing such responses IrrelevantResponse for that situation, and you can always query for __typename)

I'm not sure I want to merge this if it's supporting an "invalid" variant of the GraphQL language. What are your thoughts? /cc @abrahamcuenca

[klarstrup]

I'm not sure I follow @fbartho, this doesn't concern GraphQL parsing at all but rather an occasional scenario where some JSON APIs will respond with an empty object in some cases. This can't exactly be modeled in GraphQL so I reasoned for falling back to null as that corresponds semantically to the "empty response" that {} conveys.

Edit: Ah I see now what you mean, it has been a while. But top level fields with no selection set(primitive values) are valid to my recollection.

Edit 2: Yeah it's even an example in the docs https://spec.graphql.org/June2018/#example-e2969

[abrahamcuenca]

@klarstrup various GraphQL schema parsers throw linting or parsing errors when you don't select fields for every type of Query.

(We had a workaround/convention of typing such responses IrrelevantResponse for that situation, and you can always query for __typename)

I'm not sure I want to merge this if it's supporting an "invalid" variant of the GraphQL language. What are your thoughts? /cc @abrahamcuenca

I agree @fbartho we should avoid any invalid variants as it could throw anyone trying to troubleshoot into a rabbit hole. We can hold off on merging until we get a better workaround.

[klarstrup]

Still not following. Every field of Query(or indeed Mutation) doesn't necessarily have fields.