Modify use of pause

[andyrichardson]

The proposed changes here are aimed to simplify the use-cases associated with the pause argument.

Rather than pause preventing requests outright - it will pause the functionality of auto fetching - whether that be on mount or when the query has changed.

The reasons for this are:

• It allows users to pause auto fetching while having the ability to manually fetch on demand (currently not an option - see example problem here)
• It improves the seperation of concerns. Config options should influence how urql acts, manual operations can be prevented with a conditional inside component logic.

There's also a small typing fix for useImmediateEffect.

Example usage

const MySearch = () => {
  const [variables, setVariables] = useState({ searchString: "" });
  const [searchResults, fetchSearchResults] = useQuery({ query, variables, skip: true });

  const throttledFetch = throttle(fetchSearchResults, 200); 

  const handleChange = (e) => {
    setVariables({ searchString: e.target.value });
    throttledFetch();
  };

  return (
    <input value={vars.searchString} onChange={handleChange} />
    <Results values={searchResults} />
  )
}

Additional notes

The advantages of this approach are pretty clear but it does highlight a bug that we haven't yet addressed - updating variables/query values and immediately executing them in the same render doesnt act as expected.

I've created an issue about this and I believe the best way to go about this is to allow arguments to be overridden when manually fetching.

[kitten]

Seems good but it'd be great to readd the comments and slightly rewrite them. Especially the one here that explains with this is using useImmediateEffect 😊

[andyrichardson]

Yup agreed - I'll add something back in 👍

[parkerziegler]

Are we initializing this to undefined with the expectation that teardown.current will never get called until it gets reassigned to the return type of effect? Seems like a safe assumption based on how this effect runs (teardown synchronously getting re-assigned in the first if below) but just wanted to get the reasoning on this change.

[kitten]

Yea, should be safe since teardown functions can be undefined in the worst case 👍

[parkerziegler]

Nice I like this. So the flow here is:

• We check args.pause on initial mount to handle the case where a user just wants the executeQuery fn.
• Also check it on all subsequent changes and prevent queries when things update.

❤️Love it. My one question is should L69, if it's being treated as the cleanup function, be written as () => setState(s => ({ ...s, fetching: false }));? Seems we're returning void currently rather than () => void.

[parkerziegler]

Two small questions but this looks awesome @andyrichardson and I can see how it solves the use case you mention ✅