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

Update refresh strategy to avoid empty page while refreshing #6054

Merged
merged 2 commits into from
Mar 26, 2021
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
12 changes: 12 additions & 0 deletions packages/ra-core/src/actions/uiActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const setSidebarVisibility = (
payload: isOpen,
});

// refresh increases version (i.e. forces refetch) and empties the cache
export const REFRESH_VIEW = 'RA/REFRESH_VIEW';

export interface RefreshViewAction {
Expand All @@ -32,6 +33,17 @@ export const refreshView = (): RefreshViewAction => ({
type: REFRESH_VIEW,
});

// soft refresh only increases version (i.e. forces refetch)
export const SOFT_REFRESH_VIEW = 'RA/SOFT_REFRESH_VIEW';

export interface SoftRefreshViewAction {
readonly type: typeof SOFT_REFRESH_VIEW;
}

export const softRefreshView = (): SoftRefreshViewAction => ({
type: SOFT_REFRESH_VIEW,
});

export const SET_AUTOMATIC_REFRESH = 'RA/SET_AUTOMATIC_REFRESH';

export interface SetAutomaticRefreshAction {
Expand Down
10 changes: 7 additions & 3 deletions packages/ra-core/src/dataProvider/Mutation.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { fireEvent, waitFor, act, render } from '@testing-library/react';
import expect from 'expect';

import Mutation from './Mutation';
import { showNotification, refreshView, setListSelectedIds } from '../actions';
import {
showNotification,
softRefreshView,
setListSelectedIds,
} from '../actions';
import DataProviderContext from './DataProviderContext';
import { renderWithRedux, TestContext } from 'ra-test';
import { useNotify } from '../sideEffect';
Expand Down Expand Up @@ -100,7 +104,7 @@ describe('Mutation', () => {
})
);
expect(historyForAssertions.location.pathname).toEqual('/a_path');
expect(dispatchSpy).toHaveBeenCalledWith(refreshView());
expect(dispatchSpy).toHaveBeenCalledWith(softRefreshView());
expect(dispatchSpy).toHaveBeenCalledWith(
setListSelectedIds('foo', [])
);
Expand Down Expand Up @@ -222,7 +226,7 @@ describe('Mutation', () => {
})
);
expect(historyForAssertions.location.pathname).toEqual('/a_path');
expect(dispatchSpy).toHaveBeenCalledWith(refreshView());
expect(dispatchSpy).toHaveBeenCalledWith(softRefreshView());
expect(dispatchSpy).toHaveBeenCalledWith(
setListSelectedIds('foo', [])
);
Expand Down
10 changes: 7 additions & 3 deletions packages/ra-core/src/dataProvider/Query.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import Query from './Query';
import { CoreAdmin, Resource } from '../core';
import { renderWithRedux, TestContext } from 'ra-test';
import DataProviderContext from './DataProviderContext';
import { showNotification, refreshView, setListSelectedIds } from '../actions';
import {
showNotification,
softRefreshView,
setListSelectedIds,
} from '../actions';
import { useNotify, useRefresh } from '../sideEffect';
import { History } from 'history';

Expand Down Expand Up @@ -311,7 +315,7 @@ describe('Query', () => {
})
);
expect(historyForAssertions.location.pathname).toEqual('/a_path');
expect(dispatchSpy).toHaveBeenCalledWith(refreshView());
expect(dispatchSpy).toHaveBeenCalledWith(softRefreshView());
expect(dispatchSpy).toHaveBeenCalledWith(
setListSelectedIds('foo', [])
);
Expand Down Expand Up @@ -432,7 +436,7 @@ describe('Query', () => {
})
);
expect(historyForAssertions.location.pathname).toEqual('/a_path');
expect(dispatchSpy).toHaveBeenCalledWith(refreshView());
expect(dispatchSpy).toHaveBeenCalledWith(softRefreshView());
expect(dispatchSpy).toHaveBeenCalledWith(
setListSelectedIds('foo', [])
);
Expand Down
36 changes: 36 additions & 0 deletions packages/ra-core/src/dataProvider/useDataProvider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,42 @@ describe('useDataProvider', () => {
expect(getOne).toBeCalledTimes(2);
});

