Skip to content

Commit

Permalink
Merge pull request #3473 from marmelab/Fix-race-condition-in-useMutation
Browse files Browse the repository at this point in the history
[RFR] Fix Review accept / reject in demo
djhi authored Jul 31, 2019
2 parents 1361e47 + 0d8a864 commit 32d2b2a
Showing 6 changed files with 106 additions and 83 deletions.
47 changes: 21 additions & 26 deletions examples/demo/src/reviews/AcceptButton.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,39 @@
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { formValueSelector } from 'redux-form';
import Button from '@material-ui/core/Button';
import ThumbUp from '@material-ui/icons/ThumbUp';
import { useTranslate, useMutation } from 'react-admin';

const options = {
undoable: true,
onSuccess: {
notification: {
body: 'resources.reviews.notification.approved_success',
level: 'info',
},
redirectTo: '/reviews',
},
onFailure: {
notification: {
body: 'resources.reviews.notification.approved_error',
level: 'warning',
},
},
};
import { useTranslate, useMutation, useNotify, useRedirect } from 'react-admin';

/**
* This custom button demonstrate using useMutation to update data
*/
const AcceptButton = ({ record }) => {
const translate = useTranslate();
const notify = useNotify();
const redirect = useRedirect();
const [approve, { loading }] = useMutation(
{
type: 'UPDATE',
resource: 'reviews',
payload: { id: record.id, data: { status: 'accepted' } },
},
options
{
undoable: true,
onSuccess: () => {
notify(
'resources.reviews.notification.approved_success',
'info',
{},
true
);
redirect('/reviews');
},
onFailure: () =>
notify(
'resources.reviews.notification.approved_error',
'warning'
),
}
);
return record && record.status === 'pending' ? (
<Button
@@ -60,8 +59,4 @@ AcceptButton.propTypes = {
comment: PropTypes.string,
};

const selector = formValueSelector('record-form');

export default connect(state => ({
comment: selector(state, 'comment'),
}))(AcceptButton);
export default AcceptButton;
48 changes: 29 additions & 19 deletions examples/demo/src/reviews/BulkAcceptButton.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,43 @@
import React from 'react';
import PropTypes from 'prop-types';
import ThumbUp from '@material-ui/icons/ThumbUp';
import { Button, useMutation, UPDATE_MANY } from 'react-admin';

const options = {
undoable: true,
onSuccess: {
notification: {
body: 'resources.reviews.notification.approved_success',
level: 'info',
},
redirectTo: '/reviews',
},
onFailure: {
notification: {
body: 'resources.reviews.notification.approved_error',
level: 'warning',
},
},
};
import {
Button,
useMutation,
useNotify,
useRedirect,
useUnselectAll,
UPDATE_MANY,
} from 'react-admin';

const BulkAcceptButton = ({ selectedIds }) => {
const notify = useNotify();
const redirect = useRedirect();
const unselectAll = useUnselectAll('reviews');
const [approve, { loading }] = useMutation(
{
type: UPDATE_MANY,
resource: 'reviews',
payload: { ids: selectedIds, data: { status: 'accepted' } },
},
options
{
undoable: true,
onSuccess: () => {
notify(
'resources.reviews.notification.approved_success',
'info',
{},
true
);
redirect('/reviews');
unselectAll();
},
onFailure: () =>
notify(
'resources.reviews.notification.approved_error',
'warning'
),
}
);

return (
47 changes: 29 additions & 18 deletions examples/demo/src/reviews/BulkRejectButton.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,43 @@
import React from 'react';
import PropTypes from 'prop-types';
import ThumbDown from '@material-ui/icons/ThumbDown';
import { Button, useMutation, UPDATE_MANY } from 'react-admin';
import {
Button,
useMutation,
useNotify,
useRedirect,
useUnselectAll,
UPDATE_MANY,
} from 'react-admin';

const options = {
undoable: true,
onSuccess: {
notification: {
body: 'resources.reviews.notification.approved_success',
level: 'info',
},
redirectTo: '/reviews',
},
onFailure: {
notification: {
body: 'resources.reviews.notification.approved_error',
level: 'warning',
},
},
};
const BulkRejectButton = ({ selectedIds }) => {
const notify = useNotify();
const redirect = useRedirect();
const unselectAll = useUnselectAll('reviews');
const [reject, { loading }] = useMutation(
{
type: UPDATE_MANY,
resource: 'reviews',
payload: { ids: selectedIds, data: { status: 'rejected' } },
},
options
{
undoable: true,
onSuccess: () => {
notify(
'resources.reviews.notification.approved_success',
'info',
{},
true
);
redirect('/reviews');
unselectAll();
},
onFailure: () =>
notify(
'resources.reviews.notification.approved_error',
'warning'
),
}
);

return (
43 changes: 25 additions & 18 deletions packages/ra-core/src/dataProvider/useDataProvider.ts
Original file line number Diff line number Diff line change
@@ -88,11 +88,21 @@ const useDataProvider = (): DataProviderHookFunction => {
const {
action = 'CUSTOM_FETCH',
undoable = false,
onSuccess = {},
onFailure = {},
onSuccess,
onFailure,
...rest
} = options;

if (onSuccess && typeof onSuccess !== 'function') {
throw new Error('The onSuccess option must be a function');
}
if (onFailure && typeof onFailure !== 'function') {
throw new Error('The onFailure option must be a function');
}
if (undoable && !onSuccess) {
throw new Error(
'You must pass an onSuccess callback calling notify() to use the undoable mode'
);
}
if (isOptimistic) {
// in optimistic mode, all fetch actions are canceled,
// so the admin uses the store without synchronization
@@ -105,8 +115,8 @@ const useDataProvider = (): DataProviderHookFunction => {
resource,
action,
rest,
successFunc: getSideEffectFunc(onSuccess),
failureFunc: getSideEffectFunc(onFailure),
onSuccess,
onFailure,
dataProvider,
dispatch,
};
@@ -118,9 +128,6 @@ const useDataProvider = (): DataProviderHookFunction => {
);
};

const getSideEffectFunc = (effect): ((args: any) => void) =>
effect instanceof Function ? effect : () => null;

/**
* In undoable mode, the hook dispatches an optimistic action and executes
* the success side effects right away. Then it waits for a few seconds to
@@ -136,8 +143,8 @@ const performUndoableQuery = ({
resource,
action,
rest,
successFunc,
failureFunc,
onSuccess,
onFailure,
dataProvider,
dispatch,
}: QueryFunctionParams) => {
@@ -156,7 +163,7 @@ const performUndoableQuery = ({
optimistic: true,
},
});
successFunc({});
onSuccess && onSuccess({});
undoableEventEmitter.once('end', ({ isUndo }) => {
dispatch(stopOptimisticMode());
if (isUndo) {
@@ -202,7 +209,7 @@ const performUndoableQuery = ({
},
});
dispatch({ type: FETCH_ERROR, error });
failureFunc(error);
onFailure && onFailure(error);
throw new Error(error.message ? error.message : error);
});
});
@@ -221,8 +228,8 @@ const performQuery = ({
resource,
action,
rest,
successFunc,
failureFunc,
onSuccess,
onFailure,
dataProvider,
dispatch,
}: QueryFunctionParams) => {
@@ -255,7 +262,7 @@ const performQuery = ({
},
});
dispatch({ type: FETCH_END });
successFunc(response);
onSuccess && onSuccess(response);
return response;
})
.catch(error => {
@@ -272,7 +279,7 @@ const performQuery = ({
},
});
dispatch({ type: FETCH_ERROR, error });
failureFunc(error);
onFailure && onFailure(error);
throw new Error(error.message ? error.message : error);
});
};
@@ -285,8 +292,8 @@ interface QueryFunctionParams {
/** The root action name, e.g. `CRUD_GET_MANY` */
action: string;
rest: any;
successFunc: (args?: any) => void;
failureFunc: (error: any) => void;
onSuccess?: (args?: any) => void;
onFailure?: (error: any) => void;
dataProvider: DataProvider;
dispatch: Dispatch;
}
2 changes: 1 addition & 1 deletion packages/ra-core/src/dataProvider/useMutation.ts
Original file line number Diff line number Diff line change
@@ -121,7 +121,7 @@ const useMutation = (
});
},
// deep equality, see https://github.com/facebook/react/issues/14476#issuecomment-471199055
[JSON.stringify({ query, options })] // eslint-disable-line react-hooks/exhaustive-deps
[JSON.stringify({ query, options }), dataProvider] // eslint-disable-line react-hooks/exhaustive-deps
);

return [mutate, state];
2 changes: 1 addition & 1 deletion packages/ra-core/src/dataProvider/useQuery.ts
Original file line number Diff line number Diff line change
@@ -112,7 +112,7 @@ const useQuery = (
});
});
// deep equality, see https://github.com/facebook/react/issues/14476#issuecomment-471199055
}, [JSON.stringify({ query, options })]); // eslint-disable-line react-hooks/exhaustive-deps
}, [JSON.stringify({ query, options }), dataProvider]); // eslint-disable-line react-hooks/exhaustive-deps

return state;
};

0 comments on commit 32d2b2a

Please sign in to comment.