Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ This is a custom hook that takes care of fetching your query and storing the res
- `skipCache`: Boolean - defaults to `false`; If `true` it will by-pass the cache and fetch, but the result will then be cached for subsequent calls. Note the `refetch` function will do this automatically
- `ssr`: Boolean - defaults to `true`. Set to `false` if you wish to skip this query during SSR
- `fetchOptionsOverrides`: Object - Specific overrides for this query. See [MDN](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch) for info on what options can be passed
- `updateResult(previousResult, result)`: Function - Custom handler for merging previous & new query results; return value will replace `data` in `useQuery` return value
- `previousResult`: Previous GraphQL query or `updateResult` result
- `result`: New GraphQL query result

### `useQuery` return value

Expand All @@ -198,6 +201,8 @@ const { loading, error, data, refetch, cacheHit, ...errors } = useQuery(QUERY);
- `error`: Boolean - `true` if `fetchError` or `httpError` or `graphQLErrors` has been set
- `data`: Object - the result of your GraphQL query
- `refetch`: Function - useful when refetching the same query after a mutation; NOTE this presets `skipCache=true`
- `fetchMore(options)`: Function - send the same query with updated options, useful for pagination.
- `options`: Object - options that will be merged into the `options` that were passed into `useQuery` (see above).
- `cacheHit`: Boolean - `true` if the query result came from the cache, useful for debugging
- `fetchError`: Object - Set if an error occured during the `fetch` call
- `httpError`: Object - Set if an error response was returned from the server
Expand Down
9 changes: 8 additions & 1 deletion src/useClientRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,14 @@ function useClientRequest(query, initialOpts = {}) {
}

dispatch({ type: actionTypes.LOADING });
const result = await client.request(revisedOperation, revisedOpts);
let result = await client.request(revisedOperation, revisedOpts);