it('should not use the cache after a hard refresh', async () => {
const getOne = jest.fn(() => {
const validUntil = new Date();
validUntil.setTime(validUntil.getTime() + 1000);
return Promise.resolve({ data: { id: 1 }, validUntil });
});
const dataProvider = { getOne };
const Refresh = () => {
const refresh = useRefresh();
return <button onClick={() => refresh(true)}>refresh</button>;
};
const { getByText, rerender } = renderWithRedux(
<DataProviderContext.Provider value={dataProvider}>
<UseGetOne key="1" />
<Refresh />
</DataProviderContext.Provider>,
{ admin: { resources: { posts: { data: {}, list: {} } } } }
);
// waitFor for the dataProvider to return
await act(async () => await new Promise(r => setTimeout(r)));
// click on the refresh button
expect(getOne).toBeCalledTimes(1);
await act(async () => {
fireEvent.click(getByText('refresh'));
await new Promise(r => setTimeout(r));
});
rerender(
<DataProviderContext.Provider value={dataProvider}>
<UseGetOne key="2" />
</DataProviderContext.Provider>
);
// waitFor for the dataProvider to return
await act(async () => await new Promise(r => setTimeout(r)));
expect(getOne).toBeCalledTimes(2);
});

it('should not use the cache after an update', async () => {
const getOne = jest.fn(() => {
const validUntil = new Date();
Expand Down
5 changes: 5 additions & 0 deletions packages/ra-core/src/reducer/admin/resource/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
UnregisterResourceAction,
REFRESH_VIEW,
RefreshViewAction,
SOFT_REFRESH_VIEW,
SoftRefreshViewAction,
} from '../../../actions';

import data from './data';
Expand All @@ -17,6 +19,7 @@ type ActionTypes =
| RegisterResourceAction
| UnregisterResourceAction
| RefreshViewAction
| SoftRefreshViewAction
| { type: 'OTHER_ACTION'; payload?: any; meta?: { resource?: string } };

export default (previousState = initialState, action: ActionTypes) => {
Expand Down Expand Up @@ -45,6 +48,7 @@ export default (previousState = initialState, action: ActionTypes) => {

if (
action.type !== REFRESH_VIEW &&
action.type !== SOFT_REFRESH_VIEW &&
(!action.meta || !action.meta.resource)
) {
return previousState;
Expand All @@ -56,6 +60,7 @@ export default (previousState = initialState, action: ActionTypes) => {
...acc,
[resource]:
action.type === REFRESH_VIEW ||
action.type === SOFT_REFRESH_VIEW ||
action.meta.resource === resource
? {
props: previousState[resource].props,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { Reducer } from 'redux';

import { Identifier } from '../../../../types';
import { FETCH_END, REFRESH_VIEW } from '../../../../actions';
import {
FETCH_END,
REFRESH_VIEW,
SOFT_REFRESH_VIEW,
} from '../../../../actions';
import {
GET_LIST,
CREATE,
Expand Down Expand Up @@ -35,6 +39,17 @@ const cachedRequestsReducer: Reducer<State> = (
// force refresh
return initialState;
}
if (action.type === SOFT_REFRESH_VIEW) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key difference: a soft_refresh doesn't empty the cached requests, only invalidates them

// remove validity only
const newState = {};
Object.keys(previousState).forEach(key => {
newState[key] = {
...previousState[key],
validity: undefined,
};
});
return newState;
}
if (action.meta && action.meta.optimistic) {
if (
action.meta.fetch === CREATE ||
Expand Down
8 changes: 6 additions & 2 deletions packages/ra-core/src/reducer/admin/resource/list/validity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { Reducer } from 'redux';
import { FETCH_END, REFRESH_VIEW } from '../../../../actions';
import {
FETCH_END,
REFRESH_VIEW,
SOFT_REFRESH_VIEW,
} from '../../../../actions';
import { GET_LIST, CREATE } from '../../../../core';

interface ValidityRegistry {
Expand All @@ -12,7 +16,7 @@ const validityReducer: Reducer<ValidityRegistry> = (
previousState = initialState,
{ type, payload, requestPayload, meta }
) => {
if (type === REFRESH_VIEW) {
if (type === REFRESH_VIEW || type === SOFT_REFRESH_VIEW) {
return initialState;
}
if (
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-core/src/reducer/admin/resource/validity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Reducer } from 'redux';
import { FETCH_END, REFRESH_VIEW } from '../../../actions';
import { FETCH_END, REFRESH_VIEW, SOFT_REFRESH_VIEW } from '../../../actions';
import {
CREATE,
DELETE,
Expand All @@ -25,7 +25,7 @@ const validityReducer: Reducer<ValidityRegistry> = (
previousState = initialState,
{ type, payload, requestPayload, meta }
) => {
if (type === REFRESH_VIEW) {
if (type === REFRESH_VIEW || type === SOFT_REFRESH_VIEW) {
return initialState;
}
if (
Expand Down
4 changes: 4 additions & 0 deletions packages/ra-core/src/reducer/admin/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
SetSidebarVisibilityAction,
REFRESH_VIEW,
RefreshViewAction,
SOFT_REFRESH_VIEW,
SoftRefreshViewAction,
START_OPTIMISTIC_MODE,
StartOptimisticModeAction,
STOP_OPTIMISTIC_MODE,
Expand All @@ -20,6 +22,7 @@ type ActionTypes =
| ToggleSidebarAction
| SetSidebarVisibilityAction
| RefreshViewAction
| SoftRefreshViewAction
| StartOptimisticModeAction
| StopOptimisticModeAction
| SetAutomaticRefreshAction
Expand Down Expand Up @@ -67,6 +70,7 @@ const uiReducer: Reducer<UIState> = (
automaticRefreshEnabled: action.payload,
};
case REFRESH_VIEW:
case SOFT_REFRESH_VIEW:
return {
...previousState,
viewVersion: previousState.viewVersion + 1,
Expand Down
16 changes: 14 additions & 2 deletions packages/ra-core/src/sideEffect/useRefresh.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
import { useCallback } from 'react';
import { useDispatch } from 'react-redux';
import { refreshView } from '../actions/uiActions';
import { refreshView, softRefreshView } from '../actions/uiActions';

/**
* Hook for Refresh Side Effect
*
* Returns a callback that triggers a page refresh. The callback causes a
* version increase, which forces a re-execution all queries based on the
* useDataProvider() hook, and a rerender of all components using the version
* as key.
*
* @param hard If true, the callback empties the cache, too
*
* @example
*
* const refresh = useRefresh();
* // soft refresh
* refresh();
* // hard refresh
* refresh(true)
*/
const useRefresh = () => {
const dispatch = useDispatch();
return useCallback(
(doRefresh = true) => doRefresh && dispatch(refreshView()),
Copy link
Member Author

Choose a reason for hiding this comment

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

this doRefresh parameter was undocumented and unused. Also, it's useless - one can replace it easily:

-refresh(doRefresh);
+doRefresh && refresh()

(hard?: boolean) => {
dispatch(hard ? refreshView() : softRefreshView());
Copy link
Member Author

Choose a reason for hiding this comment

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

I wondered if there should be two different actions or one action with a variable payload. I'm open to discussion on this point

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a single action is enough, with a type option which can be either hard or soft (for now)

},
[dispatch]
);
};
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/button/RefreshButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { FC, ReactElement, MouseEvent, useCallback } from 'react';
import PropTypes from 'prop-types';
import { useDispatch } from 'react-redux';
import NavigationRefresh from '@material-ui/icons/Refresh';
import { refreshView } from 'ra-core';
import { softRefreshView } from 'ra-core';

import Button, { ButtonProps } from './Button';

Expand All @@ -17,7 +17,7 @@ const RefreshButton: FC<RefreshButtonProps> = ({
const handleClick = useCallback(
event => {
event.preventDefault();
dispatch(refreshView());
dispatch(softRefreshView());
if (typeof onClick === 'function') {
onClick(event);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/button/RefreshIconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useDispatch } from 'react-redux';
import Tooltip from '@material-ui/core/Tooltip';
import IconButton, { IconButtonProps } from '@material-ui/core/IconButton';
import NavigationRefresh from '@material-ui/icons/Refresh';
import { refreshView, useTranslate } from 'ra-core';
import { softRefreshView, useTranslate } from 'ra-core';

const RefreshIconButton: FC<RefreshIconProps> = ({
label = 'ra.action.refresh',
Expand All @@ -19,7 +19,7 @@ const RefreshIconButton: FC<RefreshIconProps> = ({
const handleClick = useCallback(
event => {
event.preventDefault();
dispatch(refreshView());
dispatch(softRefreshView());
if (typeof onClick === 'function') {
onClick(event);
}
Expand Down