fix: race condition with pagination token

src/components/hooks/test/usePagination.test.tsx

@@ -0,0 +1,150 @@
+import {
+ fireEvent,
+ getByLabelText,
+ getByText,
+ render,
+ waitFor
+} from '@testing-library/react';
+import {
+ PaginatedEntityResponse,
+ RequestConfig
+} from 'models/AdminEntity/types';
+import * as React from 'react';
+import { PaginationConfig, usePagination } from '../usePagination';
+
+const valueLabel = 'pagination-value';
+const fetchLabel = 'pagination-doFetch';
+const moreItemsAvailableLabel = 'pagination-moreItemsAvailable';
+
+interface PaginationItem {
+ id: string;
+}
+
+type FetchResponse = PaginatedEntityResponse<PaginationItem>;
+interface PaginationTesterProps {
+ config: PaginationConfig<{}>;
+ doFetch: jest.Mock<Promise<FetchResponse>>;
+}
+
+const PaginationTester = ({ config, doFetch }: PaginationTesterProps) => {
+ const fetchable = usePagination(config, doFetch);
+ const onClickFetch = () => fetchable.fetch();
+
+ return (
+ <div>
+ <div aria-label={valueLabel}>
+ <ul>
+ {fetchable.value.map(({ id }) => (
+ <li key={`item-${id}`}>{`item-${id}`}</li>
+ ))}
+ </ul>
+ </div>
+ <div aria-label={moreItemsAvailableLabel}>
+ {fetchable.moreItemsAvailable ? 'true' : 'false'}
+ </div>
+ <button aria-label={fetchLabel} onClick={onClickFetch}>
+ Fetch Data
+ </button>
+ </div>
+ );
+};
+
+describe('usePagination', () => {
+ let entityCounter: number;
+ let config: PaginationConfig<{}>;
+ let doFetch: jest.Mock<Promise<FetchResponse>>;
+
+ beforeEach(() => {
+ entityCounter = 0;
+ doFetch = jest
+ .fn()
+ .mockImplementation(
+ (fetchArg: any, { limit = 25 }: RequestConfig) =>
+ Promise.resolve({
+ entities: Array.from({ length: limit }, () => {
+ const id = `${entityCounter}`;
+ entityCounter += 1;
+ return { id };
+ }),
+ token: `${entityCounter}`
+ })
+ );
+ config = {
+ cacheItems: false,
+ fetchArg: {},
+ limit: 25
+ };
+ });
+
+ const renderTester = () =>
+ render(<PaginationTester config={config} doFetch={doFetch} />);
+ const getElements = async (container: HTMLElement) => {
+ return waitFor(() => {
+ return {
+ fetchButton: getByLabelText(container, fetchLabel),
+ moreItemsAvailable: getByLabelText(
+ container,
+ moreItemsAvailableLabel
+ )
+ };
+ });
+ };
+
+ const waitForLastItemRendered = async (container: HTMLElement) => {
+ return waitFor(() => getByText(container, `item-${entityCounter - 1}`));
+ };
+
+ it('should pass returned token in subsequent calls', async () => {
+ const { container } = renderTester();
+ const { fetchButton } = await getElements(container);
+
+ expect(doFetch).toHaveBeenCalledWith(
+ expect.anything(),
+ expect.objectContaining({ token: '' })
+ );
+
+ fireEvent.click(fetchButton);
+ await waitForLastItemRendered(container);
+ expect(doFetch).toHaveBeenCalledWith(
+ expect.anything(),
+ expect.objectContaining({ token: `${config.limit}` })
+ );
+ });
+
+ it('should reset token when config changes', async () => {
+ const { container } = renderTester();
+ await getElements(container);
+
+ expect(doFetch).toHaveBeenCalledWith(
+ expect.anything(),
+ expect.objectContaining({ token: '' })
+ );
+
+ doFetch.mockClear();
+ entityCounter = 0;
+
+ // Change the config to trigger a rest of the pagination hook
+ config.limit = 10;
+ await getElements(renderTester().container);
+
+ expect(doFetch).toHaveBeenCalledWith(
+ expect.anything(),
+ expect.objectContaining({ token: '' })
+ );
+ });
+
+ it('should set moreItemsAvailable if token is returned', async () => {
+ const { moreItemsAvailable } = await getElements(
+ renderTester().container
+ );
+ expect(moreItemsAvailable.textContent).toBe('true');
+ });
+
+ it('should not set moreItemsAvailable if no token is returned', async () => {
+ doFetch.mockResolvedValue({ entities: [{ id: '0' }] });
+ const { moreItemsAvailable } = await getElements(
+ renderTester().container
+ );
+ expect(moreItemsAvailable.textContent).toBe('false');
+ });
+});

