React-apollo: Skip + refetch() not working in 2.1.2

Created on 6 Apr 2018  ·  16Comments  ·  Source: apollographql/react-apollo

Intended outcome:
I'm following the example in the document at https://www.apollographql.com/docs/react/essentials/queries.html
``` const DogPhoto = ({ breed }) => ( <Query query={GET_DOG_PHOTO} variables={{ breed }} skip={!breed} pollInterval={500} > {({ loading, error, data, startPolling, stopPolling }) => { if (loading) return null; if (error) returnError!: ${error}`;

  return (
    <img src={data.dog.displayImage} style={{ height: 100, width: 100 }} />
  );
}}


);
````

I was expecting the refetch() works when query component has skip set to true.

Actual outcome:

The refetch() doesn't fire a request. Nothing happens. No errors and warnings.

How to reproduce the issue:

My example:
https://codesandbox.io/s/wyp8qnmp88

The Query component has skip set to true, and clicking on the button doesn't fetch the data.

Version

has-reproduction

Most helpful comment

Should we make skip accept a function (variables?: TVariables) => boolean? That way Query can call it for every fetch.

All 16 comments

Isn't that the normal behaviour? If you hard code your skip to be always true, then refetch wont fire a request because you're skipping your query.

If you look at the code of the Query component, inside of componentDidMount() there is a check for the skip prop. So I don't think this is an issue/bug. Correct me if I'm wrong.

https://github.com/apollographql/react-apollo/blob/99a8a2f6b8e47dffd5f3e4623f330df8c4938569/src/Query.tsx#L194

I ran into this as well, and @DriesH is right - it won't fire unless skip is omitted or false.

What I ended up doing is exporting the apollo client itself and then manually running the query instead of using <Query/>

const res = await client.query({
      query: SEARCH_ITEMS_QUERY,
      variables: { searchTerm: e.target.value },
    });
    this.setState({ items: res.data.items, loading: false });

I'm using Graphql's @skip directive instead of using skip as an option for Apollo. It makes your query available to refetch

https://medium.com/front-end-developers/graphql-directives-3dec6106c384

Should we make skip accept a function (variables?: TVariables) => boolean? That way Query can call it for every fetch.

Here's a workaround I used. It's a class that replaces Query. It waits until we call refetch to do the first load. Also fixes the bug with loading state and skip from https://github.com/apollographql/react-apollo/issues/1869

Original reproduction fixed with this workaround: https://codesandbox.io/s/1rvvvlnwmq

Code of the workaround wrapper (use in place of Query). The code can be modified to make skip accept a function instead of reading from state.

class SkipFixWrapper extends React.Component {
  constructor() {
    super();
    this.state = {
      skip: true,
      variables: {}
    };
  }

  render() {
    const { children, query } = this.props;
    const { skip } = this.state;

    const variablesMerged = {
      ...(this.props.variables || {}),
      ...this.state.variables
    };

    return (
      <Query query={query} skip={skip} variables={variablesMerged}>
        {queryState => {
          queryState.refetch = variables => {
            this.setState({ skip: false, variables });
          };
          queryState.loading = queryState.loading && !skip;

          return children(queryState);
        }}
      </Query>
    );
  }
}

It sounds like a good idea to let skip accept a function

Use ApolloConsumer to fetch query manually- it's documented here
https://www.apollographql.com/docs/react/essentials/queries.html#manual-query

Use ApolloConsumer to fetch query manually- it's documented here
https://www.apollographql.com/docs/react/essentials/queries.html#manual-query

As far as I can tell, doing that doesn't let you display the fetched data unless you involve component state or some other intermediate store for the returned data, in which case the cache is no longer the only source of truth.

As far as the OP goes, I agree, skipping should just skip the first data fetch, but manually calling refetch() or fetchMore() should work as expected.

In any case, better documentation as to the intent of the skip property, as well as how it interacts with the rendering of <Query>'s children and the values for data, loading and error props would be great.

