apollographql / apollo-client-nextjs

Apollo Client support for the Next.js App Router
https://www.npmjs.com/package/@apollo/experimental-nextjs-app-support
MIT License
448 stars 35 forks source link

debounce triggered by useQuery in same page file #347

Closed Netail closed 2 months ago

Netail commented 2 months ago

Summary

Custom debounce hook is triggered when using useQuery hook in same page file, while non of the inputs of the debounce hook changed. Repro steps are located in the repo's README.

Repro: https://github.com/Netail/repro-apollo-nextjs

jerelmiller commented 2 months ago

Hey @Netail 👋

I think there is a bit of a feud between the updates happening in the useQuery and the way you've implemented useDebounce. I see that your useDebounce function will always trigger that useEffect when the callback option changes. Since you're passing a new function to callback every render, this means your useDebounce will run your useEffect every render:

useDebounce({
  callback: () => { // <-- New function every render
    console.log('test');
  },
  // ...
});

You're going to either need to use a useCallback here to prevent that callback function from being created every render, or you're going to need to update useDebounce to be able to handle inline functions created every render (this is a pretty good StackOverflow on doing this: https://stackoverflow.com/questions/76335194/is-this-an-accurate-polyfill-of-reacts-useeffectevent).

I can see that by moving callback inside of a useCallback prevents the issue you're seeing when useQuery is used in your component (and even reduces the log by 1!).

const callback = useCallback(() => {
  // You'll now only see this once!
  console.log('test');
});

// now 
useDebounce({
  value: "",
  callback,
  delay: 500,
});
Netail commented 2 months ago

Hi @jerelmiller 🌞

Correct, I did notice I should wrap it in a useCallback. However, is it correct behaviour that the page gets rerenderd in an infinite loop when using useQuery?

jerelmiller commented 2 months ago

@Netail no, but I did not see that happening with the useCallback change above. The component is rerendered a few times due to the lifecycle of useQuery and it requesting data, but I did not see it rendering in an infinite loop. Are you seeing something different? If so, can you update your reproduction to show this behavior with the fix above in place?

Netail commented 2 months ago

@Netail no, but I did not see that happening with the useCallback change above. The component is rerendered a few times due to the lifecycle of useQuery and it requesting data, but I did not see it rendering in an infinite loop. Are you seeing something different? If so, can you update your reproduction to show this behavior with the fix above in place?

Mhm no I thought the lifecycle of the useQuery regenerating in an infinite loop was a bug (thus also triggering the debounce)

jerelmiller commented 2 months ago

@Netail if I comment out the useDebounce hook in that same file or provide the fix for the callback option while using useQuery, I don't see it rendering in an infinite loop.

At this point, I'm not sure there is anything actionable for the maintainers to do here so I'm going to go ahead and close this issue. If you are able to reproduce the infinite loop with the fixes in places, let us know and we'd be happy to dig in further.

github-actions[bot] commented 2 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.