Skip to content
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

Add ability to refetch a query in useQuery and useQueryWithStore #6130

Merged
merged 8 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions docs/Actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ The return value of `useQuery` is an object representing the query state, using
- `error`: `null` unless the `dataProvider` threw an error, in which case it contains that error.
- `loading`: A boolean updating according to the request state
- `loaded`: A boolean updating according to the request state
- `refetch`: A function you can call to trigger a refetch. It's different than the `refresh` function returned by `useRefresh` as it won't trigger a refresh of the view, only this specific query.

This object updates according to the request state:

Expand Down Expand Up @@ -175,6 +176,8 @@ const UserProfile = ({ record }) => {

In practice, react-admin uses `useQueryWithStore` instead of `useQuery` everywhere, and you should probably do the same in your components. It really improves the User Experience, with only one little drawback: if the data changed on the backend side between two calls for the same query, the user may briefly see outdated data before the screen updates with the up-to-date data.

Just like `useQuery`, `useQueryWithStore` also returns a `¶efetch` function you can call to trigger a refetch. It's different than the `refresh` function returned by `useRefresh` as it won't trigger a refresh of the view, only this specific query.

## `useMutation` Hook

`useQuery` emits the request to the `dataProvider` as soon as the component mounts. To emit the request based on a user action, use the `useMutation` hook instead. This hook takes the same arguments as `useQuery`, but returns a callback that emits the request when executed.
Expand Down
15 changes: 14 additions & 1 deletion packages/ra-core/src/dataProvider/useQuery.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect } from 'react';
import { useCallback, useEffect, useState } from 'react';