src/components/hooks/types.ts

@@ -56,7 +56,7 @@ export interface FetchableData<T> {
  debugName: string;
  fetch(): void;
  lastError: Error | null;
- state: FetchableState<T>;
+ state: FetchableState<any>;
  value: T;
 }
 
@@ -65,6 +65,10 @@ export interface FetchableExecution {
  terminateExecution(cause: string): Promise<void>;
 }
 
+export interface PaginationValue<T> {
+ token?: string;
+ items: T[];
+}
 export interface PaginatedFetchableData<T> extends FetchableData<T[]> {
  /** Whether or not a fetch would yield more items. Useful for determining if
  * a "load more" button should be shown

src/components/hooks/usePagination.ts

@@ -1,9 +1,12 @@
-import { useContext, useEffect, useState } from 'react';
-
-import { CacheContext, getCacheKey } from 'components/Cache';
+import { CacheContext } from 'components/Cache';
 import { RequestConfig } from 'models';
-
-import { FetchFn, PaginatedFetchableData, PaginatedFetchFn } from './types';
+import { useContext, useMemo } from 'react';
+import {
+ FetchFn,
+ PaginatedFetchableData,
+ PaginatedFetchFn,
+ PaginationValue
+} from './types';
 import { useFetchableData } from './useFetchableData';
 
 export interface PaginationConfig<FetchArgType> extends RequestConfig {
@@ -33,52 +36,46 @@ export function usePagination<T, FetchArgType>(
  doFetch: PaginatedFetchFn<T, FetchArgType>
 ): PaginatedFetchableData<T> {
  const { cacheItems = false, debugName } = config;
- const cacheKey = getCacheKey(config);
- const [token, setToken] = useState('');
- const [moreItemsAvailable, setMoreItemsAvailable] = useState(false);
  const cache = useContext(CacheContext);
 
- // Reset our state if the pagination config changes
- useEffect(() => {
- setToken('');
- setMoreItemsAvailable(false);
- }, [cacheKey]);
+ const fetch: FetchFn<
+ PaginationValue<T>,
+ PaginationConfig<FetchArgType>
+ > = useMemo(
+ () => async (params, currentValue) => {
+ const { token: previousToken = '', items: previousItems = [] } =
+ currentValue || {};
+ const { fetchArg, ...requestConfig } = params;
 
- const fetch: FetchFn<T[], PaginationConfig<FetchArgType>> = async (
- params,
- currentValue = []
- ) => {
- const { fetchArg, ...requestConfig } = params;
+ // If our last fetch call returned a token,
+ // we have to pass that along in order to retrieve the next page
+ const finalConfig = { ...requestConfig, token: previousToken };
 
- // If our last fetch call returned a token,
- // we have to pass that along in order to retrieve the next page
- if (token) {
- requestConfig.token = token;
- }
+ const { entities, token } = await doFetch(fetchArg, finalConfig);
+ const result = cacheItems ? cache.mergeArray(entities) : entities;
 
- const { entities, token: newToken } = await doFetch(
- fetchArg,
- requestConfig
- );
- const values = cacheItems ? cache.mergeArray(entities) : entities;
+ const items = previousItems.concat(result);
+ return {
+ items,
+ token
+ };
+ },
+ [cache, cacheItems]
+ );
 
- if (newToken) {
- setToken(newToken);
- }
- const newValue = currentValue.concat(values);
- setMoreItemsAvailable(!!newToken);
- return newValue;
- };
+ const fetchable = useFetchableData(
+ {
+ debugName,
+ defaultValue: { token: '', items: [] },
+ doFetch: fetch
+ },
+ config
+ );
 
+ const { items: value, token } = fetchable.value;
  return {
- ...useFetchableData(
- {
- debugName,
- defaultValue: [],
- doFetch: fetch
- },
- config
- ),
- moreItemsAvailable
+ ...fetchable,
+ value,
+ moreItemsAvailable: !!token
  };
 }