Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Don't update state after a request responds if the component has unmo…
…unted.
  • Loading branch information
Joezo committed Mar 6, 2019
commit d887ae764b68594756959ca0ddab2ae2980f708a
25 changes: 19 additions & 6 deletions packages/graphql-hooks/src/useClientRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function reducer(state, action) {
*/
function useClientRequest(query, initialOpts = {}) {
const client = React.useContext(ClientContext)
const isMounted = React.useRef(true)
const operation = {
query,
variables: initialOpts.variables,
Expand All @@ -73,12 +74,22 @@ function useClientRequest(query, initialOpts = {}) {
// if so the state would be invalid, this effect ensures we reset it back
const stringifiedCacheKey = JSON.stringify(cacheKey)
React.useEffect(() => {
if (initialOpts.updateData) return // if using updateData we can assume that the consumer cares about the previous data
dispatch({ type: actionTypes.RESET_STATE, initialState })
if (!initialOpts.updateData) {
// if using updateData we can assume that the consumer cares about the previous data
dispatch({ type: actionTypes.RESET_STATE, initialState })
}
}, [stringifiedCacheKey]) // eslint-disable-line react-hooks/exhaustive-deps

React.useEffect(() => {
isMounted.current = true
return () => {
isMounted.current = false
}
}, [])

// arguments to fetchData override the useClientRequest arguments
function fetchData(newOpts) {
if (!isMounted.current) return Promise.resolve()
const revisedOpts = {
...initialOpts,
...newOpts
Expand Down Expand Up @@ -118,10 +129,12 @@ function useClientRequest(query, initialOpts = {}) {
client.cache.set(revisedCacheKey, result)
}

dispatch({
type: actionTypes.REQUEST_RESULT,
result
})
if (isMounted.current) {
dispatch({
type: actionTypes.REQUEST_RESULT,
result
})
}

return result
})
Expand Down
24 changes: 24 additions & 0 deletions packages/graphql-hooks/test/unit/useClientRequest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,30 @@ describe('useClientRequest', () => {
expect(state).toEqual({ cacheHit: false, loading: false, data: 'data' })
})

it('calls request with options & doesnt update state if component unmounts whilst request is in flight', async () => {
let fetchData, state;
const { unmount } = renderHook(
() =>
([fetchData, state] = useClientRequest(TEST_QUERY, {
variables: { limit: 2 },
operationName: 'test'
})),
{
wrapper: Wrapper
}
);

const fetchDataPromise = fetchData();
unmount();
await fetchDataPromise;

expect(mockClient.request).toHaveBeenCalledWith(
{ operationName: 'test', variables: { limit: 2 }, query: TEST_QUERY },
{ operationName: 'test', variables: { limit: 2 } }
);
expect(state).toEqual({ cacheHit: false, loading: true });
});

it('calls request with revised options', async () => {
let fetchData
renderHook(
Expand Down