-
-
Notifications
You must be signed in to change notification settings - Fork 21
Description
Currently, the useGraphQL
React hook does a lot, which is not desirable for a few reasons:
-
It's hard to test. With so many options interacting with each other, every option variation needs to be tested with most of the variations of the other options.
-
Options have a performance cost regardless if they are being used. For example, the
loadOnReset
option utilizes aReact.useCallback
and aReact.useEffect
:graphql-react/src/universal/useGraphQL.js
Lines 203 to 229 in d87d3b3
/** * Handles the [`GraphQL`]{@link GraphQL} [`reset`]{@link GraphQL#event:reset} * event. * @param {GraphQL#event:reset} event Event data. * @ignore */ const onReset = React.useCallback( ({ exceptCacheKey }) => { if (cacheKey !== exceptCacheKey && isMountedRef.current) { ReactDOM.unstable_batchedUpdates(() => { setCacheValue(graphql.cache[cacheKey]); setLoadedCacheValue(graphql.cache[cacheKey]); }); if (loadOnReset) load(); } }, [cacheKey, graphql.cache, load, loadOnReset] ); React.useEffect(() => { graphql.on('reset', onReset); return () => { graphql.off('reset', onReset); }; }, [graphql, onReset]); -
The code supporting options increases the bundle size, regardless if the options are even being used.
-
If a consumer wants to subscribe to the
GraphQL
events for their own custom purposes usingReact.useEffect
(e.g. to do something in a callback if cache is reloaded while the component is mounted) they have to reinvent wheels that are already implemented insideuseGraphQL
.
I won't be sure of the final design for the hooks without a lot of work that might also depend on some big changes to the GraphQL
client API as well as the SSR API, but here is one idea:
const onGraphQLReset = React.useCallback(
({ exceptCacheKey }) => {
if (cacheKey !== exceptCacheKey) load();
},
[load]
);
useOnGraphQLReset(onGraphQLReset);