React-apollo: useLazyQuery onCompleted called on every re-render

Created on 17 Sep 2019  路  39Comments  路  Source: apollographql/react-apollo

Intended outcome:
useLazyQuery only calls onCompleted on a successful network request

Actual outcome:
useLazyQuery calls onCompleted after every re-render even if the result is being taken from the cache

Version

npmPackages:
apollo-cache-inmemory: ^1.6.3 => 1.6.3
apollo-client: ^2.6.4 => 2.6.4
apollo-link-context: ^1.0.19 => 1.0.19
apollo-link-http: ^1.5.16 => 1.5.16
react-apollo: ^3.1.1 => 3.1.1

Most helpful comment

Any updates?

All 39 comments

The same happen for me. onCompleted of useLazyQuery is called only once when I execute simple functions like console.log

const [fetchLazy] = useLazyQuery(LAZY_QUERY, {
    variables: { id },
    onCompleted: data => {
      console.log('data ', data);
    }
  });

But if I execute a function returned by React hooks like: useState or useReducer, onCompleted will happen many times or infinite times.

  const [data, setData] = useState(null);

  const [fetchLazy] = useLazyQuery(LAZY_QUERY, {
    variables: { id },
    onCompleted: data => {
      console.log('data ', data);
      setData(data); // multiple times
    }
  });
  const [state, dispatch] = useReducer(reducer, initialState);

  const [fetchLazy] = useLazyQuery(LAZY_QUERY, {
    variables: { id },
    onCompleted: data => {
      console.log('data ', data);
      dispatch({ type: 'completed', payload: 'data' }); // infinite times
    }
  });

Version

"@apollo/react-hooks": "^3.1.1",
"@apollo/react-ssr": "^3.1.1",
"apollo-cache-inmemory": "^1.6.3",
"apollo-client": "^2.6.4",
"apollo-link-context": "^1.0.19",
"apollo-link-error": "^1.1.12",
"apollo-link-http": "^1.5.16",

@hwillson Could you take a look?

I can work on this issue if @hwillson or @bsonntag haven't started working on this.

@vilvaathibanpb I took a look and figured the problem is here: https://github.com/apollographql/react-apollo/blob/master/packages/hooks/src/utils/useBaseQuery.ts#L57. I didn't start working on fix tho, so go ahead.

@bsonntag Sure, Let me raise a PR against it. Thanks

Hey @bsonntag , I was checking the issue and my observations are:

  1. For a normal query, the useEffect hook is checked only once for any state change.
  2. For a lazy query, the useEffect is checked for multiple times.

Do you still think, we should work on fixing the undefinded here ? https://github.com/apollographql/react-apollo/blob/master/packages/hooks/src/utils/useBaseQuery.ts#L57 .

I sense that, the useEffect hook shouldn't been checked when there is a state change? What do you think?

My understanding is that useLazyQuery's onCompleted callback should only be called when a call to the query completes.

Right now onCompleted is being called on each render after the first completed call, so if the component re-renders for any other reason (like a setState or a dispatch) onCompleted will be called again.

For example, the following component will enter an infinite re-render loop when the query completes:

