apollographql / react-apollo

:recycle: React integration for Apollo Client
https://www.apollographql.com/docs/react/
MIT License
6.85k stars 787 forks source link

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

Open stefanmaric opened 5 years ago

stefanmaric commented 5 years ago

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:

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 commented 5 years ago

I just confirmed this happens only with the HoC API.

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

stefanmaric commented 5 years ago

I guess it has to do with this line: https://github.com/apollographql/react-apollo/blob/3e0a436561de1b37358166d4e9a4e14af5a94693/packages/hoc/src/query-hoc.tsx#L66

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 commented 5 years ago

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 commented 5 years ago

+1 I experience the same issue with HoCs.

GiancarlosIO commented 5 years ago

:eyes:

noahm commented 5 years ago

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 commented 5 years ago

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: https://github.com/apollographql/apollo-client/pull/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: https://github.com/apollographql/apollo-client/pull/5294

tomasfrancisco commented 5 years ago

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 commented 5 years ago

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