fix: isLoading has correct status when autoSignIn is disabled and use…

…r is signed out (#982) * fix: isLoading should be set to false when the primary useEffect completes Fixes #981 * fix: dont set refreshed user unless mounted Possibly fixes #980 but doubtful * test: finish rendering before makign assertions
bjerkio · Mar 21, 2023 · db28cdd · db28cdd
1 parent 07ac025
commit db28cdd
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 40 deletions.
diff --git a/src/AuthContext.tsx b/src/AuthContext.tsx
@@ -121,14 +121,14 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
    */
   useEffect(() => {
     isMountedRef.current = true;
+    setIsLoading(true);
     (async () => {
       const user = await userManager!.getUser();
       if (!user || user.expired) {
         // If the user is returning back from the OIDC provider, get and set the user data.
         if (hasCodeInUrl(location)) {
           const user = (await userManager.signinCallback()) || null;
           setUserData(user);
-          setIsLoading(false);
           onSignIn && onSignIn(user);
         }
         // If autoSignIn is enabled, redirect to the OIDC provider.
@@ -140,8 +140,8 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
       // Otherwise if the user is already signed in, set the user data.
       else if (isMountedRef.current) {
         setUserData(user);
-        setIsLoading(false);
       }
+      setIsLoading(false);
     })();
     return () => {
       isMountedRef.current = false;
@@ -153,7 +153,7 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
    */
   useEffect(() => {
     const updateUserData: UserLoadedCallback = (user: User): void => {
-      setUserData(user);
+      isMountedRef.current && setUserData(user);
     };
     const onSilentRenewError: SilentRenewErrorCallback =
       async (): Promise<void> => {

diff --git a/src/__tests__/AuthContext.test.tsx b/src/__tests__/AuthContext.test.tsx
@@ -97,25 +97,29 @@ describe('AuthContext', () => {
   });
 
   it('should generate a UserManager', async () => {
-    render(
-      <AuthProvider
-        authority="http://127.0.0.1"
-        clientId="client-id-test"
-        redirectUri="http://127.0.0.1"
-      />,
-    );
+    await act(async () => {
+      render(
+        <AuthProvider
+          authority="http://127.0.0.1"
+          clientId="client-id-test"
+          redirectUri="http://127.0.0.1"
+        />,
+      );
+    });
     expect(UserManager).toHaveBeenCalled();
   });
 
   it('should use post-logout redirect URI when given', async () => {
-    render(
-      <AuthProvider
-        authority="http://127.0.0.1"
-        clientId="client-id-test"
-        redirectUri="http://127.0.0.1"
-        postLogoutRedirectUri="https://localhost"
-      />,
-    );
+    await act(async () => {
+      render(
+        <AuthProvider
+          authority="http://127.0.0.1"
+          clientId="client-id-test"
+          redirectUri="http://127.0.0.1"
+          postLogoutRedirectUri="https://localhost"
+        />,
+      );
+    });
     expect(UserManager).toHaveBeenLastCalledWith(
       expect.objectContaining({
         post_logout_redirect_uri: 'https://localhost',
@@ -124,40 +128,46 @@ describe('AuthContext', () => {
   });
 
   it('should fall back to redirectUri when post-logout redirect URI is not given', async () => {
-    render(
-      <AuthProvider
-        authority="http://127.0.0.1"
-        clientId="client-id-test"
-        redirectUri="http://127.0.0.1"
-      />,
-    );
+    await act(async () => {
+      render(
+        <AuthProvider
+          authority="http://127.0.0.1"
+          clientId="client-id-test"
+          redirectUri="http://127.0.0.1"
+        />,
+      );
+    });
     expect(UserManager).toHaveBeenLastCalledWith(
       expect.objectContaining({ post_logout_redirect_uri: 'http://127.0.0.1' }),
     );
   });
 
   it('should use silent redirect URI when given', async () => {
-    render(
-      <AuthProvider
-        authority="http://127.0.0.1"
-        clientId="client-id-test"
-        redirectUri="http://127.0.0.1"
-        silentRedirectUri="https://localhost"
-      />,
-    );
+    await act(async () => {
+      render(
+        <AuthProvider
+          authority="http://127.0.0.1"
+          clientId="client-id-test"
+          redirectUri="http://127.0.0.1"
+          silentRedirectUri="https://localhost"
+        />,
+      );
+    });
     expect(UserManager).toHaveBeenLastCalledWith(
       expect.objectContaining({ silent_redirect_uri: 'https://localhost' }),
     );
   });
 
   it('should fall back to redirectUri when silent redirect URI is not given', async () => {
-    render(
-      <AuthProvider
-        authority="http://127.0.0.1"
-        clientId="client-id-test"
-        redirectUri="http://127.0.0.1"
-      />,
-    );
+    await act(async () => {
+      render(
+        <AuthProvider
+          authority="http://127.0.0.1"
+          clientId="client-id-test"
+          redirectUri="http://127.0.0.1"
+        />,
+      );
+    });
     expect(UserManager).toHaveBeenLastCalledWith(
       expect.objectContaining({ silent_redirect_uri: 'http://127.0.0.1' }),
     );