chore(react-core): ensure useDataState returns value of last dispatch (…

…#6382)

* Create nasty-lemons-agree.md

.changeset/nasty-lemons-agree.md

@@ -0,0 +1,6 @@
+---
+"@aws-amplify/ui-react-core": patch
+"@aws-amplify/ui-react-storage": patch
+---
+
+chore(react-core): ensure useDataState returns value of last dispatch

packages/react-core/src/hooks/index.ts

@@ -1,5 +1,5 @@
 export {
-  default as useDataState,
+  useDataState,
   AsyncDataAction,
   DataAction,
   DataState,

packages/react-core/src/hooks/useDataState/__tests__/useDataState.native.spec.ts

@@ -0,0 +1,12 @@
+import { renderHook } from '@testing-library/react';
+
+import useDataState from '../useDataState.native';
+
+it('throws the expected error when called with a type not mapped to an action handler', () => {
+  // turn off console.error logging for unhappy path test case
+  jest.spyOn(console, 'error').mockImplementation(() => {});
+
+  expect(() => renderHook(() => useDataState())).toThrow(
+    new Error('useDataState is not implemented for React Native')
+  );
+});

packages/react-core/src/hooks/__tests__/useDataState.spec.ts → packages/react-core/src/hooks/useDataState/__tests__/useDataState.spec.ts

@@ -1,12 +1,30 @@
 import { act, renderHook, waitFor } from '@testing-library/react';
+
 import useDataState from '../useDataState';
 
 const asyncAction = jest.fn((_prev: string, next: string) =>
   Promise.resolve(next)
 );
 const syncAction = jest.fn((_prev: string, next: string) => next);
 
-const errorMessage = 'Unhappy!';
+const sleepyAction = jest.fn(
+  (
+    _: string,
+    { timeout, fail }: { fail?: boolean; timeout: number }
+  ): Promise<string> =>
+    new Promise((resolve, reject) =>
+      setTimeout(
+        () =>
+          fail
+            ? reject(new Error(timeout.toString()))
+            : resolve(timeout.toString()),
+        timeout
+      )
+    )
+);
+
+const error = new Error('Unhappy!');
+const errorMessage = error.message;
 const unhappyAction = jest.fn((_, isUnhappy: boolean) =>
   isUnhappy ? Promise.reject(new Error(errorMessage)) : Promise.resolve()
 );
@@ -15,6 +33,13 @@ const initData = 'initial-data';
 const nextData = 'next-data';
 
 describe('useDataState', () => {
+  beforeAll(() => {
+    let id = 0;
+    Object.defineProperty(globalThis, 'crypto', {
+      value: { randomUUID: () => ++id },
+    });
+  });
+
   beforeEach(() => {
     jest.clearAllMocks();
   });
@@ -112,7 +137,7 @@ describe('useDataState', () => {
     });
 
     expect(onError).toHaveBeenCalledTimes(1);
-    expect(onError).toHaveBeenCalledWith(errorMessage);
+    expect(onError).toHaveBeenCalledWith(error);
   });
 
   it('handles an error and resets error state on the next call to handleAction', async () => {
@@ -157,5 +182,86 @@ describe('useDataState', () => {
     });
   });
 
-  it.todo('only returns the value of the last call to handleAction');
+  it('only returns the value of the last dispatch in the happy path', async () => {
+    jest.useFakeTimers();
+
+    const defaultValue = '';
+    const timeoutOne = 2000;
+    const timeoutTwo = 1000;
+    const expectedResult = timeoutTwo.toString();
+
+    const { result } = renderHook(() =>
+      useDataState(sleepyAction, defaultValue)
+    );
+
+    const [initState, dispatch] = result.current;
+
+    act(() => {
+      dispatch({ timeout: timeoutOne });
+    });
+
+    expect(initState.data).toBe(defaultValue);
+
+    expect(sleepyAction).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      dispatch({ timeout: timeoutTwo });
+    });
+
+    expect(sleepyAction).toHaveBeenCalledTimes(2);
+
+    jest.runAllTimers();
+
+    await waitFor(() => {
+      const [resolvedState] = result.current;
+
+      // assert both calls have completed
+      expect(sleepyAction.mock.results.length).toBe(2);
+
+      expect(resolvedState.data).toBe(expectedResult);
+      expect(resolvedState.isLoading).toBe(false);
+      expect(resolvedState.hasError).toBe(false);
+    });
+  });
+
+  it('only returns the value of the last dispatch in the unhappy path', async () => {
+    jest.useFakeTimers();
+
+    const defaultValue = '';
+    const timeoutOne = 2000;
+    const timeoutTwo = 1000;
+    const expectedResult = timeoutTwo.toString();
+
+    const { result } = renderHook(() =>
+      useDataState(sleepyAction, defaultValue)
+    );
+
+    const [initState, dispatch] = result.current;
+
+    act(() => {
+      dispatch({ timeout: timeoutOne, fail: true });
+    });
+
+    expect(initState.data).toBe(defaultValue);
+
+    expect(sleepyAction).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      dispatch({ timeout: timeoutTwo, fail: true });
+    });
+
+    jest.runAllTimers();
+
+    await waitFor(() => {
+      const [resolvedState] = result.current;
+
+      // assert both calls have completed
+      expect(sleepyAction.mock.results.length).toBe(2);
+
+      expect(resolvedState.data).toBe(defaultValue);
+      expect(resolvedState.message).toBe(expectedResult);
+      expect(resolvedState.hasError).toBe(true);
+      expect(resolvedState.isLoading).toBe(false);
+    });
+  });
 });

