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

[RFR] Use data provider without saga #3468

Merged
merged 6 commits into from
Jul 30, 2019
Merged

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Jul 29, 2019

  • Rename the fetch directory in ra-core to dataProvider
  • [BC Break] Add <DataProviderContext> and use it in <AdminCore>
  • Copy fetch and undo sagas logic inside useDataProvider
  • Bypass these sagas in useDataProvider by no longer including the fetch meta
  • [BC Break] Reimplementing the Undo feature using an EventEmitter
  • Fix tests
  • Add Upgrade guide

Overall this is completely transparent unless you're using a custom app. The benefit is that the useDataProvider code is much easier to follow, with less layers to look at. Also, merging fetch and undo really makes sense as the Undo feature can only undo a fetch action.

The cons are the use of the EventEmitter, which I preferred over a temporary redux state, and the removal of the possibility to hack the react-admin dataProvider logic by replacing the fetch saga.

From that point on, technically we could remove the fetch and auth sagas, as well as the crudXXX action creators. However, we keep them for backward compatibility's sake.

@fzaninotto fzaninotto added this to the 3.0.0 milestone Jul 29, 2019
@@ -4,6 +4,7 @@ import { act, cleanup } from 'react-testing-library';

import EditController from './EditController';
import renderWithRedux from '../util/renderWithRedux';
import { DataProviderContext } from '../dataProvider';
Copy link
Contributor

Choose a reason for hiding this comment

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

Not used ?

const useDataProvider = (): DataProviderHookFunction => {
const dispatch = useDispatch() as Dispatch;
const dataProvider = useContext(DataProviderContext) || defaultDataProvider;
const isOptimistic = useSelector(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need this reducer to handle optimistic detection. Can't w achieve the same with metas on the action ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because the use case is the following:

  1. user clicks on Delete post
  2. the useDataProvider hook optimistically deletes the record from the store and redirects to the post list
  3. the list calls the dataProvider for the list of posts on mount
  4. While waiting for the result, the list shows the posts from the sorte, where the deleted post has been removed
  5. As the deletion wasn't performed on the server yet, the server returns results including the deleted record
  6. The list shows the deleted post again

To avoid that, the 'optimistic mode' blocks queries while an optimistic update is pending, so 3 and 5 never happen.

@djhi djhi merged commit 1361e47 into next Jul 30, 2019
@djhi djhi deleted the useDataProvider-without-saga branch July 30, 2019 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants