Add caching to the useEntityPermission hook (#9064)

* Add caching to the useEntityPermission hook Signed-off-by: Joon Park <joonp@spotify.com> * Remove useRef Signed-off-by: Joon Park <joonp@spotify.com> * Use useSWR hook Signed-off-by: Joon Park <joonp@spotify.com>
backstage · Feb 2, 2022 · 6acc8f7 · 6acc8f7
1 parent 74298c1
commit 6acc8f7
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 47 deletions.
diff --git a/.changeset/weak-oranges-drive.md b/.changeset/weak-oranges-drive.md
@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-catalog-react': patch
+---
+
+Add caching to the useEntityPermission hook
+
+The hook now caches the authorization decision based on the permission + the entity, and returns the cache match value as the default `allowed` value while loading. This helps avoid flicker in UI elements that would be conditionally rendered based on the `allowed` result of this hook.
diff --git a/plugins/permission-react/package.json b/plugins/permission-react/package.json
@@ -32,7 +32,8 @@
     "@backstage/plugin-permission-common": "^0.4.0",
     "cross-fetch": "^3.0.6",
     "react-router": "6.0.0-beta.0",
-    "react-use": "^17.2.4"
+    "react-use": "^17.2.4",
+    "swr": "^1.1.2"
   },
   "peerDependencies": {
     "@types/react": "^16.13.1 || ^17.0.0",

diff --git a/plugins/permission-react/src/hooks/usePermission.test.tsx b/plugins/permission-react/src/hooks/usePermission.test.tsx
@@ -19,9 +19,8 @@ import { render } from '@testing-library/react';
 import { usePermission } from './usePermission';
 import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import { TestApiProvider } from '@backstage/test-utils';
-import { permissionApiRef } from '../apis';
-
-const mockAuthorize = jest.fn();
+import { PermissionApi, permissionApiRef } from '../apis';
+import { SWRConfig } from 'swr';
 
 const permission = {
   name: 'access.something',
@@ -39,49 +38,48 @@ const TestComponent: FC = () => {
   );
 };
 
+function renderComponent(mockApi: PermissionApi) {
+  return render(
+    <SWRConfig value={{ provider: () => new Map() }}>
+      <TestApiProvider apis={[[permissionApiRef, mockApi]]}>
+        <TestComponent />
+      </TestApiProvider>
+      ,
+    </SWRConfig>,
+  );
+}
+
 describe('usePermission', () => {
+  const mockPermissionApi = { authorize: jest.fn() };
+
   it('Returns loading when permissionApi has not yet responded.', () => {
-    mockAuthorize.mockReturnValueOnce(new Promise(() => {}));
+    mockPermissionApi.authorize.mockReturnValueOnce(new Promise(() => {}));
 
-    const { getByText } = render(
-      <TestApiProvider
-        apis={[[permissionApiRef, { authorize: mockAuthorize }]]}
-      >
-        <TestComponent />
-      </TestApiProvider>,
-    );
+    const { getByText } = renderComponent(mockPermissionApi);
 
-    expect(mockAuthorize).toHaveBeenCalledWith({ permission });
+    expect(mockPermissionApi.authorize).toHaveBeenCalledWith({ permission });
     expect(getByText('loading')).toBeTruthy();
   });
 
   it('Returns allowed when permissionApi allows authorization.', async () => {
-    mockAuthorize.mockResolvedValueOnce({ result: AuthorizeResult.ALLOW });
+    mockPermissionApi.authorize.mockResolvedValueOnce({
+      result: AuthorizeResult.ALLOW,
+    });
 
-    const { findByText } = render(
-      <TestApiProvider
-        apis={[[permissionApiRef, { authorize: mockAuthorize }]]}
-      >
-        <TestComponent />
-      </TestApiProvider>,
-    );
+    const { findByText } = renderComponent(mockPermissionApi);
 
-    expect(mockAuthorize).toHaveBeenCalledWith({ permission });
+    expect(mockPermissionApi.authorize).toHaveBeenCalledWith({ permission });
     expect(await findByText('content')).toBeTruthy();
   });
 
   it('Returns not allowed when permissionApi denies authorization.', async () => {
-    mockAuthorize.mockResolvedValueOnce({ result: AuthorizeResult.DENY });
+    mockPermissionApi.authorize.mockResolvedValueOnce({
+      result: AuthorizeResult.DENY,
+    });
 
-    const { findByText } = render(
-      <TestApiProvider
-        apis={[[permissionApiRef, { authorize: mockAuthorize }]]}
-      >
-        <TestComponent />
-      </TestApiProvider>,
-    );
+    const { findByText } = renderComponent(mockPermissionApi);
 
-    expect(mockAuthorize).toHaveBeenCalledWith({ permission });
+    expect(mockPermissionApi.authorize).toHaveBeenCalledWith({ permission });
     await expect(findByText('content')).rejects.toThrowError();
   });
 });
diff --git a/plugins/permission-react/src/hooks/usePermission.ts b/plugins/permission-react/src/hooks/usePermission.ts
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-import useAsync from 'react-use/lib/useAsync';
 import { useApi } from '@backstage/core-plugin-api';
 import { permissionApiRef } from '../apis';
 import {
   AuthorizeResult,
   Permission,
 } from '@backstage/plugin-permission-common';
+import useSWR from 'swr';
 
 /** @public */
 export type AsyncPermissionResult = {
@@ -30,31 +30,33 @@ export type AsyncPermissionResult = {
 };
 
 /**
- * React hook utlity for authorization. Given a {@link @backstage/plugin-permission-common#Permission} and an optional
- * resourceRef, it will return whether or not access is allowed (for the given resource, if resourceRef is provided). See
- * {@link @backstage/plugin-permission-common/PermissionClient#authorize} for more details.
+ * React hook utility for authorization. Given a
+ * {@link @backstage/plugin-permission-common#Permission} and an optional
+ * resourceRef, it will return whether or not access is allowed (for the given
+ * resource, if resourceRef is provided). See
+ * {@link @backstage/plugin-permission-common/PermissionClient#authorize} for
+ * more details.
+ *
+ * Note: This hook uses stale-while-revalidate to help avoid flicker in UI
+ * elements that would be conditionally rendered based on the `allowed` result
+ * of this hook.
  * @public
  */
 export const usePermission = (
   permission: Permission,
   resourceRef?: string,
 ): AsyncPermissionResult => {
   const permissionApi = useApi(permissionApiRef);
-
-  const { loading, error, value } = useAsync(async () => {
-    const { result } = await permissionApi.authorize({
-      permission,
-      resourceRef,
-    });
-
+  const { data, error } = useSWR({ permission, resourceRef }, async args => {
+    const { result } = await permissionApi.authorize(args);
     return result;
-  }, [permissionApi, permission, resourceRef]);
+  });
 
-  if (loading) {
-    return { loading: true, allowed: false };
-  }
   if (error) {
     return { error, loading: false, allowed: false };
   }
-  return { loading: false, allowed: value === AuthorizeResult.ALLOW };
+  if (data === undefined) {
+    return { loading: true, allowed: false };
+  }
+  return { loading: false, allowed: data === AuthorizeResult.ALLOW };
 };
diff --git a/yarn.lock b/yarn.lock
@@ -23027,6 +23027,11 @@ swap-case@^2.0.2:
   dependencies:
     tslib "^2.0.3"
 
+swr@^1.1.2:
+  version "1.1.2"
+  resolved "https://registry.npmjs.org/swr/-/swr-1.1.2.tgz#9f3de2541931fccf03c48f322f1fc935a7551612"
+  integrity sha512-UsM0eo5T+kRPyWFZtWRx2XR5qzohs/LS4lDC0GCyLpCYFmsfTk28UCVDbOE9+KtoXY4FnwHYiF+ZYEU3hnJ1lQ==
+
 symbol-observable@1.2.0, symbol-observable@^1.0.4, symbol-observable@^1.1.0, symbol-observable@^1.2.0:
   version "1.2.0"
   resolved "https://registry.npmjs.org/symbol-observable/-/symbol-observable-1.2.0.tgz#c22688aed4eab3cdc2dfeacbb561660560a00804"