From 7f0d28201ac951f241b53c799b975c6826adc1cd Mon Sep 17 00:00:00 2001 From: fzaninotto Date: Sun, 19 May 2019 23:22:43 +0200 Subject: [PATCH 1/2] add useGetOne hook --- .../ra-core/src/controller/EditController.tsx | 19 +- .../ra-core/src/controller/ShowController.tsx | 24 +-- packages/ra-core/src/controller/useGetOne.ts | 66 +++++++ packages/ra-core/src/reducer/admin/index.ts | 2 + .../src/reducer/admin/requests.spec.ts | 163 ++++++++++++++++++ .../ra-core/src/reducer/admin/requests.ts | 58 +++++++ packages/ra-core/src/sideEffect/fetch.ts | 10 +- packages/ra-core/src/types.ts | 7 + .../ra-core/src/util/TestContext.spec.tsx | 1 + 9 files changed, 318 insertions(+), 32 deletions(-) create mode 100644 packages/ra-core/src/controller/useGetOne.ts create mode 100644 packages/ra-core/src/reducer/admin/requests.spec.ts create mode 100644 packages/ra-core/src/reducer/admin/requests.ts diff --git a/packages/ra-core/src/controller/EditController.tsx b/packages/ra-core/src/controller/EditController.tsx index 3db0896031b..88971f691ff 100644 --- a/packages/ra-core/src/controller/EditController.tsx +++ b/packages/ra-core/src/controller/EditController.tsx @@ -3,12 +3,14 @@ import { ReactNode, useEffect, useCallback } from 'react'; import { useSelector, useDispatch } from 'react-redux'; import { reset as resetForm } from 'redux-form'; import inflection from 'inflection'; -import { crudGetOne, crudUpdate, startUndoable } from '../actions'; + +import { crudUpdate, startUndoable } from '../actions'; import { REDUX_FORM_NAME } from '../form'; import { useCheckMinimumRequiredProps } from './checkMinimumRequiredProps'; import { Translate, Record, Identifier, ReduxState } from '../types'; import { RedirectionSideEffect } from '../sideEffect'; import { useTranslate } from '../i18n'; +import useGetOne from './useGetOne'; interface ChildrenFuncParams { isLoading: boolean; @@ -85,23 +87,14 @@ const EditController = (props: Props) => { const { basePath, children, id, resource, undoable } = props; - const record = useSelector((state: ReduxState) => - state.admin.resources[props.resource] - ? state.admin.resources[props.resource].data[props.id] - : null - ); - - const isLoading = useSelector( - (state: ReduxState) => state.admin.loading > 0 - ); - const version = useSelector( (state: ReduxState) => state.admin.ui.viewVersion ); + const { record, loading } = useGetOne(resource, id, basePath, version); + useEffect(() => { dispatch(resetForm(REDUX_FORM_NAME)); - dispatch(crudGetOne(resource, id, basePath)); }, [resource, id, basePath, version]); if (!children) { @@ -139,7 +132,7 @@ const EditController = (props: Props) => { ); return children({ - isLoading, + isLoading: loading, defaultTitle, save, resource, diff --git a/packages/ra-core/src/controller/ShowController.tsx b/packages/ra-core/src/controller/ShowController.tsx index 7ffcd59bdea..87547fe0daa 100644 --- a/packages/ra-core/src/controller/ShowController.tsx +++ b/packages/ra-core/src/controller/ShowController.tsx @@ -1,11 +1,12 @@ -import { ReactNode, useEffect } from 'react'; +import { ReactNode } from 'react'; // @ts-ignore -import { useDispatch, useSelector } from 'react-redux'; +import { useSelector } from 'react-redux'; import inflection from 'inflection'; -import { crudGetOne } from '../actions'; + import { useCheckMinimumRequiredProps } from './checkMinimumRequiredProps'; import { Translate, Record, Identifier, ReduxState } from '../types'; import { useTranslate } from '../i18n'; +import useGetOne from './useGetOne'; interface ChildrenFuncParams { isLoading: boolean; @@ -75,25 +76,12 @@ const ShowController = (props: Props) => { useCheckMinimumRequiredProps('Show', ['basePath', 'resource'], props); const { basePath, children, id, resource } = props; const translate = useTranslate(); - const dispatch = useDispatch(); - - const record = useSelector((state: ReduxState) => - state.admin.resources[props.resource] - ? state.admin.resources[props.resource].data[props.id] - : null - ); - - const isLoading = useSelector( - (state: ReduxState) => state.admin.loading > 0 - ); const version = useSelector( (state: ReduxState) => state.admin.ui.viewVersion ); - useEffect(() => { - dispatch(crudGetOne(resource, id, basePath)); - }, [resource, id, basePath, version]); + const { record, loading } = useGetOne(resource, id, basePath, version); if (!children) { return null; @@ -109,7 +97,7 @@ const ShowController = (props: Props) => { record, }); return children({ - isLoading, + isLoading: loading, defaultTitle, resource, basePath, diff --git a/packages/ra-core/src/controller/useGetOne.ts b/packages/ra-core/src/controller/useGetOne.ts new file mode 100644 index 00000000000..b7e0c9455ca --- /dev/null +++ b/packages/ra-core/src/controller/useGetOne.ts @@ -0,0 +1,66 @@ +import { useEffect } from 'react'; +// @ts-ignore +import { useSelector, useDispatch } from 'react-redux'; + +import { crudGetOne } from '../actions'; +import { ReduxState, Record, Identifier } from '../types'; + +export interface GetOneResponse { + record?: Record; + loading: boolean; + loaded: boolean; + error?: any; +} + +/** + * @typedef GetOneResponse + * @type {Object} + * @property {Object} record + * @property {boolean} loading + * @property {boolean} loaded + * @property {Object} error + */ + +/** + * Fetches a record by resource name and id and returns that record. + * + * Optimistically loads the rrcord from the store if it was already fetched before. + * + * @example const { record } = useGetOne('posts', 123, '/posts'); + * + * @param {string} resource The resource name, e.g. "posts" + * @param {string|int} id the resource identifier, e.g. 123 + * @param {string} basePath the currrent url basePath. Used for redirections. + * @param {number} version An optional integer used to force fetching the record again + * + * @returns {GetOneResponse} An object with the shape { record, error, loading, loaded } + */ +const useGetOne = ( + resource: string, + id: Identifier, + basePath?: string, + version?: number +): GetOneResponse => { + const dispatch = useDispatch(); + const action = crudGetOne(resource, id, basePath); + useEffect(() => { + dispatch(action); + }, [resource, id, basePath, version]); + const record = useSelector((state: ReduxState) => + state.admin.resources[resource] + ? state.admin.resources[resource].data[id] + : null + ); + const { loading, loaded, error } = useSelector((state: ReduxState) => { + const key = JSON.stringify({ + type: action.meta.fetch, + payload: action.payload, + }); + return state.admin.requests && state.admin.requests[key] + ? state.admin.requests[key] + : { loading: false, loaded: false }; + }); + return { record, loading, loaded, error }; +}; + +export default useGetOne; diff --git a/packages/ra-core/src/reducer/admin/index.ts b/packages/ra-core/src/reducer/admin/index.ts index 598df0e254b..44e53d04aa8 100644 --- a/packages/ra-core/src/reducer/admin/index.ts +++ b/packages/ra-core/src/reducer/admin/index.ts @@ -9,6 +9,7 @@ import record from './record'; import references, { getPossibleReferenceValues as referencesGetPossibleReferenceValues, } from './references'; +import requests from './requests'; import saving from './saving'; import ui from './ui'; import auth, { isLoggedIn as authIsLoggedIn } from './auth'; @@ -19,6 +20,7 @@ export default combineReducers({ notifications, record, references, + requests, saving, ui, auth, diff --git a/packages/ra-core/src/reducer/admin/requests.spec.ts b/packages/ra-core/src/reducer/admin/requests.spec.ts new file mode 100644 index 00000000000..7bd0d57c4a7 --- /dev/null +++ b/packages/ra-core/src/reducer/admin/requests.spec.ts @@ -0,0 +1,163 @@ +import expect from 'expect'; +import { + FETCH_START, + FETCH_END, + FETCH_ERROR, + FETCH_CANCEL, +} from '../../actions/fetchActions'; +import reducer from './requests'; + +describe('requests reducer', () => { + it('should return an empty object by default', () => { + expect(reducer(undefined, { type: 'OTHER_ACTION' })).toEqual({}); + }); + + it('should return a loading state upon FETCH_START', () => { + expect( + reducer( + {}, + { + type: 'GET_ONE_LOADING', + payload: { id: 123 }, + meta: { + fetchStatus: FETCH_START, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: true, + }, + }); + }); + it('should return a reloading state upon second FETCH_START', () => { + expect( + reducer( + { + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: true, + loading: false, + }, + }, + { + type: 'GET_ONE_LOADING', + payload: { id: 123 }, + meta: { + fetchStatus: FETCH_START, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: true, + loading: true, + }, + }); + }); + it('should return a loaded state upon FETCH_END', () => { + expect( + reducer( + { + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: true, + }, + }, + { + type: 'GET_ONE_SUCCESS', + requestPayload: { id: 123 }, + meta: { + fetchStatus: FETCH_END, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: true, + loading: false, + }, + }); + }); + it('should return an error state upon FETCH_ERROR', () => { + expect( + reducer( + { + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: true, + }, + }, + { + type: 'GET_ONE_ERROR', + requestPayload: { id: 123 }, + error: 'problem!', + meta: { + fetchStatus: FETCH_ERROR, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: false, + error: 'problem!', + }, + }); + }); + it('should not keep the error on subsequent reloads', () => { + expect( + reducer( + { + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: false, + error: 'problem!', + }, + }, + { + type: 'GET_ONE_LOADING', + payload: { id: 123 }, + meta: { + fetchStatus: FETCH_START, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: true, + }, + }); + }); + it('should return a non loading state upon FETCH_CANCEL', () => { + expect( + reducer( + { + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: true, + }, + }, + { + type: 'GET_ONE_ERROR', + requestPayload: { id: 123 }, + meta: { + fetchStatus: FETCH_CANCEL, + fetchResponse: 'GET_ONE', + }, + } + ) + ).toEqual({ + '{"type":"GET_ONE","payload":{"id":123}}': { + loaded: false, + loading: false, + }, + }); + }); +}); diff --git a/packages/ra-core/src/reducer/admin/requests.ts b/packages/ra-core/src/reducer/admin/requests.ts new file mode 100644 index 00000000000..e65cda0a4e2 --- /dev/null +++ b/packages/ra-core/src/reducer/admin/requests.ts @@ -0,0 +1,58 @@ +import { Reducer } from 'redux'; +import { + FETCH_START, + FETCH_END, + FETCH_ERROR, + FETCH_CANCEL, +} from '../../actions/fetchActions'; + +export interface State { + [key: string]: { + loaded?: boolean; + loading?: boolean; + error?: string; + }; +} + +const requestsReducer: Reducer = ( + previousState = {}, + { payload, requestPayload, error, meta } +) => { + if (!meta || !meta.fetchStatus) { + return previousState; + } + const key = JSON.stringify({ + type: meta.fetchResponse, + payload: requestPayload || payload, + }); + const previousStatus = previousState[key]; + switch (meta.fetchStatus) { + case FETCH_START: + return { + ...previousState, + [key]: + previousStatus && previousStatus.loaded + ? { loaded: true, loading: true } // reloading: do not reset loaded + : { loaded: false, loading: true }, + }; + case FETCH_END: + return { + ...previousState, + [key]: { loaded: true, loading: false }, + }; + case FETCH_ERROR: + return { + ...previousState, + [key]: { error, loaded: false, loading: false }, + }; + case FETCH_CANCEL: + return { + ...previousState, + [key]: { loaded: false, loading: false }, + }; + default: + return previousState; + } +}; + +export default requestsReducer; diff --git a/packages/ra-core/src/sideEffect/fetch.ts b/packages/ra-core/src/sideEffect/fetch.ts index ee8b373ea82..b9aabac8842 100644 --- a/packages/ra-core/src/sideEffect/fetch.ts +++ b/packages/ra-core/src/sideEffect/fetch.ts @@ -104,7 +104,15 @@ export function* handleFetch( } yield all([ - put({ type: `${type}_LOADING`, payload, meta }), + put({ + type: `${type}_LOADING`, + payload, + meta: { + ...meta, + fetchResponse: restType, + fetchStatus: FETCH_START, + }, + }), put({ type: FETCH_START }), ]); const response = yield call( diff --git a/packages/ra-core/src/types.ts b/packages/ra-core/src/types.ts index bb91ca85793..5aec971a7f0 100644 --- a/packages/ra-core/src/types.ts +++ b/packages/ra-core/src/types.ts @@ -48,6 +48,13 @@ export interface ReduxState { optimistic: boolean; viewVersion: number; }; + requests: { + [key: string]: { + loaded?: boolean; + loading?: boolean; + error?: string; + }; + }; resources: { [name: string]: { data: any; diff --git a/packages/ra-core/src/util/TestContext.spec.tsx b/packages/ra-core/src/util/TestContext.spec.tsx index a849c8df5c1..f0b58dec8b3 100644 --- a/packages/ra-core/src/util/TestContext.spec.tsx +++ b/packages/ra-core/src/util/TestContext.spec.tsx @@ -17,6 +17,7 @@ const primedStore = { oneToMany: {}, possibleValues: {}, }, + requests: {}, resources: {}, saving: false, ui: { From 675e524920b718c4e71a43d7f0727df9bc0c1a41 Mon Sep 17 00:00:00 2001 From: fzaninotto Date: Mon, 20 May 2019 00:18:29 +0200 Subject: [PATCH 2/2] Introduce useOptimisticQuery hook That hook fetches the data provider and returns the data from the a=cache as soon as it finds it. --- .../src/actions/dataActions/crudGetOne.ts | 25 ++----- .../ra-core/src/controller/EditController.tsx | 17 ++++- .../ra-core/src/controller/ShowController.tsx | 17 ++++- packages/ra-core/src/controller/useGetOne.ts | 47 ++++-------- .../src/controller/useOptimisticQuery.ts | 73 +++++++++++++++++++ 5 files changed, 126 insertions(+), 53 deletions(-) create mode 100644 packages/ra-core/src/controller/useOptimisticQuery.ts diff --git a/packages/ra-core/src/actions/dataActions/crudGetOne.ts b/packages/ra-core/src/actions/dataActions/crudGetOne.ts index 295ab3334a3..75cc6b661af 100644 --- a/packages/ra-core/src/actions/dataActions/crudGetOne.ts +++ b/packages/ra-core/src/actions/dataActions/crudGetOne.ts @@ -10,23 +10,14 @@ import { export const crudGetOne = ( resource: string, id: Identifier, - basePath: string, - refresh: RefreshSideEffect = true + meta: any ): CrudGetOneAction => ({ type: CRUD_GET_ONE, payload: { id }, meta: { resource, fetch: GET_ONE, - basePath, - onFailure: { - notification: { - body: 'ra.notification.item_doesnt_exist', - level: 'warning', - }, - redirectTo: 'list', - refresh, - }, + ...meta, }, }); @@ -41,12 +32,7 @@ export interface CrudGetOneAction { readonly meta: { resource: string; fetch: typeof GET_ONE; - basePath: string; - onFailure: { - notification: NotificationSideEffect; - redirectTo: RedirectionSideEffect; - refresh: RefreshSideEffect; - }; + [key: string]: any; }; } @@ -68,11 +54,9 @@ export interface CrudGetOneFailureAction { readonly requestPayload: RequestPayload; readonly meta: { resource: string; - notification: NotificationSideEffect; - redirectTo: RedirectionSideEffect; - refresh: RefreshSideEffect; fetchResponse: typeof GET_ONE; fetchStatus: typeof FETCH_ERROR; + [key: string]: any; }; } @@ -87,5 +71,6 @@ export interface CrudGetOneSuccessAction { resource: string; fetchResponse: typeof GET_ONE; fetchStatus: typeof FETCH_END; + [key: string]: any; }; } diff --git a/packages/ra-core/src/controller/EditController.tsx b/packages/ra-core/src/controller/EditController.tsx index 88971f691ff..48187dda3f4 100644 --- a/packages/ra-core/src/controller/EditController.tsx +++ b/packages/ra-core/src/controller/EditController.tsx @@ -91,7 +91,22 @@ const EditController = (props: Props) => { (state: ReduxState) => state.admin.ui.viewVersion ); - const { record, loading } = useGetOne(resource, id, basePath, version); + const { data: record, loading } = useGetOne( + resource, + id, + { + basePath, + onFailure: { + notification: { + body: 'ra.notification.item_doesnt_exist', + level: 'warning', + }, + redirectTo: 'list', + refresh: true, + }, + }, + version + ); useEffect(() => { dispatch(resetForm(REDUX_FORM_NAME)); diff --git a/packages/ra-core/src/controller/ShowController.tsx b/packages/ra-core/src/controller/ShowController.tsx index 87547fe0daa..45914d36372 100644 --- a/packages/ra-core/src/controller/ShowController.tsx +++ b/packages/ra-core/src/controller/ShowController.tsx @@ -81,7 +81,22 @@ const ShowController = (props: Props) => { (state: ReduxState) => state.admin.ui.viewVersion ); - const { record, loading } = useGetOne(resource, id, basePath, version); + const { data: record, loading } = useGetOne( + resource, + id, + { + basePath, + onFailure: { + notification: { + body: 'ra.notification.item_doesnt_exist', + level: 'warning', + }, + redirectTo: 'list', + refresh: true, + }, + }, + version + ); if (!children) { return null; diff --git a/packages/ra-core/src/controller/useGetOne.ts b/packages/ra-core/src/controller/useGetOne.ts index b7e0c9455ca..57a8b2ab100 100644 --- a/packages/ra-core/src/controller/useGetOne.ts +++ b/packages/ra-core/src/controller/useGetOne.ts @@ -1,12 +1,9 @@ -import { useEffect } from 'react'; -// @ts-ignore -import { useSelector, useDispatch } from 'react-redux'; - import { crudGetOne } from '../actions'; import { ReduxState, Record, Identifier } from '../types'; +import useOptimisticQuery from './useOptimisticQuery'; export interface GetOneResponse { - record?: Record; + data?: Record; loading: boolean; loaded: boolean; error?: any; @@ -15,7 +12,7 @@ export interface GetOneResponse { /** * @typedef GetOneResponse * @type {Object} - * @property {Object} record + * @property {Object} data * @property {boolean} loading * @property {boolean} loaded * @property {Object} error @@ -24,43 +21,31 @@ export interface GetOneResponse { /** * Fetches a record by resource name and id and returns that record. * - * Optimistically loads the rrcord from the store if it was already fetched before. + * Optimistically loads the record from the store if it was already fetched before. * - * @example const { record } = useGetOne('posts', 123, '/posts'); + * @example const { data } = useGetOne('posts', 123); * * @param {string} resource The resource name, e.g. "posts" * @param {string|int} id the resource identifier, e.g. 123 - * @param {string} basePath the currrent url basePath. Used for redirections. + * @param {object} meta the action meta (including side effects) * @param {number} version An optional integer used to force fetching the record again * - * @returns {GetOneResponse} An object with the shape { record, error, loading, loaded } + * @returns {GetOneResponse} An object with the shape { data, error, loading, loaded } */ const useGetOne = ( resource: string, id: Identifier, - basePath?: string, - version?: number + meta: any = {}, + version: number ): GetOneResponse => { - const dispatch = useDispatch(); - const action = crudGetOne(resource, id, basePath); - useEffect(() => { - dispatch(action); - }, [resource, id, basePath, version]); - const record = useSelector((state: ReduxState) => - state.admin.resources[resource] - ? state.admin.resources[resource].data[id] - : null + return useOptimisticQuery( + crudGetOne(resource, id, meta), + (state: ReduxState) => + state.admin.resources[resource] + ? state.admin.resources[resource].data[id] + : null, + version ); - const { loading, loaded, error } = useSelector((state: ReduxState) => { - const key = JSON.stringify({ - type: action.meta.fetch, - payload: action.payload, - }); - return state.admin.requests && state.admin.requests[key] - ? state.admin.requests[key] - : { loading: false, loaded: false }; - }); - return { record, loading, loaded, error }; }; export default useGetOne; diff --git a/packages/ra-core/src/controller/useOptimisticQuery.ts b/packages/ra-core/src/controller/useOptimisticQuery.ts new file mode 100644 index 00000000000..365c262e48d --- /dev/null +++ b/packages/ra-core/src/controller/useOptimisticQuery.ts @@ -0,0 +1,73 @@ +import { useEffect } from 'react'; +// @ts-ignore +import { useSelector, useDispatch } from 'react-redux'; + +import { ReduxState } from '../types'; + +export interface AsyncQueryResponse { + data?: any; + loading: boolean; + loaded: boolean; + error?: any; +} + +export interface FetchAction { + type: string; + payload: any; + meta: { + fetch: string; + resource: string; + onSuccess?: any; + onFailure?: any; + }; +} + +/** + * @typedef AsyncQueryResponse + * @type {Object} + * @property {Object} data + * @property {boolean} loading + * @property {boolean} loaded + * @property {Object} error + */ + +/** + * Uses an action to call the data provider + * and optimistically loads the result from the store. + * + * @example + * + * const { data, loading, loaded, error } = useOptimisticQuery( + * { type 'RA/GET_ONE', payload: { id: 123 }, meta: { fetch: GET_ONE, resource: 'posts' } } + * state => state.admin.resources.posts[123] + * ); + * + * @param {object} action The action to dispatch, e.g. { type 'RA/GET_ONE', payload: { id: 123 }, meta: { fetch: GET_ONE, resource: 'posts' } } + * @param {function} selector The selector function to get the result from the store + * @param {number} version The version number. Increase to force a new fetch + * + * @returns {GetOneResponse} An object with the shape { data, error, loading, loaded } + */ +const useOptimisticQuery = ( + action: FetchAction, + selector: (state: ReduxState) => string, + version: number = 1 +) => { + const dispatch = useDispatch(); + useEffect(() => { + dispatch(action); + }, [JSON.stringify({ action, version })]); + const data = useSelector(selector); + const { loading, loaded, error } = useSelector((state: ReduxState) => { + const key = JSON.stringify({ + type: action.meta.fetch, + payload: action.payload, + }); + return state.admin.requests && state.admin.requests[key] + ? state.admin.requests[key] + : { loading: false, loaded: false }; + }); + return { data, loading, loaded, error }; +}; + +export default useOptimisticQuery;