packages/react-core/src/hooks/useDataState/index.ts

@@ -0,0 +1,2 @@
+export { default as useDataState } from './useDataState';
+export { AsyncDataAction, DataAction, DataState } from './types';

packages/react-core/src/hooks/useDataState/types.ts

@@ -0,0 +1,13 @@
+export interface DataState<T> {
+  data: T;
+  hasError: boolean;
+  isLoading: boolean;
+  message: string | undefined;
+}
+
+export type DataAction<T = any, K = any> = (prevData: T, input: K) => T;
+
+export type AsyncDataAction<T = any, K = any> = (
+  prevData: T,
+  input: K
+) => Promise<T>;

packages/react-core/src/hooks/useDataState/useDataState.native.ts

@@ -0,0 +1,3 @@
+export default function useDataState(): void {
+  throw new Error('useDataState is not implemented for React Native');
+}

packages/react-core/src/hooks/useDataState.ts → packages/react-core/src/hooks/useDataState/useDataState.ts

@@ -1,19 +1,7 @@
-import { isFunction } from '@aws-amplify/ui';
 import React from 'react';
+import { isFunction } from '@aws-amplify/ui';
 
-export interface DataState<T> {
-  data: T;
-  hasError: boolean;
-  isLoading: boolean;
-  message: string | undefined;
-}
-
-export type DataAction<T = any, K = any> = (prevData: T, input: K) => T;
-
-export type AsyncDataAction<T = any, K = any> = (
-  prevData: T,
-  input: K
-) => Promise<T>;
+import { AsyncDataAction, DataAction, DataState } from './types';
 
 // default state
 const INITIAL_STATE = { hasError: false, isLoading: false, message: undefined };
@@ -27,12 +15,15 @@ const resolveMaybeAsync = async <T>(
   return awaited;
 };
 