import { useSafeSetState } from '../util/hooks';
import { OnSuccess, OnFailure } from '../types';
Expand Down Expand Up @@ -77,17 +77,27 @@ const useQuery = (
const { type, resource, payload } = query;
const { withDeclarativeSideEffectsSupport, ...otherOptions } = options;
const version = useVersion(); // used to allow force reload
// used to force a refetch without relying on version
// which might trigger other queries as well
const [innerVersion, setInnerVersion] = useState(0);

const refetch = useCallback(() => {
setInnerVersion(prevInnerVersion => prevInnerVersion + 1);
}, []);

const requestSignature = JSON.stringify({
query,
options: otherOptions,
version,
innerVersion,
});
const [state, setState] = useSafeSetState<UseQueryValue>({
data: undefined,
error: null,
total: null,
loading: true,
loaded: false,
refetch,
});
const dataProvider = useDataProvider();
const dataProviderWithDeclarativeSideEffects = useDataProviderWithDeclarativeSideEffects();
Expand Down Expand Up @@ -118,13 +128,15 @@ const useQuery = (
total,
loading: false,
loaded: true,
refetch,
});
})
.catch(error => {
setState({
error,
loading: false,
loaded: false,
refetch,
});
});
}, [
Expand Down Expand Up @@ -158,6 +170,7 @@ export type UseQueryValue = {
error?: any;
loading: boolean;
loaded: boolean;
refetch: () => void;
};

export default useQuery;
130 changes: 88 additions & 42 deletions packages/ra-core/src/dataProvider/useQueryWithStore.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { waitFor } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';
import expect from 'expect';

import { renderWithRedux } from 'ra-test';
Expand All @@ -21,7 +21,12 @@ const UseQueryWithStore = ({
totalSelector
);
if (callback) callback(hookValue);
return <div>hello</div>;
return (
<>
<div>hello</div>
<button onClick={() => hookValue.refetch()}>refetch</button>
</>
);
};

describe('useQueryWithStore', () => {
Expand All @@ -40,22 +45,23 @@ describe('useQueryWithStore', () => {
</DataProviderContext.Provider>,
{ admin: { resources: { posts: { data: {} } } } }
);
expect(callback).toBeCalledWith({
data: undefined,
loading: true,
loaded: false,
error: null,
total: null,
});
let callArgs = callback.mock.calls[0][0];
expect(callArgs.data).toBeUndefined();
expect(callArgs.loading).toEqual(true);
expect(callArgs.loaded).toEqual(false);
expect(callArgs.error).toBeNull();
expect(callArgs.total).toBeNull();
callback.mockClear();
await new Promise(resolve => setImmediate(resolve)); // dataProvider Promise returns result on next tick
expect(callback).toBeCalledWith({
data: { id: 1, title: 'titleFromDataProvider' },
loading: false,
loaded: true,
error: null,
total: null,
callArgs = callback.mock.calls[1][0];
expect(callArgs.data).toEqual({
id: 1,
title: 'titleFromDataProvider',
});
expect(callArgs.loading).toEqual(false);
expect(callArgs.loaded).toEqual(true);
expect(callArgs.error).toBeNull();
expect(callArgs.total).toBeNull();
});

it('should return data from the store first, then data from dataProvider', async () => {
Expand Down Expand Up @@ -90,26 +96,27 @@ describe('useQueryWithStore', () => {
},
}
);
expect(callback).toBeCalledWith({
data: { id: 2, title: 'titleFromReduxStore' },
loading: true,
loaded: true,
error: null,
total: null,
});
let callArgs = callback.mock.calls[0][0];
expect(callArgs.data).toEqual({ id: 2, title: 'titleFromReduxStore' });
expect(callArgs.loading).toEqual(true);
expect(callArgs.loaded).toEqual(true);
expect(callArgs.error).toBeNull();
expect(callArgs.total).toBeNull();
callback.mockClear();
await waitFor(() => {
expect(dataProvider.getOne).toHaveBeenCalled();
});
// dataProvider Promise returns result on next tick
await waitFor(() => {
expect(callback).toBeCalledWith({
data: { id: 2, title: 'titleFromDataProvider' },
loading: false,
loaded: true,
error: null,
total: null,
callArgs = callback.mock.calls[1][0];
expect(callArgs.data).toEqual({
id: 2,
title: 'titleFromDataProvider',
});
expect(callArgs.loading).toEqual(false);
expect(callArgs.loaded).toEqual(true);
expect(callArgs.error).toBeNull();
expect(callArgs.total).toBeNull();
});
});

Expand All @@ -129,22 +136,22 @@ describe('useQueryWithStore', () => {
</DataProviderContext.Provider>,
{ admin: { resources: { posts: { data: {} } } } }
);
expect(callback).toBeCalledWith({
data: undefined,
loading: true,
loaded: false,
error: null,
total: null,
});
let callArgs = callback.mock.calls[0][0];
expect(callArgs.data).toBeUndefined();
expect(callArgs.loading).toEqual(true);
expect(callArgs.loaded).toEqual(false);
expect(callArgs.error).toBeNull();
expect(callArgs.total).toBeNull();
callback.mockClear();
await new Promise(resolve => setImmediate(resolve)); // dataProvider Promise returns result on next tick
expect(callback).toBeCalledWith({
data: undefined,
loading: false,
loaded: false,
error: { message: 'error' },
total: null,
await waitFor(() => {
expect(dataProvider.getOne).toHaveBeenCalled();
});
callArgs = callback.mock.calls[0][0];
expect(callArgs.data).toBeUndefined();
expect(callArgs.loading).toEqual(false);
expect(callArgs.loaded).toEqual(false);
expect(callArgs.error).toEqual({ message: 'error' });
expect(callArgs.total).toBeNull();
});

it('should refetch the dataProvider on refresh', async () => {
Expand Down Expand Up @@ -186,6 +193,45 @@ describe('useQueryWithStore', () => {
});
});

it('should refetch the dataProvider when refetch is called', async () => {
const dataProvider = {
getOne: jest.fn(() =>
Promise.resolve({
data: { id: 3, title: 'titleFromDataProvider' },
})
),
};
const { getByText } = renderWithRedux(
<DataProviderContext.Provider value={dataProvider}>
<UseQueryWithStore
query={{
type: 'getOne',
resource: 'posts',
payload: { id: 3 },
}}
/>
</DataProviderContext.Provider>,
{
admin: {
resources: {
posts: {
data: {
3: { id: 3, title: 'titleFromReduxStore' },
},
},
},
},
}
);
await waitFor(() => {
expect(dataProvider.getOne).toBeCalledTimes(1);
});
fireEvent.click(getByText('refetch'));
await waitFor(() => {
expect(dataProvider.getOne).toBeCalledTimes(2);
});
});

it('should call the dataProvider twice for different requests in the same tick', async () => {
const dataProvider = {
getOne: jest.fn(() =>
Expand Down
29 changes: 20 additions & 9 deletions packages/ra-core/src/dataProvider/useQueryWithStore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { useSelector } from 'react-redux';
import isEqual from 'lodash/isEqual';

Expand All @@ -20,6 +20,7 @@ export interface StateResult {
error?: any;
loading: boolean;
loaded: boolean;
refetch: () => void;
}

export interface QueryOptions {
Expand Down Expand Up @@ -114,19 +115,26 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
dataSelector: (state: State) => any = defaultDataSelector(query),
totalSelector: (state: State) => number = defaultTotalSelector(query),
isDataLoaded: (data: any) => boolean = defaultIsDataLoaded
): {
data?: any;
total?: number;
error?: any;
loading: boolean;
loaded: boolean;
} => {
): StateResult => {
Copy link
Member

Choose a reason for hiding this comment

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

rename to UseQueryWithStoreValue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be a breaking change, right?

Copy link
Member

Choose a reason for hiding this comment

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

Why? The type wasn't named or exported before this PR.

const { type, resource, payload } = query;
const version = useVersion(); // used to allow force reload
const requestSignature = JSON.stringify({ query, options, version });
// used to force a refetch without relying on version
// which might trigger other queries as well
const [innerVersion, setInnerVersion] = useState(0);
const requestSignature = JSON.stringify({
query,
options,
version,
innerVersion,
});
const requestSignatureRef = useRef(requestSignature);
const data = useSelector(dataSelector);
const total = useSelector(totalSelector);

const refetch = useCallback(() => {
setInnerVersion(prevInnerVersion => prevInnerVersion + 1);
}, []);

const [state, setState]: [
StateResult,
(StateResult) => void
Expand All @@ -136,6 +144,7 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
error: null,
loading: true,
loaded: isDataLoaded(data),
refetch,
});

useEffect(() => {
Expand All @@ -148,6 +157,7 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
error: null,
loading: true,
loaded: isDataLoaded(data),
refetch,
});
} else if (!isEqual(state.data, data) || state.total !== total) {
// the dataProvider response arrived in the Redux store
Expand All @@ -173,6 +183,7 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
state.total,
total,
isDataLoaded,
refetch,
]);

const dataProvider = useDataProvider();
Expand Down