useLazyQuery onCompleted called on every re-render

[PovilasSlekys]

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

[huongdevvn]

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",

[huongdevvn]

@hwillson Could you take a look?

[vilvaathibanpb]

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

[bsonntag]

@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.

[vilvaathibanpb]

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

[vilvaathibanpb]

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?

[bsonntag]

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:

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

  return (
    <>
      <button onClick={() => fetchUsers()}>Fetch</button>
      {/* Render users */}
    </>
  );
}

[vilvaathibanpb]

@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.

[bsonntag]

All I know is that that change was introduced on #3497

Maybe @hwillson can explain?

[vilvaathibanpb]

Sure. lets wait for him to respond

[dnknitro]

Any update on this issue?

[vilvaathibanpb]

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

[Siggnja]

Any updates?

[holliepearson]

Any progress or possible work around?

[PovilasSlekys]

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.

[vilvaathibanpb]

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

[hwillson]

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 #3497 and publish a new RA version today, would that be a good stop gap for now?

[grollec]

Would be nice, thanks.

[hwillson]

Rolled back and deployed in 3.1.3 - thanks!

[santiagoalmeidabolannos]

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

[RuslanZavacky]

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.

[futantan]

this issue still happens in version 3.1.3

[MarkPolivchuk]

@hwillson Ran into this on 3.1.3 today.

[dylanwulf]

@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

[arindamINT]

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>
    </>
  );
}

[yairtal]

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

[tomerzcod7]

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

[dhavaljardosh]

 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.

[chattling]

@dhavaljardosh How about this:

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

[arindamINT]

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",
			},
		},
	});
}, []);

[Phoenix1405]

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.
}

[chattling]

@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.

[arindamINT]

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.

[riturajratan]

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
    },
  });

[LucasClaude]

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]);

[squarewave24]

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>

[LucasClaude]

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.

[squarewave24]

@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 :)

[arindamINT]

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.