if (state.data && result.data && revisedOpts.updateResult) {
if (typeof revisedOpts.updateResult !== 'function') {
throw new Error('options.updateResult must be a function');
}
result.data = revisedOpts.updateResult(state.data, result.data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the name is updateResult, maybe we should pass the entire result? It might be useful for a developer to change / override the errors in some circumstances.

useClientRequest would then look something like:

let result = await client.request(revisedOperation, revisedOpts);

if (revisedOpts.updateResult) {
  if (typeof revisedOpts.updateResult !== 'function') {
    throw new Error('options.updateResult must be a function');
  }
  result = revisedOpts.updateResult(state, result)
}

The pagination example updateResult:

const updateResult = (prevResult, newResult) => ({
  ...newResult,
  data: {
    ...newResult.data,
    allPosts: [...prevResult.data.allPosts, ...result.data.allPosts]
  }
});

useQuery(MY_QUERY, {
  updateResult
})

It makes updating the data a bit more verbose, but the developer could write a wrapper if it was a really common usecase:

const dataUpdater = (fn) => (prevResult, newResult) => ({
  ...newResult,
  data: fn(prevResult.data, newResult.data)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, passing the entire result back allows internal properties like loading & error to be changed, or new ones to be added which could be abused or cause tricky bugs. We're going to rename this to updateData as a better description of what it's doing & if we see a use case for changing / overriding errors in the future, we could always introduce an updateErrors option.

}

if (revisedOpts.useCache && client.cache) {
client.cache.set(revisedCacheKey, result);
Expand Down
12 changes: 7 additions & 5 deletions src/useQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ const defaultOpts = {
};

module.exports = function useQuery(query, opts = {}) {
const allOpts = { ...defaultOpts, ...opts };
const client = React.useContext(ClientContext);
const [calledDuringSSR, setCalledDuringSSR] = React.useState(false);
const [queryReq, state] = useClientRequest(query, {
...defaultOpts,
...opts
});
const [queryReq, state] = useClientRequest(query, allOpts);

if (client.ssrMode && opts.ssr !== false && !calledDuringSSR) {
// result may already be in the cache from previous SSR iterations
Expand All @@ -30,6 +28,10 @@ module.exports = function useQuery(query, opts = {}) {

return {
...state,
refetch: () => queryReq({ skipCache: true })
refetch: () => queryReq({ skipCache: true }),
fetchMore: ({ variables, ...rest } = {}) =>
// merge variables so they don't all have to be passed back in,
// just the ones that are changing
queryReq({ ...rest, variables: { ...allOpts.variables, ...variables } })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

};
};
94 changes: 89 additions & 5 deletions test/unit/useClientRequest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('useClientRequest', () => {
get: jest.fn(),
set: jest.fn()
},
request: jest.fn().mockResolvedValue({ some: 'data' })
request: jest.fn().mockResolvedValue({ data: 'data' })
};
});

Expand Down Expand Up @@ -127,7 +127,7 @@ describe('useClientRequest', () => {
{ operationName: 'test', variables: { limit: 2 }, query: TEST_QUERY },
{ operationName: 'test', variables: { limit: 2 } }
);
expect(state).toEqual({ cacheHit: false, loading: false, some: 'data' });
expect(state).toEqual({ cacheHit: false, loading: false, data: 'data' });
});

it('calls request with revised options', async () => {
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('useClientRequest', () => {
expect(state).toEqual({
cacheHit: false,
loading: false,
some: 'data'
data: 'data'
});
});

Expand All @@ -198,7 +198,7 @@ describe('useClientRequest', () => {
expect(state).toEqual({
cacheHit: false,
loading: false,
some: 'data'
data: 'data'
});
});

Expand All @@ -212,7 +212,91 @@ describe('useClientRequest', () => {
await fetchData();

expect(mockClient.cache.set).toHaveBeenCalledWith('cacheKey', {
some: 'data'
data: 'data'
});
});

describe('options.updateRequest', () => {
it('is called with old & new data if the data has changed & the result is returned', async () => {
let fetchData, state;
const updateResultMock = jest.fn().mockReturnValue('merged data');
testHook(
() =>
([fetchData, state] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: updateResultMock
})),
{ wrapper: Wrapper }
);

// first fetch to populate state
await fetchData();

mockClient.request.mockResolvedValueOnce({ data: 'new data' });
await fetchData({ variables: { limit: 20 } });

expect(updateResultMock).toHaveBeenCalledWith('data', 'new data');
expect(state).toEqual({
cacheHit: false,
data: 'merged data',
loading: false
});
});

it('is not called if there is no old data', async () => {
let fetchData;
const updateResultMock = jest.fn();
testHook(
() =>
([fetchData] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: updateResultMock
})),
{ wrapper: Wrapper }
);

await fetchData();

expect(updateResultMock).not.toHaveBeenCalled();
});

it('is not called if there is no new data', async () => {
let fetchData;
const updateResultMock = jest.fn();
testHook(
() =>
([fetchData] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: updateResultMock
})),
{ wrapper: Wrapper }
);

await fetchData();

mockClient.request.mockReturnValueOnce({ errors: ['on no!'] });
await fetchData({ variables: { limit: 20 } });

expect(updateResultMock).not.toHaveBeenCalled();
});

it('throws if updateResult is not a function', async () => {
let fetchData;
testHook(
() =>
([fetchData] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: 'do I look like a function to you?'
})),
{ wrapper: Wrapper }
);

// first fetch to populate state
await fetchData();

expect(fetchData({ variables: { limit: 20 } })).rejects.toThrow(
'options.updateResult must be a function'
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 excellent tests

});
});
});
Expand Down
40 changes: 38 additions & 2 deletions test/unit/useQuery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ describe('useQuery', () => {
});
});

it('returns initial state from useClientRequest & refetch', () => {
it('returns initial state from useClientRequest, refetch & fetchMore', () => {
let state;
testHook(() => (state = useQuery(TEST_QUERY)), { wrapper: Wrapper });
expect(state).toEqual({
loading: true,
cacheHit: false,
refetch: expect.any(Function)
refetch: expect.any(Function),
fetchMore: expect.any(Function)
});
});

Expand All @@ -70,6 +71,41 @@ describe('useQuery', () => {
expect(mockQueryReq).toHaveBeenCalledWith({ skipCache: true });
});

it('sends a query with original options when fetchMore is called', () => {
let fetchMore;
testHook(
() =>
({ fetchMore } = useQuery(TEST_QUERY, {
variables: { skip: 0, first: 10 }
})),
{
wrapper: Wrapper
}
);
fetchMore();
expect(mockQueryReq).toHaveBeenCalledWith({
variables: { skip: 0, first: 10 }
});
});

it('passes options & merges variables when fetchMore is called', () => {
let fetchMore;
testHook(
() =>
({ fetchMore } = useQuery(TEST_QUERY, {
variables: { skip: 0, first: 10 }
})),
{
wrapper: Wrapper
}
);
fetchMore({ extra: 'option', variables: { skip: 10, extra: 'variable' } });
expect(mockQueryReq).toHaveBeenCalledWith({
extra: 'option',
variables: { skip: 10, first: 10, extra: 'variable' }
});
});

it('sends the query on mount if no data & no error', () => {
testHook(() => useQuery(TEST_QUERY), { wrapper: Wrapper });
expect(mockQueryReq).toHaveBeenCalledTimes(1);
Expand Down