+/**
+ * @internal may be updated in future versions
+ */
 export default function useDataState<T, K>(
   action: DataAction<T, K> | AsyncDataAction<T, K>,
   initialData: T,
   options?: {
     onSuccess?: (data: T) => void;
-    onError?: (message: string) => void;
+    onError?: (error: Error) => void;
   }
 ): [state: DataState<T>, handleAction: (input: K) => void] {
   const [dataState, setDataState] = React.useState<DataState<T>>(() => ({
@@ -41,22 +32,33 @@ export default function useDataState<T, K>(
   }));
 
   const prevData = React.useRef(initialData);
+  const pendingId = React.useRef<string | undefined>();
 
   const { onSuccess, onError } = options ?? {};
 
   const handleAction: (input: K) => void = React.useCallback(
     (input) => {
+      const id = crypto.randomUUID();
+      pendingId.current = id;
+
       setDataState(({ data }) => ({ ...LOADING_STATE, data }));
 
       resolveMaybeAsync(action(prevData.current, input))
         .then((data: T) => {
-          if (isFunction(onSuccess)) onSuccess(data);
+          if (pendingId.current !== id) return;
 
           prevData.current = data;
+
+          if (isFunction(onSuccess)) onSuccess(data);
+
           setDataState({ ...INITIAL_STATE, data });
         })
-        .catch(({ message }: Error) => {
-          if (isFunction(onError)) onError(message);
+        .catch((error: Error) => {
+          if (pendingId.current !== id) return;
+
+          if (isFunction(onError)) onError(error);
+
+          const { message } = error;
 
           setDataState(({ data }) => ({ ...ERROR_STATE, data, message }));
         });

packages/react-storage/jest.config.ts

@@ -8,6 +8,8 @@ const config: Config = {
     '!<rootDir>/**/(index|version).(ts|tsx)',
     // do not collect from top level styles directory
     '!<rootDir>/src/styles/*.ts',
+    // do not collect coverage of test utils
+    '!<rootDir>/src/**/__testUtils__/*.(ts|tsx)',
   ],
   coverageThreshold: {
     global: {

packages/react-storage/src/components/StorageBrowser/StorageBrowserAmplify.tsx

@@ -6,13 +6,10 @@ import { createAmplifyAuthAdapter } from './adapters';
 
 export interface StorageBrowserProps extends StorageBrowserPropsBase {}
 
-export const StorageBrowser = ({
-  views,
-  displayText,
-}: StorageBrowserProps): React.JSX.Element => {
-  const { StorageBrowser } = React.useRef(
+export function StorageBrowser(props: StorageBrowserProps): React.JSX.Element {
+  const { StorageBrowser: StorageBrowserComponent } = React.useRef(
     createStorageBrowser({ config: createAmplifyAuthAdapter() })
   ).current;
 
-  return <StorageBrowser views={views} displayText={displayText} />;
-};
+  return <StorageBrowserComponent {...props} />;
+}

packages/react-storage/src/components/StorageBrowser/__tests__/StorageBrowserAmplify.spec.tsx

@@ -1,23 +1,23 @@
 import React from 'react';
-import { render, waitFor, screen } from '@testing-library/react';
+import { render } from '@testing-library/react';
 import { Amplify } from 'aws-amplify';
-import * as CreateStorageBrowserModule from '../createStorageBrowser';
+
+import { createStorageBrowser } from '../createStorageBrowser';
 import * as CreateAmplifyAuthAdapter from '../adapters/createAmplifyAuthAdapter';
-import { StorageBrowser } from '../StorageBrowserAmplify';
 import { StorageBrowserDisplayText } from '../displayText/types';
 
-const createStorageBrowserSpy = jest.spyOn(
-  CreateStorageBrowserModule,
-  'createStorageBrowser'
-);
+import { StorageBrowser } from '../StorageBrowserAmplify';
+
+jest.mock('../createStorageBrowser');
+
+const TestComponent = jest.fn(() => 'StorageBrowser');
+
+const createStorageBrowserMock = (
+  createStorageBrowser as jest.Mock
+).mockReturnValue({ StorageBrowser: TestComponent });
 
 jest.spyOn(Amplify, 'getConfig').mockReturnValue({
-  Storage: {
-    S3: {
-      bucket: 'XXXXXX',
-      region: 'region',
-    },
-  },
+  Storage: { S3: { bucket: 'XXXXXX', region: 'region' } },
 });
 
 const createAmplifyAuthAdapterSpy = jest.spyOn(
@@ -29,29 +29,27 @@ describe('StorageBrowser', () => {
   beforeEach(() => {
     jest.clearAllMocks();
   });
-  it('calls `createStorageBrowser`', async () => {
-    await waitFor(() => {
-      render(<StorageBrowser />);
-    });
 
-    expect(createStorageBrowserSpy).toHaveBeenCalledTimes(1);
-    expect(createStorageBrowserSpy).toHaveBeenCalledWith({
+  it('calls `createStorageBrowser` and `createAmplifyAuthAdapter`', () => {
+    render(<StorageBrowser />);
+
+    expect(createStorageBrowserMock).toHaveBeenCalledTimes(1);
+    expect(createStorageBrowserMock).toHaveBeenCalledWith({
       config: expect.anything(),
     });
 
     expect(createAmplifyAuthAdapterSpy).toHaveBeenCalledTimes(1);
   });
 
-  it('support passing custom displayText', async () => {
+  it('provides the expected props to the `StorageBrowser` returned from `createStorageBrowser`', () => {
     const displayText: StorageBrowserDisplayText = {
       LocationsView: { title: 'Hello' },
     };
 
-    await waitFor(() => {
-      render(<StorageBrowser displayText={displayText} />);
-    });
+    const views = { LocationsView: jest.fn() };
+
+    render(<StorageBrowser displayText={displayText} views={views} />);
 
-    const Title = screen.getByText('Hello');
-    expect(Title).toBeInTheDocument();
+    expect(TestComponent).toHaveBeenCalledWith({ displayText, views }, {});
   });
 });

packages/react-storage/src/components/StorageBrowser/__tests__/createStorageBrowser.spec.tsx

@@ -32,6 +32,16 @@ const config = {
 const input = { config };
 
 describe('createStorageBrowser', () => {
+  beforeAll(() => {
+    // defining `crypto` here to allow `useDataState` to continue working as this test file
+    // is covering a fair amount of component code as a side effect. Not ideal and should be
+    // readdressed
+    let id = 0;
+    Object.defineProperty(globalThis, 'crypto', {
+      value: { randomUUID: () => ++id },
+    });
+  });
+
   it('throws when registerAuthListener is not a function', () => {
     const input = {
       config: { getLocationCredentials, listLocations, region },

packages/react-storage/src/components/StorageBrowser/useAction/__tests__/__snapshots__/useAction.spec.ts.snap

@@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`useAction throws the expected error when called with a type not mapped to an action handler 1`] = `"No handler found for value of \`unexpected!\` provided to \`useAction\`"`;
+
+exports[`useAction throws the expected error when called with the "listLocationItems" action type 1`] = `"Value of \`listLocationItems\` cannot be provided to \`useAction\`"`;
+
+exports[`useAction throws the expected error when called with the "listLocations" action type 1`] = `"Value of \`listLocations\` cannot be provided to \`useAction\`"`;