diff --git a/package-lock.json b/package-lock.json index 39eedf1763..e03fc22de5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -61,6 +61,7 @@ "@playwright/test": "^1.30.0", "@testing-library/jest-dom": "^5.16.4", "@testing-library/react": "^12.1.3", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "^14.4.3", "@types/bootstrap": "^4.4.1", "@types/color-convert": "^2.0.0", @@ -26708,12 +26709,12 @@ "@types/js-cookie": "^3.0.3", "classnames": "^2.3.2", "js-cookie": "^3.0.5", + "lodash.debounce": "^4.0.8", "prop-types": "^15.8.1" }, "devDependencies": { "@deephaven/jsapi-shim": "file:../jsapi-shim", "@deephaven/tsconfig": "file:../tsconfig", - "@testing-library/react-hooks": "^8.0.1", "react-test-renderer": "^17.0.2" }, "engines": { @@ -26858,7 +26859,8 @@ "license": "Apache-2.0", "dependencies": { "@deephaven/log": "file:../log", - "@deephaven/utils": "file:../utils" + "@deephaven/utils": "file:../utils", + "shortid": "^2.2.16" }, "devDependencies": { "@deephaven/tsconfig": "file:../tsconfig" @@ -28726,10 +28728,10 @@ "@deephaven/tsconfig": "file:../tsconfig", "@deephaven/utils": "file:../utils", "@react-stately/data": "^3.9.1", - "@testing-library/react-hooks": "^8.0.1", "@types/js-cookie": "^3.0.3", "classnames": "^2.3.2", "js-cookie": "^3.0.5", + "lodash.debounce": "^4.0.8", "prop-types": "^15.8.1", "react-test-renderer": "^17.0.2" } @@ -28796,7 +28798,8 @@ "requires": { "@deephaven/log": "file:../log", "@deephaven/tsconfig": "file:../tsconfig", - "@deephaven/utils": "file:../utils" + "@deephaven/utils": "file:../utils", + "shortid": "^2.2.16" } }, "@deephaven/redux": { diff --git a/package.json b/package.json index 87fa6bdd4f..5a61fbd4b2 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "@playwright/test": "^1.30.0", "@testing-library/jest-dom": "^5.16.4", "@testing-library/react": "^12.1.3", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "^14.4.3", "@types/bootstrap": "^4.4.1", "@types/color-convert": "^2.0.0", diff --git a/packages/jsapi-components/package.json b/packages/jsapi-components/package.json index ea8075cf03..2ceadd1395 100644 --- a/packages/jsapi-components/package.json +++ b/packages/jsapi-components/package.json @@ -29,6 +29,7 @@ "@deephaven/log": "file:../log", "@deephaven/react-hooks": "file:../react-hooks", "@deephaven/utils": "file:../utils", + "lodash.debounce": "^4.0.8", "@react-stately/data": "^3.9.1", "@types/js-cookie": "^3.0.3", "classnames": "^2.3.2", @@ -38,7 +39,6 @@ "devDependencies": { "@deephaven/jsapi-shim": "file:../jsapi-shim", "@deephaven/tsconfig": "file:../tsconfig", - "@testing-library/react-hooks": "^8.0.1", "react-test-renderer": "^17.0.2" }, "peerDependencies": { diff --git a/packages/jsapi-components/src/index.ts b/packages/jsapi-components/src/index.ts index 0b490a9a6b..5fe407b4f9 100644 --- a/packages/jsapi-components/src/index.ts +++ b/packages/jsapi-components/src/index.ts @@ -3,10 +3,15 @@ export * from './RefreshTokenBootstrap'; export * from './RefreshTokenUtils'; export { default as useBroadcastChannel } from './useBroadcastChannel'; export { default as useBroadcastLoginListener } from './useBroadcastLoginListener'; +export { default as useDebouncedViewportSearch } from './useDebouncedViewportSearch'; export { default as useInitializeViewportData } from './useInitializeViewportData'; export { default as useTable } from './useTable'; export { default as useTableColumn } from './useTableColumn'; export { default as useTableListener } from './useTableListener'; export { default as useSelectDistinctTable } from './useSelectDistinctTable'; export { default as useSetPaddedViewportCallback } from './useSetPaddedViewportCallback'; +export { default as useTableClose } from './useTableClose'; +export { default as useTableSize } from './useTableSize'; export { default as useViewportData } from './useViewportData'; +export type { UseSelectDistinctTableResult } from './useSelectDistinctTable'; +export type { UseViewportDataResult } from './useViewportData'; diff --git a/packages/jsapi-components/src/useDebouncedViewportSearch.test.ts b/packages/jsapi-components/src/useDebouncedViewportSearch.test.ts new file mode 100644 index 0000000000..76e32e39da --- /dev/null +++ b/packages/jsapi-components/src/useDebouncedViewportSearch.test.ts @@ -0,0 +1,132 @@ +import { renderHook } from '@testing-library/react-hooks'; +import type { + Column, + FilterCondition, + FilterValue, + Table, +} from '@deephaven/jsapi-types'; +import { TableUtils } from '@deephaven/jsapi-utils'; +import { TestUtils } from '@deephaven/utils'; +import useDebouncedViewportSearch, { + DEBOUNCE_VIEWPORT_SEARCH_MS, +} from './useDebouncedViewportSearch'; +import { UseViewportDataResult } from './useViewportData'; + +// Mock js api objects +const column = TestUtils.createMockProxy({ + type: 'java.lang.String', +}); +const filterCondition = TestUtils.createMockProxy({}); +const columnFilterValue = TestUtils.createMockProxy({}); +const matchFilterValue = TestUtils.createMockProxy({}); +const table = TestUtils.createMockProxy({}); +const viewportData = TestUtils.createMockProxy< + UseViewportDataResult +>({ table }); + +beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + + TestUtils.asMock(column.filter).mockReturnValue(columnFilterValue); + TestUtils.asMock(columnFilterValue.contains).mockReturnValue(filterCondition); + TestUtils.asMock(table.findColumn).mockReturnValue(column); + + jest.spyOn(TableUtils, 'makeFilterValue').mockReturnValue(matchFilterValue); +}); + +it.each([undefined, 400])( + 'should return a function that debounces search filtering', + debounceMs => { + const searchText = 'mock.searchText'; + + const { result } = renderHook(() => + useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) + ); + + result.current(searchText); + + expect(viewportData.applyFiltersAndRefresh).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(debounceMs ?? DEBOUNCE_VIEWPORT_SEARCH_MS); + + expect(TableUtils.makeFilterValue).toHaveBeenCalledWith( + column.type, + searchText + ); + expect(viewportData.applyFiltersAndRefresh).toHaveBeenCalledWith([ + filterCondition, + ]); + } +); + +it('should return a function that safely ignores no table', () => { + const viewportNoTable = TestUtils.createMockProxy< + UseViewportDataResult + >({ table: null }); + + const searchText = 'mock.searchText'; + const debounceMs = 400; + + const { result } = renderHook(() => + useDebouncedViewportSearch(viewportNoTable, 'mock.column', debounceMs) + ); + + result.current(searchText); + jest.advanceTimersByTime(debounceMs); + + expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); + expect(viewportNoTable.applyFiltersAndRefresh).not.toHaveBeenCalled(); +}); + +it('should trim search text', () => { + const searchText = ' mock.searchText '; + const trimmedSearchText = 'mock.searchText'; + const debounceMs = 400; + + const { result } = renderHook(() => + useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) + ); + + result.current(searchText); + jest.advanceTimersByTime(debounceMs); + + expect(TableUtils.makeFilterValue).toHaveBeenCalledWith( + column.type, + trimmedSearchText + ); +}); + +it.each(['', ' '])( + 'should pass apply empty filter if search text is empty', + searchText => { + const debounceMs = 400; + + const { result } = renderHook(() => + useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) + ); + + result.current(searchText); + jest.advanceTimersByTime(debounceMs); + + expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); + expect(viewportData.applyFiltersAndRefresh).toHaveBeenCalledWith([]); + } +); + +it('should cancel debounce on unmount', () => { + const searchText = 'mock.searchText'; + const debounceMs = 400; + + const { result, unmount } = renderHook(() => + useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) + ); + + result.current(searchText); + jest.advanceTimersByTime(5); + unmount(); + jest.advanceTimersByTime(debounceMs); + + expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); + expect(viewportData.applyFiltersAndRefresh).not.toHaveBeenCalled(); +}); diff --git a/packages/jsapi-components/src/useDebouncedViewportSearch.ts b/packages/jsapi-components/src/useDebouncedViewportSearch.ts new file mode 100644 index 0000000000..ec70093da1 --- /dev/null +++ b/packages/jsapi-components/src/useDebouncedViewportSearch.ts @@ -0,0 +1,58 @@ +import debounce from 'lodash.debounce'; +import type { Table, TreeTable } from '@deephaven/jsapi-types'; +import { TableUtils } from '@deephaven/jsapi-utils'; +import { useEffect, useMemo } from 'react'; +import { UseViewportDataResult } from './useViewportData'; + +export const DEBOUNCE_VIEWPORT_SEARCH_MS = 200; + +/** + * React hook that returns a debounced search callback for filtering a table + * viewport. + * @param viewportData Table viewport to filter + * @param columnName Column name to filter by + * @param debounceMs Millisecond value to debounce + */ +export default function useDebouncedViewportSearch< + I, + T extends Table | TreeTable +>( + viewportData: UseViewportDataResult, + columnName: string, + debounceMs = DEBOUNCE_VIEWPORT_SEARCH_MS +): (searchText: string) => void { + const debouncedSearch = useMemo( + () => + debounce((searchText: string) => { + if (viewportData.table == null) { + return; + } + + const searchTextTrimmed = searchText.trim(); + + if (searchTextTrimmed === '') { + viewportData.applyFiltersAndRefresh([]); + return; + } + + const column = viewportData.table.findColumn(columnName); + const value = TableUtils.makeFilterValue( + column.type, + searchTextTrimmed + ); + const filter = [column.filter().contains(value)]; + + viewportData.applyFiltersAndRefresh(filter); + }, debounceMs), + [columnName, debounceMs, viewportData] + ); + + useEffect( + () => () => { + debouncedSearch.cancel(); + }, + [debouncedSearch] + ); + + return debouncedSearch; +} diff --git a/packages/jsapi-components/src/useInitializeViewportData.test.ts b/packages/jsapi-components/src/useInitializeViewportData.test.ts index d5323c0649..99a1b19642 100644 --- a/packages/jsapi-components/src/useInitializeViewportData.test.ts +++ b/packages/jsapi-components/src/useInitializeViewportData.test.ts @@ -1,8 +1,11 @@ import { act, renderHook } from '@testing-library/react-hooks'; -import { Table } from '@deephaven/jsapi-shim'; +import type { Table } from '@deephaven/jsapi-types'; import { KeyedItem } from '@deephaven/jsapi-utils'; import { TestUtils } from '@deephaven/utils'; import useInitializeViewportData from './useInitializeViewportData'; +import useTableSize from './useTableSize'; + +jest.mock('./useTableSize'); const tableA = TestUtils.createMockProxy
({ size: 4 }); const expectedInitialA: KeyedItem[] = [ @@ -15,6 +18,15 @@ const expectedInitialA: KeyedItem[] = [ const tableB = TestUtils.createMockProxy
({ size: 2 }); const expectedInitialB = [{ key: '0' }, { key: '1' }]; +beforeEach(() => { + TestUtils.asMock(useTableSize).mockImplementation(table => table?.size ?? 0); +}); + +it.each([null])('should safely handle no table: %s', noTable => { + const { result } = renderHook(() => useInitializeViewportData(noTable)); + expect(result.current.items).toEqual([]); +}); + it('should initialize a ListData object based on Table size', () => { const { result } = renderHook(() => useInitializeViewportData(tableA)); @@ -43,3 +55,26 @@ it('should re-initialize a ListData object if Table reference changes', () => { expect(result.current.items).toEqual(expectedInitialB); }); + +it.each([ + [3, [...expectedInitialA.slice(0, -1)]], + [5, [...expectedInitialA, { key: '4' }]], +])( + 'should re-initialize a ListData object if Table size changes', + (newSize, expectedAfterSizeChange) => { + const { result, rerender } = renderHook( + ({ table }) => useInitializeViewportData(table), + { + initialProps: { table: tableA }, + } + ); + + expect(result.current.items).toEqual(expectedInitialA); + + // Re-render with new size + TestUtils.asMock(useTableSize).mockImplementation(() => newSize); + rerender({ table: tableA }); + + expect(result.current.items).toEqual(expectedAfterSizeChange); + } +); diff --git a/packages/jsapi-components/src/useInitializeViewportData.ts b/packages/jsapi-components/src/useInitializeViewportData.ts index 6b6ce2e1bc..4af1a433ad 100644 --- a/packages/jsapi-components/src/useInitializeViewportData.ts +++ b/packages/jsapi-components/src/useInitializeViewportData.ts @@ -1,11 +1,9 @@ import { useEffect } from 'react'; import { ListData, useListData } from '@react-stately/data'; -import { Table, TreeTable } from '@deephaven/jsapi-types'; -import { - KeyedItem, - generateEmptyKeyedItems, - getSize, -} from '@deephaven/jsapi-utils'; +import type { Table, TreeTable } from '@deephaven/jsapi-types'; +import { KeyedItem, generateEmptyKeyedItems } from '@deephaven/jsapi-utils'; +import { usePrevious } from '@deephaven/react-hooks'; +import useTableSize from './useTableSize'; /** * Initializes a ListData instance that can be used for windowed views of a @@ -24,23 +22,40 @@ export default function useInitializeViewportData( ): ListData> { const viewportData = useListData>({}); + const prevTable = usePrevious(table); + + // If the table changes size, we need to re-initialize it. + const size = Math.max(0, useTableSize(table)); + // We only want this to fire 1x once the table exists. Note that `useListData` // has no way to respond to a reference change of the `table` instance so we // have to manually delete any previous keyed items from the list. useEffect(() => { + let currentSize = viewportData.items.length; + + // If our table instance has changed, we want to clear all items from state + if (table !== prevTable && currentSize) { + viewportData.remove(...viewportData.items.map(({ key }) => key)); + currentSize = 0; + } + if (!table) { return; } - if (viewportData.items.length) { - viewportData.remove(...viewportData.items.map(({ key }) => key)); + if (size > currentSize) { + viewportData.insert( + currentSize, + ...generateEmptyKeyedItems(currentSize, size - 1) + ); + } else if (size < currentSize) { + const keys = viewportData.items.slice(size).map(({ key }) => key); + viewportData.remove(...keys); } - viewportData.insert(0, ...generateEmptyKeyedItems(getSize(table))); - // Intentionally excluding viewportData since it changes on every render. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [table]); + }, [size, table]); return viewportData; } diff --git a/packages/jsapi-components/src/useSelectDistinctTable.test.ts b/packages/jsapi-components/src/useSelectDistinctTable.test.ts index d79186e931..17636f5d01 100644 --- a/packages/jsapi-components/src/useSelectDistinctTable.test.ts +++ b/packages/jsapi-components/src/useSelectDistinctTable.test.ts @@ -1,4 +1,4 @@ -import { Table } from '@deephaven/jsapi-shim'; +import type { Table } from '@deephaven/jsapi-types'; import { TestUtils } from '@deephaven/utils'; import { renderHook } from '@testing-library/react-hooks'; import useSelectDistinctTable from './useSelectDistinctTable'; @@ -12,7 +12,7 @@ beforeEach(() => { table = TestUtils.createMockProxy
(); derivedTable = TestUtils.createMockProxy
(); - (table.selectDistinct as jest.Mock).mockResolvedValue(derivedTable); + TestUtils.asMock(table.selectDistinct).mockResolvedValue(derivedTable); }); it('should create and subscribe to a `selectDistinct` derivation of a given table', async () => { diff --git a/packages/jsapi-components/src/useSelectDistinctTable.ts b/packages/jsapi-components/src/useSelectDistinctTable.ts index 102e730e2f..bb5b63728d 100644 --- a/packages/jsapi-components/src/useSelectDistinctTable.ts +++ b/packages/jsapi-components/src/useSelectDistinctTable.ts @@ -2,6 +2,16 @@ import { useCallback, useEffect } from 'react'; import { Table, TreeTable } from '@deephaven/jsapi-types'; import { usePromiseFactory } from '@deephaven/react-hooks'; +/** + * Return type of `useSelectDistinctTable` hook. + */ +export interface UseSelectDistinctTableResult { + distinctTable: Table | null; + error: string | Error | null; + isError: boolean; + isLoading: boolean; +} + /** * Creates and subscribes to a `selectDistinct` derived table and unsubscribes * on unmount. @@ -11,7 +21,7 @@ import { usePromiseFactory } from '@deephaven/react-hooks'; export default function useSelectDistinctTable( table: Table | TreeTable | null, ...columnNames: string[] -) { +): UseSelectDistinctTableResult { const selectDistinct = useCallback( async () => table?.selectDistinct(table.findColumns(columnNames)) ?? null, // Disabling the exhaustive checks due to the spreading of `columnNames` diff --git a/packages/jsapi-components/src/useSetPaddedViewportCallback.test.ts b/packages/jsapi-components/src/useSetPaddedViewportCallback.test.ts index 312541a1e0..d81ba63560 100644 --- a/packages/jsapi-components/src/useSetPaddedViewportCallback.test.ts +++ b/packages/jsapi-components/src/useSetPaddedViewportCallback.test.ts @@ -1,5 +1,5 @@ import { renderHook } from '@testing-library/react-hooks'; -import { Table } from '@deephaven/jsapi-shim'; +import type { Table } from '@deephaven/jsapi-types'; import { TestUtils } from '@deephaven/utils'; import useSetPaddedViewportCallback from './useSetPaddedViewportCallback'; diff --git a/packages/jsapi-components/src/useSetPaddedViewportCallback.ts b/packages/jsapi-components/src/useSetPaddedViewportCallback.ts index 18eabfdf08..d96721200a 100644 --- a/packages/jsapi-components/src/useSetPaddedViewportCallback.ts +++ b/packages/jsapi-components/src/useSetPaddedViewportCallback.ts @@ -25,6 +25,7 @@ export default function useSetPaddedViewportCallback( viewportPadding, getSize(table) ); + table?.setViewport(first, last); }, [table, viewportPadding, viewportSize] diff --git a/packages/jsapi-components/src/useTableClose.test.ts b/packages/jsapi-components/src/useTableClose.test.ts new file mode 100644 index 0000000000..65cf161250 --- /dev/null +++ b/packages/jsapi-components/src/useTableClose.test.ts @@ -0,0 +1,49 @@ +import { renderHook } from '@testing-library/react-hooks'; +import type { Table } from '@deephaven/jsapi-types'; +import { TestUtils } from '@deephaven/utils'; +import useTableClose from './useTableClose'; + +const table = TestUtils.createMockProxy
({}); +const closedTable = TestUtils.createMockProxy
({ isClosed: true }); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('should close table on unmount', () => { + const { unmount } = renderHook(() => useTableClose(table)); + + expect(table.close).not.toHaveBeenCalled(); + + unmount(); + + expect(table.close).toHaveBeenCalled(); +}); + +it.each([closedTable, null, undefined])( + 'should not call close if no table given or table already closed', + maybeTable => { + const { unmount } = renderHook(() => useTableClose(maybeTable)); + + unmount(); + + if (maybeTable) { + expect(maybeTable.close).not.toHaveBeenCalled(); + } + } +); + +it('should close previous table if reference changes', () => { + const nextTable = TestUtils.createMockProxy
({}); + + const { rerender } = renderHook(t => useTableClose(t), { + initialProps: table, + }); + + expect(table.close).not.toHaveBeenCalled(); + + rerender(nextTable); + + expect(table.close).toHaveBeenCalled(); + expect(nextTable.close).not.toHaveBeenCalled(); +}); diff --git a/packages/jsapi-components/src/useTableClose.ts b/packages/jsapi-components/src/useTableClose.ts new file mode 100644 index 0000000000..5b3da29c5e --- /dev/null +++ b/packages/jsapi-components/src/useTableClose.ts @@ -0,0 +1,25 @@ +import { useEffect } from 'react'; +import type { Table, TreeTable } from '@deephaven/jsapi-types'; +import { isClosed } from '@deephaven/jsapi-utils'; + +/** + * React hook that closes a given table when the reference changes or when the + * component unmounts. + * @param table + */ +export default function useTableClose( + table: Table | TreeTable | null | undefined +): void { + useEffect( + () => () => { + if (table == null) { + return; + } + + if (!isClosed(table)) { + table.close(); + } + }, + [table] + ); +} diff --git a/packages/jsapi-components/src/useTableSize.test.ts b/packages/jsapi-components/src/useTableSize.test.ts new file mode 100644 index 0000000000..8584faccb4 --- /dev/null +++ b/packages/jsapi-components/src/useTableSize.test.ts @@ -0,0 +1,50 @@ +import { act, renderHook } from '@testing-library/react-hooks'; +import { Table } from '@deephaven/jsapi-shim'; +import { TestUtils } from '@deephaven/utils'; +import useTableSize from './useTableSize'; +import useTableListener from './useTableListener'; + +jest.mock('./useTableListener'); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it.each([null, undefined])('should return 0 if no table', table => { + const { result } = renderHook(() => useTableSize(table)); + expect(result.current).toEqual(0); +}); + +it('should return the size of the given table', () => { + const size = 10; + const table = TestUtils.createMockProxy
({ size }); + + const { result } = renderHook(() => useTableSize(table)); + + expect(result.current).toEqual(size); +}); + +it('should re-render if dh.Table.EVENT_SIZECHANGED event occurs', () => { + const initialSize = 10; + const table = ({ + addEventListener: jest.fn(), + size: initialSize, + } as unknown) as Table; + + const { result } = renderHook(() => useTableSize(table)); + + const [eventEmitter, eventName, onSizeChangeHandler] = + TestUtils.extractCallArgs(useTableListener, 0) ?? []; + + expect(eventEmitter).toBe(table); + expect(eventName).toEqual(dh.Table.EVENT_SIZECHANGED); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (table as any).size = 4; + + act(() => { + onSizeChangeHandler?.({} as CustomEvent); + }); + + expect(result.current).toEqual(4); +}); diff --git a/packages/jsapi-components/src/useTableSize.ts b/packages/jsapi-components/src/useTableSize.ts new file mode 100644 index 0000000000..6bdce430db --- /dev/null +++ b/packages/jsapi-components/src/useTableSize.ts @@ -0,0 +1,23 @@ +import { useState } from 'react'; +import type { Table, TreeTable } from '@deephaven/jsapi-types'; +import { getSize } from '@deephaven/jsapi-utils'; +import useTableListener from './useTableListener'; + +/** + * React hook that returns the size of a given table or zero if table is null or + * undefined. The hook subscribes to the dh.Table.EVENT_SIZECHANGED event and + * triggers a re-render if any events are received to ensure we have the current + * size. + * @param table The table to check the size on. + */ +export default function useTableSize( + table: Table | TreeTable | null | undefined +): number { + const [, forceRerender] = useState(0); + + useTableListener(table, dh.Table.EVENT_SIZECHANGED, () => { + forceRerender(i => i + 1); + }); + + return getSize(table); +} diff --git a/packages/jsapi-components/src/useViewportData.test.ts b/packages/jsapi-components/src/useViewportData.test.ts index 9479f7bfed..791bdf7c8c 100644 --- a/packages/jsapi-components/src/useViewportData.test.ts +++ b/packages/jsapi-components/src/useViewportData.test.ts @@ -1,13 +1,16 @@ import { act, renderHook } from '@testing-library/react-hooks'; -import { Table } from '@deephaven/jsapi-shim'; +import type { FilterCondition, Table } from '@deephaven/jsapi-types'; import { OnTableUpdatedEvent, ViewportRow, generateEmptyKeyedItems, } from '@deephaven/jsapi-utils'; import { TestUtils } from '@deephaven/utils'; +import useTableSize from './useTableSize'; import useViewportData, { UseViewportDataProps } from './useViewportData'; +jest.mock('./useTableSize'); + function mockViewportRow(offsetInSnapshot: number): ViewportRow { return { offsetInSnapshot } as ViewportRow; } @@ -29,7 +32,7 @@ function getLastRegisteredEventHandler( table: Table, eventName: string ): ((event: OnTableUpdatedEvent) => void) | undefined { - const { calls } = (table.addEventListener as jest.Mock).mock; + const { calls } = TestUtils.asMock(table.addEventListener).mock; const [lastCall] = calls.filter(call => call[0] === eventName).slice(-1); return lastCall?.[1]; } @@ -39,19 +42,20 @@ const viewportSize = 10; const viewportPadding = 4; const deserializeRow = jest.fn().mockImplementation(row => row); -const options: UseViewportDataProps = { +const options: UseViewportDataProps = { table, viewportSize, viewportPadding, deserializeRow, }; -const optionsUseDefaults: UseViewportDataProps = { +const optionsUseDefaults: UseViewportDataProps = { table, }; beforeEach(() => { jest.clearAllMocks(); + TestUtils.asMock(useTableSize).mockImplementation(t => t?.size ?? 0); }); it.each([options, optionsUseDefaults])( @@ -60,7 +64,7 @@ it.each([options, optionsUseDefaults])( const { result } = renderHook(() => useViewportData(opt)); const expected = { - initialItems: [...generateEmptyKeyedItems(table.size)], + initialItems: [...generateEmptyKeyedItems(0, table.size - 1)], viewportEnd: (opt.viewportSize ?? 10) + (opt.viewportPadding ?? 50) - 1, }; @@ -70,6 +74,59 @@ it.each([options, optionsUseDefaults])( } ); +it('should return table', () => { + const { result } = renderHook(() => useViewportData(options)); + expect(result.current.table).toBe(options.table); +}); + +it('should return a callback that can apply filters and refresh viewport', () => { + const { result } = renderHook(() => useViewportData(options)); + jest.clearAllMocks(); + + const filters: FilterCondition[] = []; + + result.current.applyFiltersAndRefresh(filters); + + expect(options.table?.applyFilter).toHaveBeenCalledWith(filters); + expect(options.table?.setViewport).toHaveBeenCalledWith( + 0, + options.viewportSize! + options.viewportPadding! - 1 + ); +}); + +it('should set viewport if table size changes', () => { + const { rerender } = renderHook(() => useViewportData(options)); + jest.clearAllMocks(); + + rerender(); + expect(options.table?.setViewport).not.toHaveBeenCalled(); + + // Change table size + TestUtils.asMock(useTableSize).mockReturnValue(table.size - 5); + + rerender(); + expect(options.table?.setViewport).toHaveBeenCalled(); +}); + +it('should set viewport if table reference changes', () => { + const { rerender } = renderHook( + t => useViewportData({ ...options, table: t }), + { + initialProps: table, + } + ); + jest.clearAllMocks(); + + rerender(table); + expect(table.setViewport).not.toHaveBeenCalled(); + + // pass a new Table reference + const table2 = TestUtils.createMockProxy
({ size: table.size }); + rerender(table2); + + expect(table2.setViewport).toHaveBeenCalled(); +}); + it('should update state on dh.Table.EVENT_UPDATED event', () => { const { result } = renderHook(() => useViewportData(options)); @@ -89,7 +146,7 @@ it('should update state on dh.Table.EVENT_UPDATED event', () => { }); const expectedKeyIndex = offset + row.offsetInSnapshot; - const expectedInitialItems = [...generateEmptyKeyedItems(table.size)]; + const expectedInitialItems = [...generateEmptyKeyedItems(0, table.size - 1)]; const expectedItems = [ ...expectedInitialItems.slice(0, expectedKeyIndex), { diff --git a/packages/jsapi-components/src/useViewportData.ts b/packages/jsapi-components/src/useViewportData.ts index 1e8dd6a398..cc2f83501a 100644 --- a/packages/jsapi-components/src/useViewportData.ts +++ b/packages/jsapi-components/src/useViewportData.ts @@ -1,30 +1,37 @@ -import { useEffect } from 'react'; +import { useCallback, useEffect } from 'react'; import { ListData } from '@react-stately/data'; -import { Table, TreeTable } from '@deephaven/jsapi-types'; +import type { FilterCondition, Table, TreeTable } from '@deephaven/jsapi-types'; import { KeyedItem, RowDeserializer, createOnTableUpdatedHandler, defaultRowDeserializer, - getSize, isClosed, } from '@deephaven/jsapi-utils'; import useInitializeViewportData from './useInitializeViewportData'; import useSetPaddedViewportCallback from './useSetPaddedViewportCallback'; import useTableListener from './useTableListener'; +import useTableSize from './useTableSize'; -export interface UseViewportDataProps { - table: Table | TreeTable | null; +export interface UseViewportDataProps { + table: TTable | null; viewportSize?: number; viewportPadding?: number; - deserializeRow?: RowDeserializer; + deserializeRow?: RowDeserializer; } -export interface UseViewportDataResult { +export interface UseViewportDataResult< + TItem, + TTable extends Table | TreeTable +> { /** Manages deserialized row items associated with a DH Table */ - viewportData: ListData>; + viewportData: ListData>; /** Size of the underlying Table */ size: number; + + table: TTable | null; + /** Apply filters and refresh viewport. */ + applyFiltersAndRefresh: (filters: FilterCondition[]) => void; /** Set the viewport of the Table */ setViewport: (firstRow: number) => void; } @@ -42,13 +49,16 @@ export interface UseViewportDataResult { * @param viewportPadding * @returns An object for managing Table viewport state. */ -export default function useViewportData({ +export default function useViewportData< + TItem, + TTable extends Table | TreeTable +>({ table, viewportSize = 10, viewportPadding = 50, deserializeRow = defaultRowDeserializer, -}: UseViewportDataProps): UseViewportDataResult { - const viewportData = useInitializeViewportData(table); +}: UseViewportDataProps): UseViewportDataResult { + const viewportData = useInitializeViewportData(table); const setViewport = useSetPaddedViewportCallback( table, @@ -56,23 +66,35 @@ export default function useViewportData({ viewportPadding ); + const applyFiltersAndRefresh = useCallback( + (filters: FilterCondition[]) => { + table?.applyFilter(filters); + setViewport(0); + }, + [setViewport, table] + ); + useTableListener( table, dh.Table.EVENT_UPDATED, - createOnTableUpdatedHandler(table, viewportData, deserializeRow) + createOnTableUpdatedHandler(viewportData, deserializeRow) ); + const size = useTableSize(table); + useEffect(() => { if (table && !isClosed(table)) { // Hydrate the viewport with real data. This will fetch data from index // 0 to the end of the viewport + padding. setViewport(0); } - }, [table, setViewport]); + }, [table, setViewport, size]); return { viewportData, - size: getSize(table), + size, + table, + applyFiltersAndRefresh, setViewport, }; } diff --git a/packages/jsapi-utils/src/TableUtils.test.ts b/packages/jsapi-utils/src/TableUtils.test.ts index 5b45d1894f..6616cc3cce 100644 --- a/packages/jsapi-utils/src/TableUtils.test.ts +++ b/packages/jsapi-utils/src/TableUtils.test.ts @@ -1,5 +1,6 @@ import dh, { Column, + CustomColumn, DateWrapper, FilterCondition, FilterValue, @@ -13,6 +14,7 @@ import { Type as FilterType, TypeValue as FilterTypeValue, } from '@deephaven/filters'; +import { TestUtils } from '@deephaven/utils'; import TableUtils, { DataType, SortDirection } from './TableUtils'; import DateUtils from './DateUtils'; // eslint-disable-next-line import/no-relative-packages @@ -24,6 +26,21 @@ const EXPECT_TIME_ZONE_PARAM = expect.objectContaining({ id: DEFAULT_TIME_ZONE_ID, }); +/** + * Sends a mock event to the last registered event handler with the given event + * type. + */ +function sendEventToLastRegisteredHandler(table: Table, eventType: string) { + const event = TestUtils.createMockProxy({}); + + const lastRegisteredEventListener = TestUtils.findLastCall( + table.addEventListener, + ([et]) => et === eventType + )?.[1]; + + lastRegisteredEventListener?.(event); +} + function makeColumns(count = 5): Column[] { const columns: Column[] = []; @@ -46,6 +63,211 @@ function makeFilterCondition(type = ''): MockFilterCondition { }; } +describe('applyCustomColumns', () => { + const table = TestUtils.createMockProxy
({}); + const columns = [TestUtils.createMockProxy({})]; + + it.each([undefined, 400])( + 'should call table.applyCustomColumns and wait for dh.Table.EVENT_CUSTOMCOLUMNSCHANGED event: %s', + timeout => { + const executeAndWaitForEvent = jest.spyOn( + TableUtils, + 'executeAndWaitForEvent' + ); + + TableUtils.applyCustomColumns(table, columns, timeout); + + expect(TableUtils.executeAndWaitForEvent).toHaveBeenCalledWith( + expect.any(Function), + table, + dh.Table.EVENT_CUSTOMCOLUMNSCHANGED, + timeout ?? TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ); + + const exec = executeAndWaitForEvent.mock.lastCall?.[0]; + exec?.(table); + expect(table.applyCustomColumns).toHaveBeenCalledWith(columns); + } + ); +}); + +describe('applyFilter', () => { + const table = TestUtils.createMockProxy
({}); + const filters = [TestUtils.createMockProxy({})]; + + it.each([undefined, 400])( + 'should call table.applyFilter and wait for dh.Table.EVENT_FILTERCHANGED event: %s', + timeout => { + const executeAndWaitForEvent = jest.spyOn( + TableUtils, + 'executeAndWaitForEvent' + ); + + TableUtils.applyFilter(table, filters, timeout); + + expect(TableUtils.executeAndWaitForEvent).toHaveBeenCalledWith( + expect.any(Function), + table, + dh.Table.EVENT_FILTERCHANGED, + timeout ?? TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ); + + const exec = executeAndWaitForEvent.mock.lastCall?.[0]; + exec?.(table); + expect(table.applyFilter).toHaveBeenCalledWith(filters); + } + ); +}); + +describe.each([undefined, 400])('applyNeverFilter - timeout: %s', timeout => { + const column = TestUtils.createMockProxy({}); + const neverFilter = TestUtils.createMockProxy({}); + let makeNeverFilter: jest.SpyInstance; + + const table = TestUtils.createMockProxy
({ + findColumn: jest.fn().mockReturnValue(column), + }); + + beforeEach(() => { + makeNeverFilter = jest.spyOn(TableUtils, 'makeNeverFilter'); + makeNeverFilter.mockReturnValue(neverFilter); + }); + + afterEach(() => { + makeNeverFilter.mockRestore(); + }); + + it.each([null, undefined])( + 'should resolve to null when not given a table: %s', + async notTable => { + const columnName = 'mock.column'; + const result = await TableUtils.applyNeverFilter( + notTable, + columnName, + timeout + ); + expect(result).toBeNull(); + } + ); + + it('should call TableUtils.applyFilter with a "never filter": %s', async () => { + const applyFilter = jest + .spyOn(TableUtils, 'applyFilter') + .mockResolvedValue(table); + + const columnName = 'mock.column'; + + const result = await TableUtils.applyNeverFilter( + table, + columnName, + timeout + ); + + expect(result).toBe(table); + expect(makeNeverFilter).toHaveBeenCalledWith(column); + expect(applyFilter).toHaveBeenCalledWith( + table, + [neverFilter], + timeout ?? TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ); + }); +}); + +describe('applySort', () => { + const table = TestUtils.createMockProxy
({}); + const sorts = [TestUtils.createMockProxy({})]; + + it.each([undefined, 400])( + 'should call table.applySort and wait for dh.Table.EVENT_SORTCHANGED event: %s', + timeout => { + const executeAndWaitForEvent = jest.spyOn( + TableUtils, + 'executeAndWaitForEvent' + ); + + TableUtils.applySort(table, sorts, timeout); + + expect(TableUtils.executeAndWaitForEvent).toHaveBeenCalledWith( + expect.any(Function), + table, + dh.Table.EVENT_SORTCHANGED, + timeout ?? TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ); + + const exec = executeAndWaitForEvent.mock.lastCall?.[0]; + exec?.(table); + expect(table.applySort).toHaveBeenCalledWith(sorts); + } + ); +}); + +describe('executeAndWaitForEvent', () => { + const table = TestUtils.createMockProxy
({ + addEventListener: jest.fn(() => jest.fn()), + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it.each([null, undefined])( + 'should resolve to null if given null or undefined: %s', + async notTable => { + const exec = jest.fn(); + const actual = await TableUtils.executeAndWaitForEvent( + exec, + notTable, + 'mock.event' + ); + + expect(actual).toBeNull(); + expect(exec).not.toHaveBeenCalled(); + } + ); + + it.each(['mock.eventA', 'mock.eventB'])( + 'should call execute callback and wait for next `eventType` event: %s', + async eventType => { + const exec = jest.fn(); + const tablePromise = TableUtils.executeAndWaitForEvent( + exec, + table, + eventType + ); + + expect(exec).toHaveBeenCalledWith(table); + + sendEventToLastRegisteredHandler(table, eventType); + + const result = await tablePromise; + + expect(result).toBe(table); + } + ); + + it.each(['mock.eventA', 'mock.eventB'])( + 'should execute callback and reject promise if event timeout expires: %s', + async eventType => { + jest.useFakeTimers(); + + const exec = jest.fn(); + const timeout = 3000; + const tablePromise = TableUtils.executeAndWaitForEvent( + exec, + table, + eventType, + timeout + ); + + expect(exec).toHaveBeenCalledWith(table); + + jest.advanceTimersByTime(timeout); + + expect(tablePromise).rejects.toThrow(`Event "${eventType}" timed out.`); + } + ); +}); + describe('toggleSortForColumn', () => { it('toggles sort properly', () => { const columns = makeColumns(); diff --git a/packages/jsapi-utils/src/TableUtils.ts b/packages/jsapi-utils/src/TableUtils.ts index 417df3370f..0c92356462 100644 --- a/packages/jsapi-utils/src/TableUtils.ts +++ b/packages/jsapi-utils/src/TableUtils.ts @@ -8,6 +8,7 @@ import Log from '@deephaven/log'; import dh from '@deephaven/jsapi-shim'; import { Column, + CustomColumn, FilterCondition, FilterValue, LongWrapper, @@ -78,6 +79,8 @@ export class TableUtils { none: null, } as const; + static APPLY_TABLE_CHANGE_TIMEOUT_MS = 30000; + static REVERSE_TYPE = Object.freeze({ NONE: 'none', PRE_SORT: 'pre-sort', @@ -87,6 +90,132 @@ export class TableUtils { // Regex looking for a negative or positive integer or decimal number static NUMBER_REGEX = /^-?\d+(\.\d+)?$/; + /** + * Executes a callback on a given table and returns a Promise that will resolve + * the next time a particular event type fires on the table. + * @param exec Callback function to execute. + * @param table Table that gets passed to the `exec` function and that is + * subscribed to for a given `eventType`. + * @param eventType The event type to listen for. + * @param timeout If the event doesn't fire within the timeout, the returned + * Promise will be rejected. + * @returns a Promise to the original table that resolves on next `eventType` + * event + */ + static executeAndWaitForEvent = async ( + exec: (maybeTable: T | null | undefined) => void, + table: T | null | undefined, + eventType: string, + timeout = TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ): Promise => { + if (table == null) { + return null; + } + + const eventPromise = TableUtils.makeCancelableTableEventPromise( + table, + eventType, + timeout + ); + + exec(table); + + await eventPromise; + + return table; + }; + + /** + * Apply custom columns to a given table. Return a Promise that resolves with + * the table once the dh.Table.EVENT_CUSTOMCOLUMNSCHANGED event has fired. + * @param table The table to apply custom columns to. + * @param columns The list of column expressions or definitions to apply. + * @returns A Promise that will be resolved with the given table after the + * columns are applied. + */ + static async applyCustomColumns( + table: Table | null | undefined, + columns: (string | CustomColumn)[], + timeout = TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ): Promise
{ + return TableUtils.executeAndWaitForEvent( + t => t?.applyCustomColumns(columns), + table, + dh.Table.EVENT_CUSTOMCOLUMNSCHANGED, + timeout + ); + } + + /** + * Apply filters to a given table. + * @param table Table to apply filters to + * @param filters Filters to apply + * @param timeout Timeout before cancelling the promise that waits for the next + * dh.Table.EVENT_FILTERCHANGED event + * @returns a Promise to the Table that resolves after the next + * dh.Table.EVENT_FILTERCHANGED event + */ + static async applyFilter( + table: T | null | undefined, + filters: FilterCondition[], + timeout = TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ): Promise { + return TableUtils.executeAndWaitForEvent( + t => t?.applyFilter(filters), + table, + dh.Table.EVENT_FILTERCHANGED, + timeout + ); + } + + /** + * Apply a filter to a table that won't match anything. + * @table The table to apply the filter to + * @columnName The name of the column to apploy the filter to + * @param timeout Timeout before cancelling the promise that waits for the next + * dh.Table.EVENT_FILTERCHANGED event + * @returns a Promise to the Table that resolves after the next + * dh.Table.EVENT_FILTERCHANGED event + */ + static async applyNeverFilter( + table: T | null | undefined, + columnName: string, + timeout = TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ): Promise { + if (table == null) { + return null; + } + + const column = table.findColumn(columnName); + const filters = [TableUtils.makeNeverFilter(column)]; + + await TableUtils.applyFilter(table, filters, timeout); + + return table; + } + + /** + * Apply sorts to a given Table. + * @param table The table to apply sorts to + * @param sorts The sorts to apply + * @param timeout Timeout before cancelling the promise that waits for the next + * dh.Table.EVENT_SORTCHANGED event + * @returns a Promise to the Table that resolves after the next + * dh.Table.EVENT_SORTCHANGED event + */ + static async applySort( + table: T | null | undefined, + sorts: Sort[], + timeout = TableUtils.APPLY_TABLE_CHANGE_TIMEOUT_MS + ): Promise { + return TableUtils.executeAndWaitForEvent( + t => t?.applySort(sorts), + table, + dh.Table.EVENT_SORTCHANGED, + timeout + ); + } + static getSortIndex( sort: readonly Sort[], columnName: ColumnName @@ -1451,6 +1580,30 @@ export class TableUtils { } } + /** + * Create a filter condition that results in zero results for a given column + * @param column + */ + static makeNeverFilter(column: Column): FilterCondition { + let value = null; + + if (TableUtils.isTextType(column.type)) { + // Use 'a' so that it can work for String or Character types + value = dh.FilterValue.ofString('a'); + } else if (TableUtils.isBooleanType(column.type)) { + value = dh.FilterValue.ofBoolean(true); + } else if (TableUtils.isDateType(column.type)) { + value = dh.FilterValue.ofNumber(dh.DateWrapper.ofJsDate(new Date())); + } else { + value = dh.FilterValue.ofNumber(0); + } + + const eqFilter = column.filter().eq(value); + const notEqFilter = column.filter().notEq(value); + + return eqFilter.and(notEqFilter); + } + /** * Create a filter using the selected items * Has a flag for invertSelection as we start from a "Select All" state and a user just deselects items. @@ -1460,36 +1613,23 @@ export class TableUtils { * @param invertSelection Invert the selection (eg. All items are selected, then you deselect items) * @returns Returns a `in` or `notIn` FilterCondition as necessary, or null if no filtering should be applied (everything selected) */ - static makeSelectValueFilter( + static makeSelectValueFilter( column: Column, selectedValues: unknown[], - invertSelection: boolean - ): FilterCondition | null { + invertSelection: TInvert + ): TInvert extends true ? FilterCondition | null : FilterCondition { if (selectedValues.length === 0) { if (invertSelection) { // No filter means select everything - return null; + return null as TInvert extends true + ? FilterCondition | null + : FilterCondition; } // KLUDGE: Return a conflicting filter to show no results. // Could recognize this situation at a higher or lower level and pause updates on the // table, but this situation should be rare and that wouldn't be much gains for some added complexity - let value = null; - - if (TableUtils.isTextType(column.type)) { - // Use 'a' so that it can work for String or Character types - value = dh.FilterValue.ofString('a'); - } else if (TableUtils.isBooleanType(column.type)) { - value = dh.FilterValue.ofBoolean(true); - } else if (TableUtils.isDateType(column.type)) { - value = dh.FilterValue.ofNumber(dh.DateWrapper.ofJsDate(new Date())); - } else { - value = dh.FilterValue.ofNumber(0); - } - - const eqFilter = column.filter().eq(value); - const notEqFilter = column.filter().notEq(value); - return eqFilter.and(notEqFilter); + return TableUtils.makeNeverFilter(column); } const values = []; diff --git a/packages/jsapi-utils/src/ViewportDataUtils.test.ts b/packages/jsapi-utils/src/ViewportDataUtils.test.ts index c60d437e24..7e69559b88 100644 --- a/packages/jsapi-utils/src/ViewportDataUtils.test.ts +++ b/packages/jsapi-utils/src/ViewportDataUtils.test.ts @@ -1,6 +1,6 @@ -import { Column, Table, TreeTable } from '@deephaven/jsapi-shim'; +import type { Column, Table, TreeTable } from '@deephaven/jsapi-types'; import { TestUtils } from '@deephaven/utils'; -import { useListData } from '@react-stately/data'; +import { ListData, useListData } from '@react-stately/data'; import { act, renderHook } from '@testing-library/react-hooks'; import { KeyedItem, @@ -14,6 +14,7 @@ import { getSize, isClosed, padFirstAndLastRow, + getItemsFromListData, } from './ViewportDataUtils'; function mockViewportRow(offsetInSnapshot: number): ViewportRow { @@ -28,12 +29,14 @@ function mockColumn(name: string) { function mockUpdateEvent( offset: number, - rows: ViewportRow[] + rows: ViewportRow[], + columns: Column[] ): OnTableUpdatedEvent { return { detail: { offset, rows, + columns, }, } as OnTableUpdatedEvent; } @@ -44,7 +47,7 @@ beforeEach(() => { jest.clearAllMocks(); // Mock deserializer just returns the row given to it. - (deserializeRow as jest.Mock).mockImplementation(row => row); + TestUtils.asMock(deserializeRow).mockImplementation(row => row); }); describe('createKeyFromOffsetRow', () => { @@ -67,44 +70,20 @@ describe('createOnTableUpdatedHandler', () => { mockViewportRow(2), ]; - it('should do nothing if Table is null', () => { - const table = null; - - const { result: viewportDataRef } = renderHook(() => - useListData>({}) - ); - - const handler = createOnTableUpdatedHandler( - table, - viewportDataRef.current, - deserializeRow - ); - - const event = mockUpdateEvent(5, rows); - - act(() => { - handler(event); - }); - - expect(deserializeRow).not.toHaveBeenCalled(); - expect(viewportDataRef.current.items.length).toEqual(0); - }); + const cols: Column[] = []; it('should create a handler that adds items to a ListData of KeyedItems', () => { - const table = TestUtils.createMockProxy
({ columns: [] }); - const { result: viewportDataRef } = renderHook(() => useListData>({}) ); const handler = createOnTableUpdatedHandler( - table, viewportDataRef.current, deserializeRow ); const offset = 5; - const event = mockUpdateEvent(offset, rows); + const event = mockUpdateEvent(offset, rows, cols); const expectedItems = [ { key: '5', item: rows[0] }, { key: '6', item: rows[1] }, @@ -115,12 +94,13 @@ describe('createOnTableUpdatedHandler', () => { handler(event); }); + rows.forEach(row => { + expect(deserializeRow).toHaveBeenCalledWith(row, cols); + }); expect(viewportDataRef.current.items).toEqual(expectedItems); }); it('should create a handler that updates existing items in a ListData', () => { - const table = TestUtils.createMockProxy
({ columns: [] }); - const { result: viewportDataRef } = renderHook(() => useListData>({}) ); @@ -134,10 +114,9 @@ describe('createOnTableUpdatedHandler', () => { const offset = 0; const row = mockViewportRow(0); - const event = mockUpdateEvent(offset, [row]); + const event = mockUpdateEvent(offset, [row], cols); const handler = createOnTableUpdatedHandler( - table, viewportDataRef.current, deserializeRow ); @@ -146,6 +125,7 @@ describe('createOnTableUpdatedHandler', () => { handler(event); }); + expect(deserializeRow).toHaveBeenCalledWith(row, cols); expect(viewportDataRef.current.items).toEqual([{ key: '0', item: row }]); }); }); @@ -173,18 +153,40 @@ describe('defaultRowDeserializer', () => { describe('generateEmptyKeyedItems', () => { it.each([ - [1, [{ key: '0' }]], - [2, [{ key: '0' }, { key: '1' }]], - [5, [{ key: '0' }, { key: '1' }, { key: '2' }, { key: '3' }, { key: '4' }]], + [0, 0, [{ key: '0' }]], + [0, 1, [{ key: '0' }, { key: '1' }]], + [ + 0, + 4, + [{ key: '0' }, { key: '1' }, { key: '2' }, { key: '3' }, { key: '4' }], + ], + [3, 5, [{ key: '3' }, { key: '4' }, { key: '5' }]], ] as const)( - 'should generate a sequence of string keys for the given count: %s', - (count, expected) => { - const actual = [...generateEmptyKeyedItems(count)]; + 'should generate a sequence of string keys for the given range: %s', + (start, end, expected) => { + const actual = [...generateEmptyKeyedItems(start, end)]; expect(actual).toEqual(expected); } ); }); +describe('getItemsFromListData', () => { + it('should return items matching given keys', () => { + const keys = ['2', '4', '9']; + const listData = TestUtils.createMockProxy>>({ + getItem: jest.fn(key => ({ key: key as string, item: `item-${key}` })), + }); + + const result = getItemsFromListData(listData, ...keys); + + expect(result).toEqual([ + { key: '2', item: 'item-2' }, + { key: '4', item: 'item-4' }, + { key: '9', item: 'item-9' }, + ]); + }); +}); + describe('getSize', () => { it.each([undefined, null])( 'should return zero if no table given: %s', diff --git a/packages/jsapi-utils/src/ViewportDataUtils.ts b/packages/jsapi-utils/src/ViewportDataUtils.ts index b0d9bcd83d..125d85165e 100644 --- a/packages/jsapi-utils/src/ViewportDataUtils.ts +++ b/packages/jsapi-utils/src/ViewportDataUtils.ts @@ -10,6 +10,7 @@ export interface KeyedItem { export type OnTableUpdatedEvent = CustomEvent<{ offset: number; + columns: Column[]; rows: ViewportRow[]; }>; @@ -35,13 +36,11 @@ export function createKeyFromOffsetRow(row: ViewportRow, offset: number) { * Creates a handler function for a `dh.Table.EVENT_UPDATED` event. Rows that * get passed to the handler will be added to or updated in the given * `viewportData` object. - * @param table DH table instance. * @param viewportData State object for managing a list of KeyedItem data. * @param deserializeRow Converts a DH Row to an item object. * @returns Handler function for a `dh.Table.EVENT_UPDATED` event. */ export function createOnTableUpdatedHandler( - table: Table | TreeTable | null, viewportData: ListData>, deserializeRow: RowDeserializer ): (event: OnTableUpdatedEvent) => void { @@ -49,16 +48,12 @@ export function createOnTableUpdatedHandler( * Handler for a `dh.Table.EVENT_UPDATED` event. */ return function onTableUpdated(event: OnTableUpdatedEvent) { - if (table == null) { - return; - } - - const { offset, rows } = event.detail; + const { columns, offset, rows } = event.detail; log.debug('table updated', event.detail); rows.forEach(row => { - const item = deserializeRow(row, table.columns); + const item = deserializeRow(row, columns); const keyedItem = { key: createKeyFromOffsetRow(row, offset), @@ -97,17 +92,32 @@ export function defaultRowDeserializer( * each row in the backing table (even if these rows haven't been loaded yet). * This is needed internally by react-spectrum so it can calculate the content * area size. This generator can create a range of empty `KeyedItem` objects. - * @param count The number of items to generate + * @param start The starting index to generate + * @param end The ending index to generate */ export function* generateEmptyKeyedItems( - count: number + start: number, + end: number ): Generator, void, unknown> { // eslint-disable-next-line no-plusplus - for (let i = 0; i < count; ++i) { + for (let i = start; i <= end; ++i) { yield { key: String(i) }; } } +/** + * Get items from a given ListData by a list of keys. + * @param listData ListData to get items from + * @param keys Keys to items to be retrieved + * @returns An array of items matching the given keys + */ +export function getItemsFromListData( + listData: ListData>, + ...keys: string[] +): KeyedItem[] { + return keys.map(key => listData.getItem(key)); +} + /** * Check a Table to see if it is closed before checking its size property. This * is important because calling Table.size on a closed table throws an error. If diff --git a/packages/react-hooks/package.json b/packages/react-hooks/package.json index 27177ed36f..b38eee3e85 100644 --- a/packages/react-hooks/package.json +++ b/packages/react-hooks/package.json @@ -22,7 +22,8 @@ }, "dependencies": { "@deephaven/log": "file:../log", - "@deephaven/utils": "file:../utils" + "@deephaven/utils": "file:../utils", + "shortid": "^2.2.16" }, "peerDependencies": { "react": "^17.x" diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index 9b0397044a..b9e1c22439 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -2,7 +2,9 @@ export { default as useContextOrThrow } from './useContextOrThrow'; export { default as usePrevious } from './usePrevious'; export { default as useForwardedRef } from './useForwardedRef'; export { default as useCopyToClipboard } from './useCopyToClipboard'; +export { default as useFormWithDetachedSubmitButton } from './useFormWithDetachedSubmitButton'; export { default as usePromiseFactory } from './usePromiseFactory'; +export type { UseFormWithDetachedSubmitButtonResult } from './useFormWithDetachedSubmitButton'; export type { UsePromiseFactoryOptions, UsePromiseFactoryResult, diff --git a/packages/react-hooks/src/useFormWithDetachedSubmitButton.test.ts b/packages/react-hooks/src/useFormWithDetachedSubmitButton.test.ts new file mode 100644 index 0000000000..53cd18096c --- /dev/null +++ b/packages/react-hooks/src/useFormWithDetachedSubmitButton.test.ts @@ -0,0 +1,88 @@ +import shortid from 'shortid'; +import { TestUtils } from '@deephaven/utils'; +import { renderHook } from '@testing-library/react-hooks'; +import type { FocusableRefValue } from '@react-types/shared'; +import useFormWithDetachedSubmitButton from './useFormWithDetachedSubmitButton'; + +jest.mock('shortid'); + +let shortIdCount = 0; + +beforeEach(() => { + jest.clearAllMocks(); + + TestUtils.asMock(shortid).mockImplementation(() => { + shortIdCount += 1; + return String(shortIdCount); + }); +}); + +describe('useFormWithDetachedSubmitButton', () => { + it('should generate new formId on mount', () => { + const { rerender } = renderHook(() => useFormWithDetachedSubmitButton()); + expect(shortIdCount).toEqual(1); + + // Should not generate new id on re-render + rerender(); + expect(shortIdCount).toEqual(1); + + // Should generate new id if fresh mount + renderHook(() => useFormWithDetachedSubmitButton()); + expect(shortIdCount).toEqual(2); + }); + + it('should generate form and button props', () => { + const { result } = renderHook(() => useFormWithDetachedSubmitButton()); + + const formId = `useSubmitButtonRef-${shortIdCount}`; + + expect(result.current).toEqual({ + formProps: { + id: formId, + onSubmit: expect.any(Function), + }, + submitButtonProps: { + form: formId, + ref: expect.any(Function), + }, + }); + }); + + it.each([true, false])( + 'should include preventDefault function if preventDefault is true: %s', + enableDefaultFormSubmitBehavior => { + const { result } = renderHook(() => + useFormWithDetachedSubmitButton(enableDefaultFormSubmitBehavior) + ); + + expect(result.current.formProps.onSubmit).toEqual( + enableDefaultFormSubmitBehavior ? undefined : expect.any(Function) + ); + + if (!enableDefaultFormSubmitBehavior) { + const event = TestUtils.createMockProxy>({}); + result.current.formProps.onSubmit?.(event); + + expect(event.preventDefault).toHaveBeenCalled(); + } + } + ); + + it('should return a callback ref that sets `form` attribute to formId', () => { + const button = TestUtils.createMockProxy({}); + + const buttonRef = TestUtils.createMockProxy< + FocusableRefValue + >({ + UNSAFE_getDOMNode: jest.fn().mockReturnValue(button), + }); + + const { result } = renderHook(() => useFormWithDetachedSubmitButton()); + + result.current.submitButtonProps.ref(buttonRef); + + const formId = `useSubmitButtonRef-${shortIdCount}`; + + expect(button.setAttribute).toHaveBeenCalledWith('form', formId); + }); +}); diff --git a/packages/react-hooks/src/useFormWithDetachedSubmitButton.ts b/packages/react-hooks/src/useFormWithDetachedSubmitButton.ts new file mode 100644 index 0000000000..bdc1550497 --- /dev/null +++ b/packages/react-hooks/src/useFormWithDetachedSubmitButton.ts @@ -0,0 +1,68 @@ +import { FormEvent, useCallback, useMemo } from 'react'; +import shortid from 'shortid'; +import type { FocusableRefValue } from '@react-types/shared'; + +function preventDefault(event: FormEvent): void { + event.preventDefault(); +} + +export interface UseFormWithDetachedSubmitButtonResult { + formProps: { + id: string; + onSubmit?: (event: FormEvent) => void; + }; + submitButtonProps: { + form: string; + ref: (buttonEl: FocusableRefValue | null) => void; + }; +} + +/** + * Returns props to associate a form with a submit button. It generates a unique + * id that will be assigned to the form `id` attribute and a button `form` + * attribute. Useful for cases where a submit button exists outside of the form. + * + * e.g. + * + * const preventDefaultFormSubmit = true; + * const { formProps, submitButtonProps } = useFormWithDetachedSubmitButton(preventDefaultFormSubmit); + * + *
+ * + * + * @param enableDefaultFormSubmitBehavior Optionally enable default form submit behavior. + * @returns props that can be spread on a form component + a submit button component. + */ +export default function useFormWithDetachedSubmitButton( + enableDefaultFormSubmitBehavior = false +): UseFormWithDetachedSubmitButtonResult { + const formId = useMemo(() => `useSubmitButtonRef-${shortid()}`, []); + + const submitButtonRef = useCallback( + (buttonEl: FocusableRefValue | null) => { + buttonEl?.UNSAFE_getDOMNode().setAttribute('form', formId); + }, + [formId] + ); + + const formProps = useMemo( + () => ({ + id: formId, + onSubmit: enableDefaultFormSubmitBehavior ? undefined : preventDefault, + }), + [formId, enableDefaultFormSubmitBehavior] + ); + + const submitButtonProps = useMemo( + () => ({ + form: formId, + ref: submitButtonRef, + }), + [formId, submitButtonRef] + ); + + return { + formProps, + submitButtonProps, + }; +} diff --git a/packages/utils/src/TestUtils.test.tsx b/packages/utils/src/TestUtils.test.tsx index 0d2ae06470..667b1e2677 100644 --- a/packages/utils/src/TestUtils.test.tsx +++ b/packages/utils/src/TestUtils.test.tsx @@ -4,6 +4,43 @@ import userEvent from '@testing-library/user-event'; import TestUtils from './TestUtils'; +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('asMock', () => { + const someFunc: (name: string) => number = jest.fn( + (name: string): number => name.length + ); + + TestUtils.asMock(someFunc).mockImplementation(name => name.split(',').length); + + expect(someFunc('a,b,c')).toEqual(3); +}); + +describe('findLastCall', () => { + it('should return undefined if call not matched', () => { + const fn = jest.fn(); + + fn('AAA', 1); + + const result = TestUtils.findLastCall(fn, ([text, _num]) => text === 'BBB'); + expect(result).toBeUndefined(); + }); + + it('should find the last mock call matching the predicate', () => { + const fn = jest.fn(); + + fn('AAA', 1); + fn('BBB', 1); + fn('BBB', 2); + fn('CCC', 1); + + const result = TestUtils.findLastCall(fn, ([text, _num]) => text === 'BBB'); + expect(result).toEqual(['BBB', 2]); + }); +}); + describe('makeMockContext', () => { it('should make a MockContext object', () => { const mockContext = TestUtils.makeMockContext(); @@ -131,4 +168,48 @@ describe('createMockProxy', () => { expect(result).toBe(mock); }); + + it('should only show `in` for explicit properties', () => { + const mock = TestUtils.createMockProxy>({ + name: 'mock.name', + age: 42, + }); + + expect('name' in mock).toBeTruthy(); + expect('age' in mock).toBeTruthy(); + expect('blah' in mock).toBeFalsy(); + }); +}); + +describe('extractCallArgs', () => { + const fn = (a: string, b: string) => a.length + b.length; + const mockFn = jest.fn(fn); + + it('should return null if no calls have been made', () => { + const args = TestUtils.extractCallArgs(mockFn, 0); + expect(args).toBeNull(); + }); + + it('should return null if not given a mock fn', () => { + fn('john', 'doe'); + const args = TestUtils.extractCallArgs(fn, 0); + expect(args).toBeNull(); + }); + + it.each([ + [0, ['aaa', '111']], + [1, ['bbb', '222']], + [2, ['ccc', '333']], + [3, null], + ] as const)( + 'should return call args if index in range', + (callIndex, expected) => { + mockFn('aaa', '111'); + mockFn('bbb', '222'); + mockFn('ccc', '333'); + + const args = TestUtils.extractCallArgs(mockFn, callIndex); + expect(args).toEqual(expected); + } + ); }); diff --git a/packages/utils/src/TestUtils.ts b/packages/utils/src/TestUtils.ts index baf14a68f6..2f058a4502 100644 --- a/packages/utils/src/TestUtils.ts +++ b/packages/utils/src/TestUtils.ts @@ -31,6 +31,36 @@ export interface ClickOptions { } class TestUtils { + /** + * Type assertion to "cast" a function to it's corresponding jest.Mock + * function type. Note that this is a types only helper for type assertions. + * It will not actually convert a non-mock function. + * + * e.g. + * + * // This is actually a jest.Mock fn, but the type annotation hides this. + * const someFunc: (name: string) => number = jest.fn( + * (name: string): number => name.length + * ); + * + * // `asMock` will infer the type as jest.Mock which gives + * // us the ability to call `mockImplementation` in a type safe way. + * TestUtils.asMock(someFunc).mockImplementation(name => name.split(',').length) + */ + static asMock = ( + fn: (...args: TArgs) => TResult + ): jest.Mock => (fn as unknown) as jest.Mock; + + /** + * Find the last mock function call matching a given predicate. + * @param fn jest.Mock function + * @param predicate Predicate function to match calls against + */ + static findLastCall = ( + fn: (...args: TArgs) => TResult, + predicate: (args: TArgs) => boolean + ) => TestUtils.asMock(fn).mock.calls.reverse().find(predicate); + static makeMockContext(): MockContext { return { arc: jest.fn(), @@ -149,15 +179,17 @@ class TestUtils { * optionally be set via the constructor. Any prop that is not set will be set * to a jest.fn() instance on first access with the exeption of "then" which * will not be automatically proxied. - * @param props Optional props to set on the Proxy. + * @param props Optional props to explicitly set on the Proxy. * @returns */ static createMockProxy(props: Partial = {}): T { return new Proxy( { props: { - // Disable auto proxying of `then` to avoid issues with `await` - // treating the object as a thenable. + // Disable auto-proxying of certain properties that cause trouble. + // - Symbol.iterator - returning a jest.fn() throws a TypeError + // - then - avoid issues with `await` treating object as "thenable" + [Symbol.iterator]: undefined, then: undefined, ...props, }, @@ -176,9 +208,31 @@ class TestUtils { return target.proxies[name as keyof T]; }, + // Only consider explicitly defined props as "in" the proxy + has(target, name) { + return name in target.props; + }, } ) as T; } + + /** + * Attempt to extract the args for the nth call to a given function. This will + * only work if the given fn is a jest.Mock function. Otherwise, it returns + * null. + * @param fn + * @param callIndex Index of the function call. + */ + static extractCallArgs = ( + fn: (...args: TArgs) => TResult, + callIndex: number + ): TArgs | null => { + try { + return ((fn as jest.Mock).mock.calls[callIndex] ?? null) as TArgs | null; + } catch (err) { + return null; + } + }; } export default TestUtils;