```js
function Users() {
const [count, setCount] = useState(0);
const [fetchUsers, { data, loading }] = useLazyQuery(usersQuery, {
onCompleted: () => setCount(count => count + 1)
});

return (
<>

@bsonntag Yes. I completely get the issue. But when I debugged the issue, I dont think it is enough if we just change the undefinded condition when lazy is true, under useEffect here - https://github.com/apollographql/react-apollo/blob/master/packages/hooks/src/utils/useBaseQuery.ts#L57 .

May be this useEffect is reason for queryData.afterExecute which executes the onCompleted method. But, what I couldnt understand is, why useEffect is been checked for state change. In a normal query, the useEffect is checked / executed only when the query is fired. But for lazy query, it gets checked for every state change. Do you have any idea on it? Else I could dig deeper.

All I know is that that change was introduced on https://github.com/apollographql/react-apollo/pull/3497

Maybe @hwillson can explain?

Sure. lets wait for him to respond

Any update on this issue?

Hi @dnknitro, I am waiting for @hwillson to respond. If he doesn't by Tommorow, I will dig deep into it myself

Any updates?

Any progress or possible work around?

It seems the apollo team is disinterested in this so don't expect a solution any time soon.
As for a work around - don't use useLazyQuery :)
I managed to change all of my Lazy queries into normal queries to avoid this and everything works for me now. I think it might be worthwhile for you to check if you can do this as well.

I found a solution but to implement it, I need clarity on other code change which was merged earlier from a core member. But I am still waiting for response

Hi all - sorry for the delay here. We're heads down working on Apollo Client 3, which is going to supersede the React Apollo project (React Hooks functionality will be available from AC 3 directly). This unfortunately means we haven't had a chance to keep an eye on issues as much as we'd like 馃檨. That being said, if I revert https://github.com/apollographql/react-apollo/pull/3497 and publish a new RA version today, would that be a good stop gap for now?

Would be nice, thanks.

Rolled back and deployed in 3.1.3 - thanks!

What about this? It keeps happening to me, what can I do for now?

As a workaround, I've found a use of useQuery with skip property to work well. Something like this:

const [skipQuery, setSkipQuery] = useState(true);

let { loading } = useQuery(MyQuery, {
  client: myClient,
  variables: { var1, var2 },
  skip: skipQuery,
  onCompleted: data => {
    setData(data);
    setSkipQuery(true);
  },
});


useEffect(() => {
  setSkipQuery(false);

  return () => setSkipQuery(true);
}, [props.dep1, props.dep2]);

I hope that helps.

this issue still happens in version 3.1.3

@hwillson Ran into this on 3.1.3 today.

@MarkPolivchuk @futantan Please open a new issue _with a runnable reproduction_ if you're still having trouble with this. Also maybe take a look at @apollo/client v3 beta, there have been a lot of changes and there's a chance your issue has been fixed already. But again, if you're still having trouble, _please provide a runnable reproduction_

My API is not calling 2nd time when I am setting the compareItems value [];

const Compare = (props) => {
    const [compareItems, setCProducts] = useState([]);
    const [loadCompareData, { called, loading, data }] = useLazyQuery(
        GET_COMPARE_PRODUCT_BY_FILTER, {
        onCompleted: data => {
            console.log('data ', data);
            setCProducts(data.products.items);
        }
    }
    );

    useEffect(() => {
        const cProducts = JSON.parse(window.localStorage.getItem('compareProducts'));
        const sku = cProducts.map(item => item._sku);
        console.log('useEffect called', sku);
        loadCompareData({ variables: { sku }, refetch: { sku } });

    }, [compareItems]);

   // --- remove compareItem ---
    const clearCompareItem = (indx) => {
        setCProducts([]);
    }
return (
    <>
      <h1>Hello World</h1>
    </>
  );
}

I am having the same issue and the above solutions doesn't help my case

Any fix for this yet? This keeps happening to us with @apollo/client v3 beta as well.

 const [getComment, { loading, data }] = useLazyQuery(getCommentsQuery);
  useEffect(() => {
    getComment({
      variables: {
        input: {
          id: "5e5cb512e90bd40017385305",
        },
      },
    });
  }, []);

  if (data && data.getPost) {
    var allNewComments = data.getPost.comments;
    setAllComments(allNewComments);  // re-renders
  }

Not sure where I'm doing something wrong.

I want to call the query the 1st time automatically in useEffect and then manually in the functions.

@dhavaljardosh How about this:

const [getComment] = useLazyQuery(getCommentsQuery, {
  onCompleted: data => {
    setAllComments(data.getPost.comments) // re-renders
  }
});
  useEffect(() => {
    getComment({
      variables: {
        input: {
          id: "5e5cb512e90bd40017385305",
        },
      },
    });
  }, []);

I absolutely agree with @chattling. A bit addition from my point.

const [getComment] = useLazyQuery(getCommentsQuery, {
    onCompleted: data => {
      setAllComments(data.getPost.comments) // re-renders
    },
    onError: ({ graphQLErrors, networkError }) => {
        if (graphQLErrors) {
            if (graphQLErrors.length > 0) console.log(graphQLErrors[0].message, 'error');
        } else if (networkError) {
            console.log('Please check your network connection!', 'error');
        }
    },
  });
useEffect(() => {
    getComment({
        variables: {
            input: {
                id: "5e5cb512e90bd40017385305",
            },
        },
    });
}, []);

This seems to still be happening in 3.1.5 if I do useState and useLazyQuery together.

export const FirstApp = () => {
     const [orderNumber, setOrderNumber] = useState(null);
    const [ getOrderNumber , {loading, data}] = useLazyQuery(GET_ORDER_NUMBER);
    ...
    if (data && data.orderNumber) {
        setOrderNumber(data.orderNumber)
    }
    return (...) // someplace use { orderNumber } and trigger getOrderNumber using a button.
}

@Phoenix1405, the working design pattern would be to set state inside onCompleted event and you don't need to destructure loading and data, see example above.

This seems to still be happening in 3.1.5 if I do useState and useLazyQuery together.

keep a console.log inside the useEffect and onCompleted. Now observe whether the useEffect is firing multiple time or OnComplete.

If onCompleted log firing multiple times then the issue is somewhere else.

when the variables are same in useLazyQuery it will not call onCompleted but if variable is different then its call onCompleted again like in this example

  const [verifyUser] = useLazyQuery(GET_USER_BY_PHONE, {
    onCompleted: data => {
     // come when input is different
    },
  });

  verifyUser({variables: {filter: {phone: 1234567890}}});   // onCompleted will call
  verifyUser({variables: {filter: {phone: 1234567890}}});   // onCompleted will not call because same phone number
 verifyUser({variables: {filter: {phone: 11111111111}}}); // onCompleted will call again because phone number changed

so for this try fetchPolicy: 'network-only'
Now every time its call onCompleted so final code would be

const [verifyUser] = useLazyQuery(GET_USER_BY_PHONE, {
   fetchPolicy: 'network-only'
    onCompleted: data => {
     // come everytime when you call verifyUser
    },
  });

This worked for me as a workaround:

const [skipQuery, setSkipQuery] = useState(true);

let { loading, error, data } = useQuery(QUERY, {
    variables: { ...variables },
    skip: skipQuery,
});

useEffect(() => {
    if (!skipQuery) {
        const onCompleted = () => {};
        const onError = () => {};
        if (onCompleted || onError) {
            if (onCompleted && !loading && !error) {
                //SuccessFunctionHere
                setSkipQuery(true);
            }
            else if (onError && !loading && error) {
                //ErrorFunctionHere
                setSkipQuery(true);
            }
        }
    }
}, [loading, data, error]);

run into a similar issue where data is present on every re-render after click. so my download gets triggered every time :(

export function DownloadTestData(props) {
  const [getTestData, { loading, error, data }] = useLazyQuery(QUERY_TEST);
  if (data)
     saveAsCsv(data)

  return <Button onClick={getTestData}>export</Button>

run into a similar issue where data is present on every re-render after click. so my download gets triggered every time :(

export function DownloadTestData(props) {
  const [getTestData, { loading, error, data }] = useLazyQuery(QUERY_TEST);
  if (data)
     saveAsCsv(data)

  return <Button onClick={getTestData}>export</Button>

Try my above solution, I was having the same issue with useLazyQuery. Or simply check if there is data in a useEffect. Currently the if statement will get called on every re-render, where a useEffect with data as a dependency will only update when data is updated.

@LucasClaude hey thanks yeah that trick works for me too. it just seems wrong to have to use a extra flag and useEffects. seems the whole premise of useLazyQuery would be to use it with a button. how does it work for anyone else? the example doesn't have any workarounds like this so seems like a design flaw? or i am misunderstanding hooks :)

UseEffect will be called for any state change. So you just have to know first what is your requirements. Generally useLazyQuery use for late loading purpose.

Was this page helpful?
0 / 5 - 0 ratings