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

Fix react-admin requires custom routers to be data routers #9723

Merged
merged 6 commits into from
Mar 15, 2024

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Mar 14, 2024

The reimplentation of the warnWhenUnsavedChanges feature with react-router's useBlocker in #9657 created a breaking change. As useBlocker throws an error when not called in a DataRouter, and since we cannot call hooks conditionally, this meant that react-admin MUST be used inside a data router.

By moving the warnWhenUnsavedChanges logic from the useAugmentedForm hook to a custom component, we can now call the component conditionally - and avoid calling it when not inside a data router.

The reimplentation of the `warnWhenUnsavedChanges` feature with react-router's `useBlocker` created a breaking change. As `useBlocker` throws an error when not called in a DataRouter, and since we cannot call hooks conditionally, this meant that react-admin MUST be used inside a data router.

By moving the `warnWhenUnsavedChanges` logic from the `useAugmentedForm` hook to a custom component, we can now call the component conditionally - and avoid calling it when not inside a data router.
@fzaninotto fzaninotto added the RFR Ready For Review label Mar 14, 2024
import {
FormProvider,
FieldValues,
UseFormProps,
SubmitHandler,
} from 'react-hook-form';
import {
UNSAFE_DataRouterContext,
Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately react-router doesn't expose the DataRouterContext, see remix-run/react-router#11347

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Other than that, LGTM! 🙂

@@ -351,7 +351,7 @@ export const TagEdit = () => (
);
```

**Warning**: This feature only works if you have a dependency on react-router 6.3.0 **at most**. The react-router team disabled this possibility in react-router 6.4, so `warnWhenUnsavedChanges` will silently fail with react-router 6.4 or later.
**Note**: Due to limitations in react-router, this feature only works if you use the default router provided by react-admin, or if you use a Data Router.
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: I'd add a link to react-router's documentation for those not knowing what a Data Router is

Copy link
Member Author

Choose a reason for hiding this comment

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

you're right, added.

docs/Upgrade.md Outdated Show resolved Hide resolved
fzaninotto and others added 2 commits March 15, 2024 11:49
Co-authored-by: Jean-Baptiste Kaiser <jb@marmelab.com>
@djhi djhi requested a review from slax57 March 15, 2024 15:53
@slax57 slax57 merged commit 8a0c18b into next Mar 15, 2024
1 of 2 checks passed
@slax57 slax57 deleted the warnwhenunsavedchanges branch March 15, 2024 16:11
@fzaninotto fzaninotto added this to the 5.0.0 milestone Mar 29, 2024
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