refactor: use discriminated return type

Signed-off-by: Camila Belo <camilaibs@gmail.com>

plugins/auth-react/api-report.md

@@ -21,12 +21,19 @@ export type CookieAuthRefreshProviderProps = {
 export function useCookieAuthRefresh(options: {
   pluginId: string;
   path?: string;
-}): {
-  status: 'loading' | 'error' | 'success';
-  error?: Error;
-  result?: {
-    expiresAt: string;
-  };
-  retry: () => void;
-};
+}):
+  | {
+      status: 'loading';
+    }
+  | {
+      status: 'error';
+      error: Error;
+      retry: () => void;
+    }
+  | {
+      status: 'success';
+      data: {
+        expiresAt: string;
+      };
+    };
 ```

plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.tsx

@@ -44,16 +44,16 @@ export function CookieAuthRefreshProvider(
   const app = useApp();
   const { Progress } = app.getComponents();
 
-  const { status, error, retry } = useCookieAuthRefresh(options);
+  const result = useCookieAuthRefresh(options);
 
-  if (status === 'loading') {
+  if (result.status === 'loading') {
     return <Progress />;
   }
 
-  if (status === 'error' && error) {
+  if (result.status === 'error') {
     return (
-      <ErrorPanel error={error}>
-        <Button variant="outlined" onClick={retry}>
+      <ErrorPanel error={result.error}>
+        <Button variant="outlined" onClick={result.retry}>
           Retry
         </Button>
       </ErrorPanel>

plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.test.tsx

@@ -80,8 +80,6 @@ describe('useCookieAuthRefresh', () => {
     );
 
     expect(result.current.status).toBe('loading');
-    expect(result.current.error).toBeUndefined();
-    expect(result.current.result).toBeUndefined();
   });
 
   it('should return a loading status when retrying without previous success', async () => {
@@ -112,19 +110,25 @@ describe('useCookieAuthRefresh', () => {
       },
     );
 
-    expect(result.current.status).toBe('loading');
-    expect(result.current.error).toBeUndefined();
-    expect(result.current.result).toBeUndefined();
+    expect(result.current).toStrictEqual({ status: 'loading' });
 
-    await waitFor(() => expect(result.current.status).toBe('error'));
-    expect(result.current.result).toBeUndefined();
-    expect(result.current.error).toStrictEqual(error);
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'error',
+        error,
+        retry: expect.any(Function),
+      }),
+    );
 
-    result.current.retry();
+    if (result.current.status === 'error') {
+      result.current.retry();
+    }
 
-    await waitFor(() => expect(result.current.status).toBe('loading'));
-    expect(result.current.result).toBeUndefined();
-    expect(result.current.error).toStrictEqual(error);
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'loading',
+      }),
+    );
   });
 
   it('should return a loading status when retrying with previous success', async () => {
@@ -159,25 +163,34 @@ describe('useCookieAuthRefresh', () => {
       },
     );
 
-    expect(result.current.status).toBe('loading');
-    expect(result.current.error).toBeUndefined();
-    expect(result.current.result).toBeUndefined();
+    expect(result.current).toStrictEqual({ status: 'loading' });
 
-    await waitFor(() => expect(result.current.status).toBe('success'));
-    expect(result.current.result).toMatchObject({ expiresAt });
-    expect(result.current.error).toBeUndefined();
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'success',
+        data: { expiresAt },
+      }),
+    );
 
-    result.current.retry();
+    jest.advanceTimersByTime(tenMinutesInMilliseconds);
 
-    await waitFor(() => expect(result.current.status).toBe('error'));
-    expect(result.current.result).toMatchObject({ expiresAt });
-    expect(result.current.error).toStrictEqual(error);
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'error',
+        error,
+        retry: expect.any(Function),
+      }),
+    );
 
-    result.current.retry();
+    if (result.current.status === 'error') {
+      result.current.retry();
+    }
 
-    await waitFor(() => expect(result.current.status).toBe('loading'));
-    expect(result.current.result).toMatchObject({ expiresAt });
-    expect(result.current.error).toStrictEqual(error);
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'loading',
+      }),
+    );
   });
 
   it('should return an error status when the refresh has failed', async () => {
@@ -205,7 +218,13 @@ describe('useCookieAuthRefresh', () => {
       },
     );
 
-    await waitFor(() => expect(result.current.error).toStrictEqual(error));
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'error',
+        error,
+        retry: expect.any(Function),
+      }),
+    );
   });
 
   it('should call the api to get the cookie and use it', async () => {
@@ -233,7 +252,12 @@ describe('useCookieAuthRefresh', () => {
       ),
     );
 
-    expect(result.current.result).toMatchObject({ expiresAt });
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'success',
+        data: { expiresAt },
+      }),
+    );
   });
 
   it('should cancel the refresh when a message is received from another tab', async () => {
@@ -295,9 +319,14 @@ describe('useCookieAuthRefresh', () => {
       },
     );
 
-    await waitFor(() => expect(fetchApiMock.fetch).toHaveBeenCalledTimes(1));
+    await waitFor(() =>
+      expect(result.current).toStrictEqual({
+        status: 'success',
+        data: { expiresAt },
+      }),
+    );
 
-    expect(result.current.result).toMatchObject({ expiresAt });
+    await waitFor(() => expect(fetchApiMock.fetch).toHaveBeenCalledTimes(1));
 
     unmount();
 

plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { useEffect, useCallback, useMemo } from 'react';
+import { useEffect, useCallback } from 'react';
 import {
   discoveryApiRef,
   fetchApiRef,
@@ -34,14 +34,10 @@ export function useCookieAuthRefresh(options: {
   pluginId: string;
   // The path to used for calling the refresh cookie endpoint, default to '/cookie'
   path?: string;
-}): {
-  status: 'loading' | 'error' | 'success';
-  error?: Error;
-  result?: {
-    expiresAt: string;
-  };
-  retry: () => void;
-} {
+}):
+  | { status: 'loading' }
+  | { status: 'error'; error: Error; retry: () => void }
+  | { status: 'success'; data: { expiresAt: string } } {
   const { pluginId, path = '/cookie' } = options ?? {};
   const fetchApi = useApi(fetchApiRef);
   const storageApi = useApi(storageApiRef);
@@ -58,14 +54,19 @@ export function useCookieAuthRefresh(options: {
     if (!response.ok) {
       throw await ResponseError.fromResponse(response);
     }
-    return await response.json();
+    const data = await response.json();
+    if (!data.expiresAt) {
+      throw new Error('No expiration date found in response');
+    }
+    return data;
   });
 
   useMountEffect(actions.execute);
 
   const refresh = useCallback(
     (params: { expiresAt: string }) => {
-      // Randomize the refreshing margin to avoid all tabs refreshing at the same time
+      // Randomize the refreshing margin with a margin of 1-4 minutes to avoid all tabs refreshing at the same time
+      // It cannot be less than 5 minutes otherwise the backend will return the same expiration date
       const margin = (1 + 3 * Math.random()) * 60000;
       const delay = Date.parse(params.expiresAt) - Date.now() - margin;
       const timeout = setTimeout(actions.execute, delay);
@@ -97,43 +98,35 @@ export function useCookieAuthRefresh(options: {
     };
   }, [state, refresh, store]);
 
-  const status = useMemo(() => {
-    // Initialising
-    if (state.status === 'not-executed') {
-      return 'loading';
-    }
-
-    // First refresh or retrying without any success before
-    // Possible states transitions:
-    // e.g. not-executed -> loading (first-refresh)
-    // e.g. not-executed -> loading (first-refresh) -> error -> loading (manual-retry)
-    if (state.status === 'loading' && !state.result) {
-      return 'loading';
-    }
-
-    // Retrying after having succeeding at least once
-    // Current states is: { status: 'loading', result: {...}, error: undefined | Error }
-    // e.g. not-executed -> loading (first-refresh) -> success -> loading (scheduled-refresh) -> error -> loading (manual-retry)
-    if (state.status === 'loading' && state.error) {
-      return 'loading';
-    }
-
-    // Something went wrong during the any situation of a refresh
-    if (state.status === 'error' && state.error) {
-      return 'error';
-    }
-
-    return 'success';
-  }, [state]);
-
   const retry = useCallback(() => {
     actions.execute();
   }, [actions]);
 
-  return {
-    retry,
-    status,
-    result: state.result,
-    error: state.error,
-  };
+  // Initialising
+  if (state.status === 'not-executed') {
+    return { status: 'loading' };
+  }
+
+  // First refresh or retrying without any success before
+  // Possible state transitions:
+  // e.g. not-executed -> loading (first-refresh)
+  // e.g. not-executed -> loading (first-refresh) -> error -> loading (manual-retry)
+  if (state.status === 'loading' && !state.result) {
+    return { status: 'loading' };
+  }
+
+  // Retrying after having succeeding at least once
+  // Current state is: { status: 'loading', result: {...}, error: undefined | Error }
+  // e.g. not-executed -> loading (first-refresh) -> success -> loading (scheduled-refresh) -> error -> loading (manual-retry)
+  if (state.status === 'loading' && state.error) {
+    return { status: 'loading' };
+  }
+
+  // Something went wrong during any situation of a refresh
+  if (state.status === 'error' && state.error) {
+    return { status: 'error', error: state.error, retry };
+  }
+
+  // At this point it should be safe to assume that we have a successful refresh
+  return { status: 'success', data: state.result! };
 }