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
Prev Previous commit
Next Next commit
updateResult -> updateData
  • Loading branch information
jackdclark committed Feb 25, 2019
commit ab46beae25bed7c1dc90c4aa47c56a750a25e2a2
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,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
- `updateData(previousData, data)`: Function - Custom handler for merging previous & new query results; return value will replace `data` in `useQuery` return value
- `previousData`: Previous GraphQL query or `updateData` result
- `data`: New GraphQL query result

### `useQuery` return value

Expand Down Expand Up @@ -395,8 +395,8 @@ Here is an example where we append each paginated query to the bottom of the cur
import { React, useState } from 'react';
import { useQuery } from 'graphql-hooks';

// use options.updateResult to append the new page of posts to our current list of posts
const updateResult = (prevResult, result) => ({
// use options.updateData to append the new page of posts to our current list of posts
const updateData = (prevResult, result) => ({
...result,
allPosts: [...prevResult.allPosts, ...result.allPosts]
});
Expand All @@ -407,7 +407,7 @@ export default function PostList() {
const { loading, error, data } = useQuery(
allPostsQuery,
{ variables: { skip: skipCount, first: 10 } },
updateResult
updateData
);

if (error) return <div>There was an error!</div>;
Expand Down
8 changes: 4 additions & 4 deletions src/useClientRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ function useClientRequest(query, initialOpts = {}) {
dispatch({ type: actionTypes.LOADING });
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');
if (state.data && result.data && revisedOpts.updateData) {
if (typeof revisedOpts.updateData !== 'function') {
throw new Error('options.updateData must be a function');
}
result.data = revisedOpts.updateResult(state.data, result.data);
result.data = revisedOpts.updateData(state.data, result.data);
}

if (revisedOpts.useCache && client.cache) {
Expand Down
2 changes: 1 addition & 1 deletion src/useQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module.exports = function useQuery(query, opts = {}) {
refetch: (options = {}) =>
queryReq({
skipCache: true,
updateResult: (_, result) => result,
updateData: (_, data) => data,
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: We should add a comment here saying why we're doing this

Copy link
Contributor Author

@jackdclark jackdclark Feb 25, 2019

Choose a reason for hiding this comment

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

Agree, I've added an explanation & added a note to the README for refetch

...options
})
};
Expand Down
24 changes: 12 additions & 12 deletions test/unit/useClientRequest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,12 @@ describe('useClientRequest', () => {
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');
const updateDataMock = jest.fn().mockReturnValue('merged data');
testHook(
() =>
([fetchData, state] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: updateResultMock
updateData: updateDataMock
})),
{ wrapper: Wrapper }
);
Expand All @@ -235,7 +235,7 @@ describe('useClientRequest', () => {
mockClient.request.mockResolvedValueOnce({ data: 'new data' });
await fetchData({ variables: { limit: 20 } });

expect(updateResultMock).toHaveBeenCalledWith('data', 'new data');
expect(updateDataMock).toHaveBeenCalledWith('data', 'new data');
expect(state).toEqual({
cacheHit: false,
data: 'merged data',
Expand All @@ -245,29 +245,29 @@ describe('useClientRequest', () => {

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

await fetchData();

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

it('is not called if there is no new data', async () => {
let fetchData;
const updateResultMock = jest.fn();
const updateDataMock = jest.fn();
testHook(
() =>
([fetchData] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: updateResultMock
updateData: updateDataMock
})),
{ wrapper: Wrapper }
);
Expand All @@ -277,16 +277,16 @@ describe('useClientRequest', () => {
mockClient.request.mockReturnValueOnce({ errors: ['on no!'] });
await fetchData({ variables: { limit: 20 } });

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

it('throws if updateResult is not a function', async () => {
it('throws if updateData is not a function', async () => {
let fetchData;
testHook(
() =>
([fetchData] = useClientRequest(TEST_QUERY, {
variables: { limit: 10 },
updateResult: 'do I look like a function to you?'
updateData: 'do I look like a function to you?'
})),
{ wrapper: Wrapper }
);
Expand All @@ -295,7 +295,7 @@ describe('useClientRequest', () => {
await fetchData();

expect(fetchData({ variables: { limit: 20 } })).rejects.toThrow(
'options.updateResult must be a function'
'options.updateData 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
16 changes: 8 additions & 8 deletions test/unit/useQuery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('useQuery', () => {
refetch();
expect(mockQueryReq).toHaveBeenCalledWith({
skipCache: true,
updateResult: expect.any(Function)
updateData: expect.any(Function)
});
});

Expand All @@ -84,21 +84,21 @@ describe('useQuery', () => {
wrapper: Wrapper
}
);
const updateResult = () => {};
const updateData = () => {};
refetch({
extra: 'option',
variables: { skip: 10, first: 10, extra: 'variable' },
updateResult
updateData
});
expect(mockQueryReq).toHaveBeenCalledWith({
skipCache: true,
extra: 'option',
variables: { skip: 10, first: 10, extra: 'variable' },
updateResult
updateData
});
});

it('gets updateResult to replace the result by default', () => {
it('gets updateData to replace the result by default', () => {
let refetch;
testHook(
() =>
Expand All @@ -109,11 +109,11 @@ describe('useQuery', () => {
wrapper: Wrapper
}
);
mockQueryReq.mockImplementationOnce(({ updateResult }) => {
return updateResult('previousResult', 'result');
mockQueryReq.mockImplementationOnce(({ updateData }) => {
return updateData('previousData', 'data');
});
refetch();
expect(mockQueryReq).toHaveReturnedWith('result');
expect(mockQueryReq).toHaveReturnedWith('data');
});

it('sends the query on mount if no data & no error', () => {
Expand Down