apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Support @skip and @include directives on the query level #349

Open ktsosno opened 3 years ago

ktsosno commented 3 years ago

Feature request

Currently @skip and @include directives only exist on the field level, so we can't skip a top level query if there is a directive value set.

Motivation

We're trying to batch queries together to avoid multiple tiered network requests. The issue is with the following:

query MyQuery(
  $requiredVarA: String!
  $skipCriteria: boolean
) {
  someQuery {
    ...
  }
  someQuery {
    ...
  }
  conditionalQuery(value: $requiredVarA) @skip(if: $skipCriteria) {
    ...
  }
}

In this situation, a the $requiredVarA will be null at the time of the conditionalQuery query being invoked, this will mean a coerced non-null value for $requiredVarA and the query will still attempt fire despite not have any fields.

Proposed solution

Proposing a directive that precedes the query:

query MyQuery(
  $requiredVarA: String!
  # Skip criteria is !!requiredVarA
  $includeCriteria: boolean
) {
  ...
  @include(if: $includeCriteria) {
    conditionalQuery(value: $requiredVarA) {
      ...
    }
  }
}

This allows us to batch multiple queries together with the wrapped query only firing if the $includeCriteria is true.

Outstanding Questions

What kinds of effects would this have on cached results if a portion of the top level request is changing? I'm assuming it would just rehydrate the cache but not invoke any kind of onComplete hooks. An auto cache-and-network fetch policy?

dylanwulf commented 3 years ago

Please try to avoid typing @skip and @include in github without wrapping them in backticks like this: `@skip`. Without the backticks, you wind up accidentally tagging people with the usernames skip and include.

we can't skip a top level query if there is a directive value set.

What you're referring to as "top level queries" are actually just normal fields on the Query type. @skip and @include already work on these fields.

Your problem is not that the conditionalQuery field is being invoked. Your problem is that you declared a variable called requiredVarA as a non-nullable string, then you passed in null. If you pass in null for a variable that you declared as non-nullable, then you will get an error no matter how/where this variable is used.

query MyQuery(
  $requiredVarA: String! # <-- variable is declared as non-nullable here. This is where the error is coming from.
  $skipCriteria: boolean
) {
  someQuery {
    ...
  }
  someQuery {
    ...
  }
  conditionalQuery(value: $requiredVarA) @skip(if: $skipCriteria) {
    ...
  }
}

Some options to work around this:

  1. You could pass in an empty string for requiredVarA when you don't have a value for it yet
  2. You could use the skip option on useQuery when you don't have a value for requiredVarA yet: https://www.apollographql.com/docs/react/data/queries/#skip
ktsosno commented 3 years ago

Thanks - updated my original comment. I can't change the argument types since conditionalQuery would expect $requiredVarA to be that of String!. If I drop the non-nullable constraint, the types will mismatch with something along the lines of:

Variable "$requiredVarA" of type "String" used in position expecting type "String!".

So the problem ends up being that the field on the query is executed before the skip or include can be evaluated. If I use the skip option on the useQuery hook, I'll short circuit the execution of the entire query. In the scenario I have here, I still want someQuery to be invoked in absence of the conditionalQuery.

dylanwulf commented 3 years ago

@skip and @include are executed on the server side. Sounds like this problem is more relevant to the server than the client. This project (apollo-client) is for the client side.

ktsosno commented 3 years ago

The final constructed query is executed on the server side. This doesn't change the potential semantics of the client query I'm trying to construct. It feels reasonable to be able to exclude a query on a Query type by some value without having to change my argument types. Relaxing the argument types to be non-required changes the shape of the query rather than allowing me to conditionally invoke a query in the first place.

There are plenty of ways around it, but thought I'd bring it up, hence this being a feature request and not a bug.

bignimbus commented 1 year ago

Hi @ktsosno 👋🏻 I'm doing some housekeeping and transferred your feature request here. Thanks so much for your patience!