Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

React Apollo V3: graphql breaks when using @include / @skip directive and skip option is true #3367

Open
stefanmaric opened this issue Aug 14, 2019 · 9 comments

Comments

@stefanmaric
Copy link

Intended outcome:

I expect graphql to work as it did with react-apollo@2.

Actual outcome:

After upgrading to react-apollo@3, components that use graphql and also use the @include / @skip directive at a top-level field, break with the following error if skip is true:

Uncaught Invariant Violation: Invalid variable referenced in @include directive.

How to reproduce the issue:

Here an simplified test case I crafted: https://codesandbox.io/s/react-apollo-v3-skip-test-case-h5bw1

If you downgrade react-apollo@2.5.8 inside the sandbox, you will notice it works well.

To clarify, these conditions have to apply for it to break:

  • Using react-apollo v3
  • Having an @include in a top-level field (it doesn't break if it is in a nested field)
  • skip option is set to true

Quick peak:

  query myPokemon($showOthers: Boolean!, $showAttacks: Boolean!) {
    others: pokemons(first: 3) @include(if: $showOthers) {
      name
    }
    pokemon(name: "Pikachu") {
      id
      name
      attacks @include(if: $showAttacks) {
        fast {
          name
        }
        special {
          name
        }
      }
    }
  }
const Pokemon = graphql(pokemonQuery, {
  options: ({ showAttacks, showOthers }) => ({
    variables: {
      showAttacks: Boolean(showAttacks),
      showOthers: Boolean(showOthers)
    }
  }),
  skip: ({ skip }) => Boolean(skip)
})(({ data }) => (
  <pre>
    <code>{JSON.stringify(data, null, 2)}</code>
  </pre>
));

Version

This happens with react-apollo@3.0.0, and also with the new, individual packages.

The envinfo preset seems outdated and doesn't include the new namespaced packages.

In the prod setup I got this error, we migrated using the namespaced packages. But was also able to reproduce with the react-apollo bundle.

@stefanmaric
Copy link
Author

I just confirmed this happens only with the HoC API.

I added RenderProps and Hooks examples to the test case and those work.

@stefanmaric
Copy link
Author

stefanmaric commented Aug 14, 2019

I guess it has to do with this line:

if (!shouldSkip && !opts.variables && operation.variables.length > 0) {

I would presume calculateVariablesFromProps is not called to save some CPU cycles, but since the <Query /> component is rendered anyways, it is given empty variables and somehow it seems to be compiling the query even tho the prop skip is set to true.

@stefanmaric
Copy link
Author

I tested locally by removing shouldSkip from that conditional, and it indeed works and all test pass.

But not sure if it is something that has to be fixed on the <Query /> component instead.

@cagdastulek
Copy link

+1
I experience the same issue with HoCs.

@hwillson hwillson self-assigned this Aug 19, 2019
@giancarlosisasi
Copy link

👀

@hwillson hwillson removed their assignment Sep 6, 2019
@noahm
Copy link

noahm commented Sep 7, 2019

I'm having this issue as well (or something very similar to it), except I'm using a rather ancient react-apollo@2.1.9 and it only started happening once I upgraded apollo-client and apollo-cache-inmemory to versions 2.6 and 1.6 respectively.

@noahm
Copy link

noahm commented Sep 9, 2019

I've been digging in a bit further to this issue today. I've narrowed the issue down to a change between apollo-client@2.6.0-beta.6 and apollo-client@2.6.0-beta.7, this PR: apollographql/apollo-client#4743. There used to be a try-catch around the initial attempt to read a query from the cache that would just return undefined when an exception was thrown (diff here).

In testing, simply re-adding a try-catch around that one statement has fixed the issues on my end. With that in mind, I think this issue would be more appropriate to be moved from react-apollo over to apollo-client. Opened a prospective fix: apollographql/apollo-client#5294

@tomasfrancisco
Copy link

I got rid of this error

Uncaught Invariant Violation: Invalid variable referenced in @include directive.

Setting a default value to the arguments I'm passing to @skip or @include, like so:

query myPokemon($showOthers: Boolean! = true, $showAttacks: Boolean! = true) {
    ...
}

@noahm
Copy link

noahm commented Oct 20, 2019

Aha, that's an excellent workaround for the time being! Thanks for sharing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants