diff --git a/src/form/useOptions.test.ts b/src/form/useOptions.test.ts index 01924f96..4a6ff774 100644 --- a/src/form/useOptions.test.ts +++ b/src/form/useOptions.test.ts @@ -90,7 +90,7 @@ describe('useOptions', () => { size: 1, totalElements: 1, totalPages: 1, - content: [ { id: 1, label: 'Nederland' } ] + content: [{ id: 1, label: 'Nederland' }] }, loading: false }); @@ -102,7 +102,7 @@ describe('useOptions', () => { const options = generateOptions(); return useOptions({ - options: reloadOptions === 'all' ? options : [ options[0] ], + options: reloadOptions === 'all' ? options : [options[0]], value: undefined, labelForOption: (option: Option) => option.label, isOptionEqual: (a, b) => a.id === b.id, @@ -149,7 +149,69 @@ describe('useOptions', () => { size: 1, totalElements: 1, totalPages: 1, - content: [ { id: 1, label: '1' } ] + content: [{ id: 1, label: '1' }] + }, + loading: false + }); + }); + + it('should when the value changes update the options', () => { + const options = generateOptions(); + + const { result, rerender } = renderHook( + ({ value }) => { + return useOptions({ + options: options, + value, + labelForOption: (option: Option) => option.label, + isOptionEqual: (a, b) => a.id === b.id, + query: '', + pageNumber: 1, + size: 2, + optionsShouldAlwaysContainValue: true + }); + }, + { + initialProps: { + value: { id: -1, label: 'test' } + } + } + ); + + // Check if the value is prepended to the + expect(result.current).toEqual({ + page: { + first: true, + last: false, + number: 1, + numberOfElements: 2, + size: 2, + totalElements: 9, + totalPages: 5, + content: [ + { id: -1, label: 'test' }, + { id: 1, label: '1' }, + { id: 2, label: '2' } + ] + }, + loading: false + }); + + rerender({ value: options[0] }); + + expect(result.current).toEqual({ + page: { + first: true, + last: false, + number: 1, + numberOfElements: 2, + size: 2, + totalElements: 9, + totalPages: 5, + content: [ + { id: 1, label: '1' }, + { id: 2, label: '2' } + ] }, loading: false }); @@ -208,7 +270,7 @@ describe('useOptions', () => { size: 1, totalElements: 1, totalPages: 1, - content: [ { id: 1, label: '1' } ] + content: [{ id: 1, label: '1' }] }, loading: false }); @@ -611,9 +673,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: false }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); // Should start by loading expect(result.current).toEqual({ @@ -756,6 +816,96 @@ describe('useOptions', () => { expect(fetcher).toHaveBeenLastCalledWith({ page: 1, query: '', size: 2 }); }); + it('should when the value changes update the options but not re-fetch', async () => { + expect.assertions(6); + + const options = generateOptions(); + const { promise, resolve } = resolvablePromise>(); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const fetcher = jest.fn(({ size, page }) => { + return promise; + }); + + const { result, rerender } = renderHook( + ({ value }) => { + return useOptions({ + options: fetcher, + value, + labelForOption: (option: Option) => option.label, + isOptionEqual: (a, b) => a.id === b.id, + query: '', + pageNumber: 1, + size: 2, + optionsShouldAlwaysContainValue: true + }); + }, + { + initialProps: { + value: { id: -1, label: 'test' } + } + } + ); + + // Should start by loading + expect(result.current).toEqual({ + page: { + ...emptyPage(), + content: [{ id: -1, label: 'test' }] + }, + loading: true + }); + + await act(async () => { + await resolve(pageOf(options, 1, 2)); + }); + + // Check if the first page of 5 pages total is given including the value. + expect(result.current).toEqual({ + page: { + first: true, + last: false, + number: 1, + numberOfElements: 2, + size: 2, + totalElements: 9, + totalPages: 5, + content: [ + { id: -1, label: 'test' }, + { id: 1, label: '1' }, + { id: 2, label: '2' } + ] + }, + loading: false + }); + + expect(fetcher).toBeCalledTimes(1); + expect(fetcher).toBeCalledWith({ page: 1, query: '', size: 2 }); + + rerender({ value: options[0] }); + + // Should remove the old value from the options + expect(result.current).toEqual({ + page: { + first: true, + last: false, + number: 1, + numberOfElements: 2, + size: 2, + totalElements: 9, + totalPages: 5, + content: [ + { id: 1, label: '1' }, + { id: 2, label: '2' } + ] + }, + loading: false + }); + + // Should not have called the options fetcher again + expect(fetcher).toBeCalledTimes(1); + }); + it('should when the query changes re-fetch the options and go back in loading state', async () => { expect.assertions(8); @@ -873,10 +1023,13 @@ describe('useOptions', () => { expect.assertions(8); const options = generateOptions(); - const { promise: promise1, resolve: resolve1 } = resolvablePromise>(); - const { promise: promise2, resolve: resolve2 } = resolvablePromise>(); + const { promise: promise1, resolve: resolve1 } = + resolvablePromise>(); + const { promise: promise2, resolve: resolve2 } = + resolvablePromise>(); - const fetcher = jest.fn() + const fetcher = jest + .fn() .mockImplementationOnce(() => promise1) .mockImplementationOnce(() => promise2); @@ -982,10 +1135,13 @@ describe('useOptions', () => { expect.assertions(8); const options = generateOptions(); - const { promise: promise1, resolve: resolve1 } = resolvablePromise>(); - const { promise: promise2, resolve: resolve2 } = resolvablePromise>(); + const { promise: promise1, resolve: resolve1 } = + resolvablePromise>(); + const { promise: promise2, resolve: resolve2 } = + resolvablePromise>(); - const fetcher = jest.fn() + const fetcher = jest + .fn() .mockImplementationOnce(() => promise1) .mockImplementationOnce(() => promise2); @@ -1110,9 +1266,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: false }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); await act(async () => { await resolve(pageOf(generateOptions(), 1, 2)); @@ -1158,9 +1312,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: true }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); await act(async () => { await resolve(pageOf(generateOptions(), 1, 2)); @@ -1206,9 +1358,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: true }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); await act(async () => { await resolve(pageOf(generateOptions(), 1, 2)); @@ -1254,9 +1404,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: true }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); await act(async () => { await resolve(pageOf(generateOptions(), 1, 2)); @@ -1308,9 +1456,7 @@ describe('useOptions', () => { optionsShouldAlwaysContainValue: true }; - const { result } = renderHook(() => - useOptions(config) - ); + const { result } = renderHook(() => useOptions(config)); await act(async () => { await resolve(pageOf(generateOptions(), 1, 2)); diff --git a/src/form/useOptions.ts b/src/form/useOptions.ts index 0bfa5b6d..435bb1c7 100644 --- a/src/form/useOptions.ts +++ b/src/form/useOptions.ts @@ -1,7 +1,11 @@ import { emptyPage, Page } from '@42.nl/spring-connect'; -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import { pageOf } from '../utilities/page/page'; -import { FetchOptionsCallback, FieldCompatibleWithPredeterminedOptions, isOptionSelected } from './option'; +import { + FetchOptionsCallback, + FieldCompatibleWithPredeterminedOptions, + isOptionSelected +} from './option'; type UseOptionConfig = FieldCompatibleWithPredeterminedOptions & { value?: T | T[]; @@ -30,15 +34,13 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { optionsShouldAlwaysContainValue } = config; - const [ loading, setLoading ] = useState( + const [loading, setLoading] = useState( () => !Array.isArray(optionsOrFetcher) ); - const [ page, setPage ] = useState(() => { + const [pageFromOptions, setPageFromOptions] = useState(() => { if (Array.isArray(optionsOrFetcher)) { - const page = pageFromOptionsArray(optionsOrFetcher); - appendValueToPage(page); - return page; + return pageFromOptionsArray(optionsOrFetcher); } else { return emptyPage(); } @@ -61,7 +63,7 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { // When the options are loaded make sure that the options always // contain the value that the user has selected when // `optionsShouldAlwaysContainValue` is `true`. - function appendValueToPage(page: Page): void { + function appendValueToPage(page: Page): Page { /* When a form component shows the selected value in one place, but allows the user to select values from the another place @@ -73,12 +75,12 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { visible. */ if (!optionsShouldAlwaysContainValue) { - return; + return page; } // If there is no value there is no need to add it if (!value) { - return; + return page; } if (Array.isArray(value)) { @@ -97,7 +99,7 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { return !isValueIncludedInPage; }); - page.content = [ ...prepend, ...page.content ]; + return { ...page, content: [...prepend, ...page.content] }; } else { const isValueIncludedInPage = page.content.some((option) => isOptionSelected({ @@ -111,10 +113,10 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { // If the value is in the page there is no need to add it if (isValueIncludedInPage) { - return; + return page; } - page.content.unshift(value); + return { ...page, content: [value, ...page.content] }; } } @@ -132,8 +134,7 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { size }); - appendValueToPage(resultPage); - setPage(resultPage); + setPageFromOptions(resultPage); setLoading(false); } @@ -143,13 +144,18 @@ export function useOptions(config: UseOptionConfig): UseOptionResult { } else if (lastWatch.current !== watch) { lastWatch.current = watch; - const newPage = pageFromOptionsArray(optionsOrFetcher); - appendValueToPage(newPage); - setPage(newPage); + setPageFromOptions(pageFromOptionsArray(optionsOrFetcher)); } // We only want to reload when the following props change // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ watch ]); + }, [watch]); + + const page = useMemo( + () => appendValueToPage(pageFromOptions), + // We only want to reload when the following props change + // eslint-disable-next-line react-hooks/exhaustive-deps + [value, pageFromOptions] + ); return { loading, page }; }