I'm in agreement that this behavior should be available via the <Query> component's render props. It's a common use case and wouldn't require the use of manual queries. Being able to pass a function to skip would be great.

Until then, here's a workaround that I'm using, implementing hooks. It's pretty close to @flpvsk 's solution, except:

  • it doesn't merge the new variables in with the old. (You get variables in the render props, so you could merge this in with your new variables at a different point)
  • it falls back to the default refetch after the first fetch, or when you're not skiping in the first place

So it's a little more consistent with the default refetch, which could be helpful if you're trying to patch an existing codebase.

import { Query as ApolloQuery } from 'react-apollo'

const Query = ({ children, variables: originalVariables, skip, ...queryProps}) => {
    const [fetchAfterSkip, setFetchAfterSkip] = useState(false)
    const [fetchAfterSkipVariables, setFetchAfterSkipVariables] = useState(null)

    const variables = fetchAfterSkipVariables || originalVariables || {}
    const shouldSkip = skip === true && fetchAfterSkip === false
    return (
        <ApolloQuery variables={variables} skip={shouldSkip} {...queryProps}>
            {({ refetch, ...response }) => {
                const refetchWhenSkipped = (newVariables) => {
                    if (fetchAfterSkip) {
                        refetch(newVariables)
                    } else {
                        setFetchAfterSkip(true)
                        setFetchAfterSkipVariables(newVariables)
                    }
                }
                return children({
                    ...response,
                    refetch: skip ? refetchWhenSkipped : refetch
                })
            }}
        </ApolloQuery>
    )
}

Then, the child components can use refetch(variables) to 'un-skip' the query.

Allowing skip to be a function would be simple and I'd be happy to make a PR!

@good-idea is fetchedAfterSkip supposed to be fetchAfterSkip?

@valoricDe Yeah, good catch, just edited my post. Thanks!

+1 for this

I am trying out hooks right now so I improvised a useManualQuery hook:

import { useState, useEffect } from 'react';
import { useApolloClient } from '@apollo/react-hooks';

const IDLE = {
  loading: false,
  error: undefined,
  data: {},
};

export default function useManualQuery(query) {
  const client = useApolloClient();

  const [status, setStatus] = useState(IDLE);
  const [variables, setVariables] = useState();

  useEffect(() => {
    if (variables) {
      setStatus({
        ...IDLE,
        loading: true,
      });
      client
        .query({
          query,
          variables,
        })
        .then(
          ({ data }) =>
            setStatus({
              ...IDLE,
              data,
            }),
          error => {
            setStatus({
              ...IDLE,
              error,
            });
          }
        );
    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [query, variables]);
  return [status, setVariables];
}

Styled after React's own useState hook, it returns an array with two elements, the first one the status, comprising data, loading and error properties, the second, a function that can be called to trigger the query, with the query variables as its only argument:

export default function SomeComponent() {
   const [{loading, error, data}, doQuery] = useManualQuery(gql` ... `);
   // ......
   // Somewhere in the code, usually responding to some event elsewhere:
   doQuery({id: ...});
}

It is just a toy right now, not even a work-in-progress and certainly not a generic solution since I am using it in a very particular instance. A more generic version would have useManualQuery receiving, instead of just the query, an object with all the set of properties for any query, and the variables passed to the action function would be merged with the default ones.

Also, the order of the array elements useManualQuery returns could be reversed to match the order of useMutation that is, action first, status second.

Purists would point out that I should use useReducer for the state but I honestly can't find a reason to go to the trouble of assembling action objects to get dispatched when I can just as easily assemble the status straight away, at least not at this level of complexity.

having the same issue with hooks

React Apollo has been refactored to use React Hooks behind the scenes for everything, which means a lot has changed since this issue was opened (and a lot of outstanding issues have been resolved). We'll close this issue off, but please let us know if you're still encountering this problem using React Apollo >= 3.1.0. Thanks!

@hwillson can confirm this is still an issue https://codesandbox.io/s/pupstagram-qgsee, using apollo hooks 3.1.1

Was this page helpful?
0 / 5 - 0 ratings