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

[BC Break] Cleanup dataProvider code #7001

Merged
merged 34 commits into from
Jan 3, 2022
Merged

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Dec 16, 2021

We no longer need to rely on Redux for the data fetching.

  • Remove data provider actions
  • Remove data provider reducers
  • Remove data provider optimistic / pessimistic query logic
  • Remove base data provider hooks (useQuery, useQueryWithStore, useMutation)
  • Remove data provider base components (<Query>, <Mutation>)
  • Fix tests
  • Write doc
  • Write upgrade guide

@fzaninotto fzaninotto added this to the 4.0.0-alpha.1 milestone Dec 16, 2021
@vercel vercel bot temporarily deployed to Preview – react-admin December 28, 2021 08:08 Inactive
@fzaninotto fzaninotto changed the title Replace version and loading in Redux by react-query hooks [BC Break] Cleanup dataProvider code Dec 28, 2021
@fzaninotto fzaninotto mentioned this pull request Dec 28, 2021
@fzaninotto fzaninotto marked this pull request as ready for review December 31, 2021 09:29
@vercel vercel bot temporarily deployed to Preview – react-admin December 31, 2021 09:29 Inactive
@fzaninotto fzaninotto added the RFR Ready For Review label Dec 31, 2021
@fzaninotto
Copy link
Member Author

The PR is now Ready for Review

UPGRADE.md Outdated Show resolved Hide resolved
const BanUserButton = ({ userId }) => {
- const { loaded, error, data } = useMutation({
- type: 'banUser',
- payload: userId
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- payload: userId
- payload: { id: userId }

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessarily, and nto the purpose of this snippet.

UPGRADE.md Outdated Show resolved Hide resolved
UPGRADE.md Show resolved Hide resolved
UPGRADE.md Show resolved Hide resolved
UPGRADE.md Outdated Show resolved Hide resolved
UPGRADE.md Outdated Show resolved Hide resolved
Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Remove / Update references to removed actions:

* and therefore we need a custom action (CRUD_GET_LIST) that will be used.

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Update comment as mutation callbacks can no longer be used as event handlers:

- return <button disabled={isLoading} onClick={updateMany}>Reset views</button>;
+ return <button disabled={isLoading} onClick={() => updateMany()}>Reset views</button>;

* return <button disabled={isLoading} onClick={updateMany}>Reset views</button>;

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Typo

- For instance, here is how to override the side eggects for the `getOne` query in a `<Show>` component: 
+ For instance, here is how to override the side effects for the `getOne` query in a `<Show>` component: 

For instance, here is how to override the side eggects for the `getOne` query in a `<Show>` component:

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Is it still valid to pass an onSuccess side effect like in this comment ?

* May include side effects to be executed upon success or failure, e.g. { onSuccess: { refresh: true } }

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Remove useVersion hook from Reference.md line 212

- * `useVersion` 

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Edit's successMessage is still referenced in some files and should be removed:

  • docs/CreateEdit.md
  • packages/ra-core/src/controller/edit/useEditContext.tsx
  • packages/ra-ui-materialui/src/detail/Edit.tsx
  • packages/ra-ui-materialui/src/detail/EditView.tsx

@fzaninotto
Copy link
Member Author

Review applied

Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

There an eslint warning left but otherwise, this is great!

@vercel vercel bot temporarily deployed to Preview – react-admin January 3, 2022 09:04 Inactive
@fzaninotto fzaninotto merged commit 06bf8d7 into next Jan 3, 2022
@fzaninotto fzaninotto deleted the userefresh-react-query branch January 3, 2022 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants