From 6eeae1336053c7a6b37a17418e080ef579a6f094 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Tue, 13 Oct 2020 12:20:16 +0200 Subject: [PATCH] Relax useDataProvider signature to ease custom methods usage --- .../ra-core/src/dataProvider/Mutation.tsx | 6 +- .../ra-core/src/dataProvider/Query.spec.tsx | 19 +++ packages/ra-core/src/dataProvider/Query.tsx | 6 +- .../getDataProviderCallArguments.ts | 46 ++++++ .../src/dataProvider/useDataProvider.spec.js | 155 +++++++++++++----- .../src/dataProvider/useDataProvider.ts | 45 +++-- ...eDataProviderWithDeclarativeSideEffects.ts | 59 +++++-- .../src/dataProvider/useMutation.spec.tsx | 24 ++- .../ra-core/src/dataProvider/useMutation.ts | 18 +- packages/ra-core/src/dataProvider/useQuery.ts | 25 ++- 10 files changed, 319 insertions(+), 84 deletions(-) create mode 100644 packages/ra-core/src/dataProvider/getDataProviderCallArguments.ts diff --git a/packages/ra-core/src/dataProvider/Mutation.tsx b/packages/ra-core/src/dataProvider/Mutation.tsx index 45cc974bf0b..cb7e19b8bb1 100644 --- a/packages/ra-core/src/dataProvider/Mutation.tsx +++ b/packages/ra-core/src/dataProvider/Mutation.tsx @@ -20,7 +20,7 @@ interface Props { params: ChildrenFuncParams ) => JSX.Element; type: string; - resource: string; + resource?: string; payload?: any; options?: any; } @@ -57,7 +57,9 @@ const Mutation: FunctionComponent = ({ type, resource, payload, - options, + // Provides an undefined onSuccess just so the key `onSuccess` is defined + // This is used to detect options in useDataProvider + options = { onSuccess: undefined }, }) => children( ...useMutation( diff --git a/packages/ra-core/src/dataProvider/Query.spec.tsx b/packages/ra-core/src/dataProvider/Query.spec.tsx index 270ef41a95a..67c110fe62e 100644 --- a/packages/ra-core/src/dataProvider/Query.spec.tsx +++ b/packages/ra-core/src/dataProvider/Query.spec.tsx @@ -564,4 +564,23 @@ describe('Query', () => { meta: { resource: 'foo' }, }); }); + + it('should allow custom dataProvider methods without resource', () => { + const dataProvider = { + mytype: jest.fn(() => Promise.resolve({ data: { foo: 'bar' } })), + }; + + const myPayload = {}; + const { getByText, dispatch } = renderWithRedux( + + + {({}) =>
} + + + ); + const action = dispatch.mock.calls[0][0]; + expect(action.type).toEqual('CUSTOM_FETCH'); + expect(action.meta.resource).toBeUndefined(); + expect(dataProvider.mytype).toHaveBeenCalledWith(myPayload); + }); }); diff --git a/packages/ra-core/src/dataProvider/Query.tsx b/packages/ra-core/src/dataProvider/Query.tsx index fe908ce25c9..d5c93324937 100644 --- a/packages/ra-core/src/dataProvider/Query.tsx +++ b/packages/ra-core/src/dataProvider/Query.tsx @@ -12,7 +12,7 @@ interface ChildrenFuncParams { interface Props { children: (params: ChildrenFuncParams) => JSX.Element; type: string; - resource: string; + resource?: string; payload?: any; options?: any; } @@ -71,7 +71,9 @@ const Query: FunctionComponent = ({ type, resource, payload, - options, + // Provides an undefined onSuccess just so the key `onSuccess` is defined + // This is used to detect options in useDataProvider + options = { onSuccess: undefined }, }) => children( useQuery( diff --git a/packages/ra-core/src/dataProvider/getDataProviderCallArguments.ts b/packages/ra-core/src/dataProvider/getDataProviderCallArguments.ts new file mode 100644 index 00000000000..8be249bae7f --- /dev/null +++ b/packages/ra-core/src/dataProvider/getDataProviderCallArguments.ts @@ -0,0 +1,46 @@ +import { UseDataProviderOptions } from '../types'; + +// List of properties we expect in the options +const OptionsProperties = [ + 'action', + 'fetch', + 'meta', + 'onFailure', + 'onSuccess', + 'undoable', +]; + +const isDataProviderOptions = (value: any) => { + let options = value as UseDataProviderOptions; + + return Object.keys(options).some(key => OptionsProperties.includes(key)); +}; + +// As all dataProvider methods do not have the same signature, we must differentiate +// standard methods which have the (resource, params, options) signature +// from the custom ones +export const getDataProviderCallArguments = (args: any[]) => { + const lastArg = args[args.length - 1]; + let allArguments = [...args]; + + let resource; + let payload; + let options; + + if (isDataProviderOptions(lastArg)) { + options = lastArg as UseDataProviderOptions; + allArguments = allArguments.slice(0, args.length - 1); + } + + if (typeof allArguments[0] === 'string') { + resource = allArguments[0]; + payload = allArguments[1]; + } + + return { + resource, + payload, + allArguments, + options, + }; +}; diff --git a/packages/ra-core/src/dataProvider/useDataProvider.spec.js b/packages/ra-core/src/dataProvider/useDataProvider.spec.js index ad542c94ae2..19a98c344be 100644 --- a/packages/ra-core/src/dataProvider/useDataProvider.spec.js +++ b/packages/ra-core/src/dataProvider/useDataProvider.spec.js @@ -24,6 +24,36 @@ const UseGetOne = () => { return
loading
; }; +const UseCustomVerb = ({ onSuccess }) => { + const [data, setData] = useState(); + const [error, setError] = useState(); + const dataProvider = useDataProvider(); + useEffect(() => { + dataProvider + .customVerb({ id: 1 }, ['something'], { onSuccess }) + .then(res => setData(res.data)) + .catch(e => setError(e)); + }, [dataProvider, onSuccess]); + if (error) return
{error.message}
; + if (data) return
{JSON.stringify(data)}
; + return
loading
; +}; + +const UseCustomVerbWithStandardSignature = ({ onSuccess }) => { + const [data, setData] = useState(); + const [error, setError] = useState(); + const dataProvider = useDataProvider(); + useEffect(() => { + dataProvider + .customVerb('posts', { id: 1 }, { onSuccess }) + .then(res => setData(res.data)) + .catch(e => setError(e)); + }, [dataProvider, onSuccess]); + if (error) return
{error.message}
; + if (data) return
{JSON.stringify(data)}
; + return
loading
; +}; + describe('useDataProvider', () => { afterEach(cleanup); @@ -107,6 +137,57 @@ describe('useDataProvider', () => { expect(dispatch.mock.calls[4][0].type).toBe('RA/FETCH_END'); }); + it('should call custom verbs with standard signature (resource, payload, options)', async () => { + const onSuccess = jest.fn(); + const customVerb = jest.fn(() => Promise.resolve({ data: null })); + const dataProvider = { customVerb }; + renderWithRedux( + + + + ); + // wait for the dataProvider to return + await act(async () => { + await new Promise(resolve => setTimeout(resolve)); + }); + + expect(customVerb).toHaveBeenCalledWith('posts', { id: 1 }); + }); + + it('should accept custom arguments for custom verbs', async () => { + const customVerb = jest.fn(() => Promise.resolve({ data: null })); + const dataProvider = { customVerb }; + renderWithRedux( + + + + ); + // wait for the dataProvider to return + await act(async () => { + await new Promise(resolve => setTimeout(resolve)); + }); + + expect(customVerb).toHaveBeenCalledWith({ id: 1 }, ['something']); + }); + + it('should accept custom arguments for custom verbs and allow options', async () => { + const onSuccess = jest.fn(); + const customVerb = jest.fn(() => Promise.resolve({ data: null })); + const dataProvider = { customVerb }; + renderWithRedux( + + + + ); + // wait for the dataProvider to return + await act(async () => { + await new Promise(resolve => setTimeout(resolve)); + }); + + expect(customVerb).toHaveBeenCalledWith({ id: 1 }, ['something']); + expect(onSuccess).toHaveBeenCalledWith({ data: null }); + }); + describe('options', () => { it('should accept an action option to dispatch a custom action', async () => { const UseGetOneWithCustomAction = () => { @@ -317,44 +398,44 @@ describe('useDataProvider', () => { 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(); - validUntil.setTime(validUntil.getTime() + 1000); - return Promise.resolve({ data: { id: 1 }, validUntil }); - }); - const dataProvider = { - getOne, - update: () => Promise.resolve({ data: { id: 1, foo: 'bar' } }), - }; - const Update = () => { - const [update] = useUpdate('posts', 1, { foo: 'bar ' }); - return ; - }; - const { getByText, rerender } = renderWithRedux( - - - - , - { admin: { resources: { posts: { data: {}, list: {} } } } } - ); - // wait for the dataProvider to return - await act(async () => await new Promise(r => setTimeout(r))); - expect(getOne).toBeCalledTimes(1); - // click on the update button - await act(async () => { - fireEvent.click(getByText('update')); - await new Promise(r => setTimeout(r)); + it('should not use the cache after an update', async () => { + const getOne = jest.fn(() => { + const validUntil = new Date(); + validUntil.setTime(validUntil.getTime() + 1000); + return Promise.resolve({ data: { id: 1 }, validUntil }); + }); + const dataProvider = { + getOne, + update: () => Promise.resolve({ data: { id: 1, foo: 'bar' } }), + }; + const Update = () => { + const [update] = useUpdate('posts', 1, { foo: 'bar ' }); + return ; + }; + const { getByText, rerender } = renderWithRedux( + + + + , + { admin: { resources: { posts: { data: {}, list: {} } } } } + ); + // wait for the dataProvider to return + await act(async () => await new Promise(r => setTimeout(r))); + expect(getOne).toBeCalledTimes(1); + // click on the update button + await act(async () => { + fireEvent.click(getByText('update')); + await new Promise(r => setTimeout(r)); + }); + rerender( + + + + ); + // wait for the dataProvider to return + await act(async () => await new Promise(r => setTimeout(r))); + expect(getOne).toBeCalledTimes(2); }); - rerender( - - - - ); - // wait for the dataProvider to return - await act(async () => await new Promise(r => setTimeout(r))); - expect(getOne).toBeCalledTimes(2); }); }); diff --git a/packages/ra-core/src/dataProvider/useDataProvider.ts b/packages/ra-core/src/dataProvider/useDataProvider.ts index 09681e1da5f..da7eefaec20 100644 --- a/packages/ra-core/src/dataProvider/useDataProvider.ts +++ b/packages/ra-core/src/dataProvider/useDataProvider.ts @@ -15,13 +15,9 @@ import { import { FETCH_END, FETCH_ERROR, FETCH_START } from '../actions/fetchActions'; import { showNotification } from '../actions/notificationActions'; import { refreshView } from '../actions/uiActions'; -import { - ReduxState, - DataProvider, - DataProviderProxy, - UseDataProviderOptions, -} from '../types'; +import { ReduxState, DataProvider, DataProviderProxy } from '../types'; import useLogoutIfAccessDenied from '../auth/useLogoutIfAccessDenied'; +import { getDataProviderCallArguments } from './getDataProviderCallArguments'; // List of dataProvider calls emitted while in optimistic mode. // These calls get replayed once the dataProvider exits optimistic mode @@ -132,11 +128,14 @@ const useDataProvider = (): DataProviderProxy => { if (typeof name === 'symbol') { return; } - return ( - resource: string, - payload: any, - options: UseDataProviderOptions - ) => { + return (...args) => { + const { + resource, + payload, + allArguments, + options, + } = getDataProviderCallArguments(args); + const type = name.toString(); const { action = 'CUSTOM_FETCH', @@ -179,6 +178,7 @@ const useDataProvider = (): DataProviderProxy => { rest, store, type, + allArguments, undoable, }; if (isOptimistic) { @@ -214,6 +214,7 @@ const doQuery = ({ store, undoable, logoutIfAccessDenied, + allArguments, }) => { const resourceState = store.getState().admin.resources[resource]; if (canReplyWithCache(type, payload, resourceState)) { @@ -240,6 +241,7 @@ const doQuery = ({ dataProvider, dispatch, logoutIfAccessDenied, + allArguments, }) : performQuery({ type, @@ -252,6 +254,7 @@ const doQuery = ({ dataProvider, dispatch, logoutIfAccessDenied, + allArguments, }); }; @@ -275,6 +278,7 @@ const performUndoableQuery = ({ dataProvider, dispatch, logoutIfAccessDenied, + allArguments, }: QueryFunctionParams) => { dispatch(startOptimisticMode()); if (window) { @@ -320,7 +324,13 @@ const performUndoableQuery = ({ }); dispatch({ type: FETCH_START }); try { - dataProvider[type](resource, payload) + dataProvider[type] + .apply( + dataProvider, + typeof resource !== 'undefined' + ? [resource, payload] + : allArguments + ) .then(response => { if (process.env.NODE_ENV !== 'production') { validateResponseFormat(response, type); @@ -441,6 +451,7 @@ const performQuery = ({ dataProvider, dispatch, logoutIfAccessDenied, + allArguments, }: QueryFunctionParams) => { dispatch({ type: action, @@ -453,8 +464,15 @@ const performQuery = ({ meta: { resource, ...rest }, }); dispatch({ type: FETCH_START }); + try { - return dataProvider[type](resource, payload) + return dataProvider[type] + .apply( + dataProvider, + typeof resource !== 'undefined' + ? [resource, payload] + : allArguments + ) .then(response => { if (process.env.NODE_ENV !== 'production') { validateResponseFormat(response, type); @@ -552,6 +570,7 @@ interface QueryFunctionParams { dataProvider: DataProvider; dispatch: Dispatch; logoutIfAccessDenied: (error?: any) => Promise; + allArguments: any[]; } export default useDataProvider; diff --git a/packages/ra-core/src/dataProvider/useDataProviderWithDeclarativeSideEffects.ts b/packages/ra-core/src/dataProvider/useDataProviderWithDeclarativeSideEffects.ts index 4f98b4342c0..2b70f1c89eb 100644 --- a/packages/ra-core/src/dataProvider/useDataProviderWithDeclarativeSideEffects.ts +++ b/packages/ra-core/src/dataProvider/useDataProviderWithDeclarativeSideEffects.ts @@ -1,7 +1,8 @@ import useDataProvider from './useDataProvider'; import { useMemo } from 'react'; -import { DataProviderProxy, UseDataProviderOptions } from '../types'; +import { DataProviderProxy } from '../types'; import useDeclarativeSideEffects from './useDeclarativeSideEffects'; +import { getDataProviderCallArguments } from './getDataProviderCallArguments'; /** * This version of the useDataProvider hook ensure Query and Mutation components are still usable @@ -20,21 +21,51 @@ const useDataProviderWithDeclarativeSideEffects = (): DataProviderProxy => { if (typeof name === 'symbol') { return; } - return ( - resource: string, - payload: any, - options: UseDataProviderOptions - ) => { - const { onSuccess, onFailure } = getSideEffects( + return (...args) => { + const { resource, - options - ); + payload, + allArguments, + options, + } = getDataProviderCallArguments(args); + + let onSuccess; + let onFailure; + let finalOptions = options; + let finalAllArguments = allArguments; + + if ( + Object.keys(options).some(key => + ['onSuccess', 'onFailure'].includes(key) + ) + ) { + const sideEffect = getSideEffects(resource, options); + let { + onSuccess: ignoreOnSuccess, // Used to extract options without onSuccess + onFailure: ignoreOnFailure, // Used to extract options without onFailure + ...otherOptions + } = options; + onSuccess = sideEffect.onSuccess; + onFailure = sideEffect.onFailure; + finalOptions = otherOptions; + finalAllArguments = [...args].slice(0, args.length - 1); + } + try { - return target[name.toString()](resource, payload, { - ...options, - onSuccess, - onFailure, - }); + return target[name.toString()].apply( + target, + typeof resource === 'string' + ? [ + resource, + payload, + { + ...finalOptions, + onSuccess, + onFailure, + }, + ] + : finalAllArguments + ); } catch (e) { // turn synchronous exceptions (e.g. in parameter preparation) // into async ones, otherwise they'll be lost diff --git a/packages/ra-core/src/dataProvider/useMutation.spec.tsx b/packages/ra-core/src/dataProvider/useMutation.spec.tsx index a766e87283e..e6877ec84e4 100644 --- a/packages/ra-core/src/dataProvider/useMutation.spec.tsx +++ b/packages/ra-core/src/dataProvider/useMutation.spec.tsx @@ -68,7 +68,7 @@ describe('useMutation', () => { {mutate => ( } + + + ); + fireEvent.click(getByText('Hello')); + const action = dispatch.mock.calls[0][0]; + expect(action.type).toEqual('CUSTOM_FETCH'); + expect(action.meta.resource).toBeUndefined(); + expect(dataProvider.mytype).toHaveBeenCalledWith(myPayload); + }); }); diff --git a/packages/ra-core/src/dataProvider/useMutation.ts b/packages/ra-core/src/dataProvider/useMutation.ts index c3a4f294bdd..20c5bf24426 100644 --- a/packages/ra-core/src/dataProvider/useMutation.ts +++ b/packages/ra-core/src/dataProvider/useMutation.ts @@ -156,11 +156,13 @@ const useMutation = ( setState(prevState => ({ ...prevState, loading: true })); - finalDataProvider[params.type]( - params.resource, - params.payload, - params.options - ) + finalDataProvider[params.type] + .apply( + finalDataProvider, + typeof params.resource !== 'undefined' + ? [params.resource, params.payload, params.options] + : [params.payload, params.options] + ) .then(({ data, total }) => { setState({ data, @@ -195,7 +197,7 @@ const useMutation = ( export interface Mutation { type: string; - resource: string; + resource?: string; payload: object; } @@ -302,9 +304,9 @@ const hasDeclarativeSideEffectsSupport = ( }; const sanitizeOptions = (args?: MutationOptions) => { - if (!args) return {}; + if (!args) return { onSuccess: undefined }; const { withDeclarativeSideEffectsSupport, ...options } = args; - return options; + return { onSuccess: undefined, ...options }; }; export default useMutation; diff --git a/packages/ra-core/src/dataProvider/useQuery.ts b/packages/ra-core/src/dataProvider/useQuery.ts index b0df27368c1..b9ac3fbf414 100644 --- a/packages/ra-core/src/dataProvider/useQuery.ts +++ b/packages/ra-core/src/dataProvider/useQuery.ts @@ -67,11 +67,18 @@ import useVersion from '../controller/useVersion'; * ); * }; */ -const useQuery = (query: Query, options: QueryOptions = {}): UseQueryValue => { +const useQuery = ( + query: Query, + options: QueryOptions = { onSuccess: undefined } +): UseQueryValue => { const { type, resource, payload } = query; - const { withDeclarativeSideEffectsSupport, ...rest } = options; + const { withDeclarativeSideEffectsSupport, ...otherOptions } = options; const version = useVersion(); // used to allow force reload - const requestSignature = JSON.stringify({ query, options: rest, version }); + const requestSignature = JSON.stringify({ + query, + options: otherOptions, + version, + }); const [state, setState] = useSafeSetState({ data: undefined, error: null, @@ -89,13 +96,19 @@ const useQuery = (query: Query, options: QueryOptions = {}): UseQueryValue => { * * @deprecated to be removed in 4.0 */ - const dataProviderWithSideEffects = withDeclarativeSideEffectsSupport + const finalDataProvider = withDeclarativeSideEffectsSupport ? dataProviderWithDeclarativeSideEffects : dataProvider; setState(prevState => ({ ...prevState, loading: true })); - dataProviderWithSideEffects[type](resource, payload, rest) + finalDataProvider[type] + .apply( + finalDataProvider, + typeof resource !== 'undefined' + ? [resource, payload, otherOptions] + : [payload, otherOptions] + ) .then(({ data, total }) => { setState({ data, @@ -124,7 +137,7 @@ const useQuery = (query: Query, options: QueryOptions = {}): UseQueryValue => { export interface Query { type: string; - resource: string; + resource?: string; payload: object; }