-
Notifications
You must be signed in to change notification settings - Fork 91
Pagination #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pagination #44
Changes from 9 commits
a04f137
67f87ee
b4298d6
7c61910
eb5116f
6fa845a
6a020ca
46317a4
f222040
ab46bea
03ec998
c51efb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
||
| } | ||
|
|
||
| if (revisedOpts.useCache && client.cache) { | ||
| client.cache.set(revisedCacheKey, result); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ describe('useClientRequest', () => { | |
| get: jest.fn(), | ||
| set: jest.fn() | ||
| }, | ||
| request: jest.fn().mockResolvedValue({ some: 'data' }) | ||
| request: jest.fn().mockResolvedValue({ data: 'data' }) | ||
| }; | ||
| }); | ||
|
|
||
|
|
@@ -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 () => { | ||
|
|
@@ -181,7 +181,7 @@ describe('useClientRequest', () => { | |
| expect(state).toEqual({ | ||
| cacheHit: false, | ||
| loading: false, | ||
| some: 'data' | ||
| data: 'data' | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -198,7 +198,7 @@ describe('useClientRequest', () => { | |
| expect(state).toEqual({ | ||
| cacheHit: false, | ||
| loading: false, | ||
| some: 'data' | ||
| data: 'data' | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -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' | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 excellent tests |
||
| }); | ||
| }); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should simplify the example - this is could be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the separation between data & meta is good here? There are redundant parts to this example that could be removed & make it more readable though, how about: