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 usePermissions always triggers a re-render even though the permissions are unchanged #5607

Merged
merged 2 commits into from
Dec 1, 2020

Conversation

fzaninotto
Copy link
Member

Problem

All react-admin routes (list, edit, create, etc) are wrapped by WithPermissions. This component renders the page content optimistically with empty permissions, then calls authProvider.getPermissions(), and passes the result to the child.

In many cases, this means the routes will render twice: once with empty permissions, and once with the permissions returned by the authProvider.

Solution

WithPermissions calls authProvider.getPermissions() with empty params. In many cases, the result of this call is always the same.

useGetPermissions can cache the result from the first call, and initialize its state with it.

that way, in most cases, the return from authProvider.getPermissions() will be the same as the cached one, and we'll save a re-render.

This should git a short perf boost to all apps, whether they use permissions or not.

Before

12 renders when coming back to the Post List page

image

After

11 renders when coming back to the Post List page

image

@fzaninotto fzaninotto added the RFR Ready For Review label Nov 30, 2020
@fzaninotto fzaninotto changed the title Fix usePermissions always triggers e re-render even though the permissions are unchanged Fix usePermissions always triggers a re-render even though the permissions are unchanged Nov 30, 2020
@fzaninotto
Copy link
Member Author

That won't work because even though the permissions don't change, usePermissions changes the loading state once auth.getPermissions() returns, and that triggers a rerender.

This is unfortunate here as we don't use the loading state.

this means we must introduce a new hook that only stores the permissions, but not the loading state.

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.

I can't figure a better name but I'm not convinced by usePermissionsOptimized. I think most people will end up using it just because of the name even though they need its loading state

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.

Oh and you forgot to document it ;)

@fzaninotto
Copy link
Member Author

It's an internal API. I'm not even sure we should export it.

@djhi djhi merged commit a0aa570 into master Dec 1, 2020
@djhi djhi deleted the fix-usePermissions-rerender branch December 1, 2020 09:00
@fzaninotto fzaninotto added this to the 3.11 milestone Dec 15, 2020
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.

2 participants