-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 incorrect <Ready>
page when dynamically load routes with no resources
#8490
Conversation
Needs to fix implementation of
<Ready> with import CSV pop up will not be shown. (which now should be the correct behavior, previous behavior is incorrect.) The <Ready> needs to be moved to <Layout>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall your fix seems good, thanks!
But it would be nice to have some tests to prove that it works, and cover this case in the future. Could you add some?
I have added a test case so the initial state of lazyRoutes is empty and later is set to a route by useEffect. It triggers a rerender of the useConfigureAdminRouterFromChildren hook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, there is a linter warning in packages/ra-no-code/src/Admin.tsx
line 15:
'Ready' is defined but never used
packages/ra-core/src/core/useConfigureAdminRouterFromChildren.spec.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
As a follow up of feature #7609 under the use case of defining an admin without resources but routes,
There seems to be a bug if additional routes are lazily added to the list of child routes when user navigates in the app.
It triggers a rerender https://github.com/marmelab/react-admin/blame/master/packages/ra-core/src/core/useConfigureAdminRouterFromChildren.tsx#L123 and it incorrectly redirects the user to
<Ready />
componentThis pull requests adds additional checks similar to https://github.com/marmelab/react-admin/blame/master/packages/ra-core/src/core/useConfigureAdminRouterFromChildren.tsx#L253