diff --git a/packages/jsapi-components/src/index.ts b/packages/jsapi-components/src/index.ts index 16f750045a..8606285751 100644 --- a/packages/jsapi-components/src/index.ts +++ b/packages/jsapi-components/src/index.ts @@ -11,6 +11,7 @@ export { default as useDebouncedViewportSearch } from './useDebouncedViewportSea export * from './useDebouncedViewportSelectionFilter'; export * from './useFilterConditionFactories'; export * from './useFilteredItemsWithDefaultValue'; +export * from './useFormatter'; export * from './useGetItemIndexByValue'; export * from './useGetItemPosition'; export { default as useInitializeViewportData } from './useInitializeViewportData'; diff --git a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx index 7d3b1f2bbd..704dfc84d9 100644 --- a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx @@ -3,12 +3,12 @@ import { Picker as PickerBase, PickerProps as PickerPropsBase, } from '@deephaven/components'; -import { useApi } from '@deephaven/jsapi-bootstrap'; import { dh as DhType } from '@deephaven/jsapi-types'; -import { Formatter } from '@deephaven/jsapi-utils'; +import { Settings } from '@deephaven/jsapi-utils'; import Log from '@deephaven/log'; import { PICKER_ITEM_HEIGHT, PICKER_TOP_OFFSET } from '@deephaven/utils'; import { useCallback, useEffect, useMemo } from 'react'; +import useFormatter from '../../useFormatter'; import useGetItemIndexByValue from '../../useGetItemIndexByValue'; import { useViewportData } from '../../useViewportData'; import { getPickerKeyColumn } from './PickerUtils'; @@ -24,6 +24,8 @@ export interface PickerProps extends Omit { labelColumn?: string; // TODO #1890 : descriptionColumn, iconColumn + + settings?: Settings; } export function Picker({ @@ -31,14 +33,10 @@ export function Picker({ keyColumn: keyColumnName, labelColumn: labelColumnName, selectedKey, + settings, ...props }: PickerProps): JSX.Element { - const dh = useApi(); - - const formatValue = useMemo(() => { - const formatter = new Formatter(dh); - return formatter.getFormattedString.bind(formatter); - }, [dh]); + const { getFormattedString: formatValue } = useFormatter(settings); const keyColumn = useMemo( () => getPickerKeyColumn(table, keyColumnName), @@ -100,7 +98,7 @@ export function Picker({ isCanceled = true; }; }, - [getItemIndexByValue, setViewport] + [getItemIndexByValue, settings, setViewport] ); return ( diff --git a/packages/jsapi-components/src/useFormatter.test.ts b/packages/jsapi-components/src/useFormatter.test.ts new file mode 100644 index 0000000000..c39a531f3e --- /dev/null +++ b/packages/jsapi-components/src/useFormatter.test.ts @@ -0,0 +1,69 @@ +import { useApi } from '@deephaven/jsapi-bootstrap'; +import { bindAllMethods, TestUtils } from '@deephaven/utils'; +import { + createFormatterFromSettings, + Formatter, + Settings, +} from '@deephaven/jsapi-utils'; +import type { dh as DhType } from '@deephaven/jsapi-types'; +import { renderHook } from '@testing-library/react-hooks'; +import { useFormatter } from './useFormatter'; + +jest.mock('@deephaven/jsapi-bootstrap'); +jest.mock('@deephaven/jsapi-utils', () => { + const actual = jest.requireActual('@deephaven/jsapi-utils'); + return { + ...actual, + createFormatterFromSettings: jest.fn(), + }; +}); +jest.mock('@deephaven/utils', () => ({ + ...jest.requireActual('@deephaven/utils'), + bindAllMethods: jest.fn(), +})); + +const { asMock, createMockProxy } = TestUtils; + +beforeEach(() => { + jest.clearAllMocks(); + expect.hasAssertions(); +}); + +describe('useFormatter', () => { + const mock = { + dh: createMockProxy(), + formatter: createMockProxy(), + settings: createMockProxy(), + }; + + beforeEach(() => { + asMock(useApi).mockReturnValue(mock.dh); + + asMock(bindAllMethods) + .mockName('bindAllMethods') + .mockImplementation(a => a); + + asMock(createFormatterFromSettings) + .mockName('createFormatterFromSettings') + .mockReturnValue(mock.formatter); + }); + + it('should return members of a `Formatter` instance based on settings', () => { + const { result } = renderHook(() => useFormatter(mock.settings)); + + expect(createFormatterFromSettings).toHaveBeenCalledWith( + mock.dh, + mock.settings + ); + + expect(bindAllMethods).toHaveBeenCalledWith(mock.formatter); + + expect(result.current).toEqual({ + getColumnFormat: mock.formatter.getColumnFormat, + getColumnFormatMapForType: mock.formatter.getColumnFormatMapForType, + getColumnTypeFormatter: mock.formatter.getColumnTypeFormatter, + getFormattedString: mock.formatter.getFormattedString, + timeZone: mock.formatter.timeZone, + }); + }); +}); diff --git a/packages/jsapi-components/src/useFormatter.ts b/packages/jsapi-components/src/useFormatter.ts new file mode 100644 index 0000000000..0900b0a8c8 --- /dev/null +++ b/packages/jsapi-components/src/useFormatter.ts @@ -0,0 +1,55 @@ +import { useApi } from '@deephaven/jsapi-bootstrap'; +import { + createFormatterFromSettings, + Formatter, + Settings, +} from '@deephaven/jsapi-utils'; +import { bindAllMethods } from '@deephaven/utils'; +import { useMemo } from 'react'; + +export type UseFormatterResult = Pick< + Formatter, + | 'getColumnFormat' + | 'getColumnFormatMapForType' + | 'getColumnTypeFormatter' + | 'getFormattedString' + | 'timeZone' +>; + +/** + * Returns a subset of members of a `Formatter` instance. The `Formatter` will be + * constructed based on the given options or fallback to the configuration found + * in the current `FormatSettingsContext`. Members that are functions are bound + * to the `Formatter` instance, so they are safe to destructure. Static methods + * can still be accessed statically from the `Formatter` class. + * @param settings Optional settings to use when constructing the `Formatter` + */ +export function useFormatter(settings?: Settings): UseFormatterResult { + const dh = useApi(); + + const formatter = useMemo(() => { + const instance = createFormatterFromSettings(dh, settings); + + // Bind all methods so we can destructure them + bindAllMethods(instance); + + return instance; + }, [dh, settings]); + + const { + getColumnFormat, + getColumnFormatMapForType, + getColumnTypeFormatter, + getFormattedString, + } = formatter; + + return { + getColumnFormat, + getColumnFormatMapForType, + getColumnTypeFormatter, + getFormattedString, + timeZone: formatter.timeZone, + }; +} + +export default useFormatter; diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts index 8bbb81d22f..e21d200523 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts @@ -54,6 +54,20 @@ describe('useGetItemIndexByValue', () => { } ); + it('should throw if seekRow fails', async () => { + asMock(mockTable.seekRow).mockRejectedValue('Some error'); + + const { result } = renderHook(() => + useGetItemIndexByValue({ + columnName: 'mock.columnName', + table: mockTable, + value: 'mock.value', + }) + ); + + expect(result.current()).rejects.toEqual('Some error'); + }); + it.each([ ['mock.value', 42, 42], ['mock.value', -1, null], diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.ts b/packages/jsapi-components/src/useGetItemIndexByValue.ts index c1e91556af..ca2cc2008f 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.ts @@ -31,7 +31,6 @@ export function useGetItemIndexByValue({ const columnValueType = tableUtils.getValueType(column.type); const index = await table.seekRow(0, column, columnValueType, value); - return index === -1 ? null : index; }, [columnName, table, tableUtils, value]); } diff --git a/packages/jsapi-utils/src/FormatterUtils.test.ts b/packages/jsapi-utils/src/FormatterUtils.test.ts index abd01caab7..97bb49e5ba 100644 --- a/packages/jsapi-utils/src/FormatterUtils.test.ts +++ b/packages/jsapi-utils/src/FormatterUtils.test.ts @@ -1,5 +1,8 @@ -import Formatter from './Formatter'; +import dh from '@deephaven/jsapi-shim'; +import { TestUtils } from '@deephaven/utils'; +import Formatter, { FormattingRule } from './Formatter'; import FormatterUtils, { + createFormatterFromSettings, getColumnFormats, getDateTimeFormatterOptions, } from './FormatterUtils'; @@ -9,7 +12,31 @@ import { TableColumnFormatType, } from './formatters'; import TableUtils from './TableUtils'; -import { ColumnFormatSettings, DateTimeFormatSettings } from './Settings'; +import Settings, { + ColumnFormatSettings, + DateTimeFormatSettings, +} from './Settings'; + +jest.mock('./Formatter', () => { + const FormatterActual = jest.requireActual('./Formatter').default; + + // Extract static methods + const { makeColumnFormatMap, makeColumnFormattingRule } = FormatterActual; + + // Wrap Formatter constructor so we can spy on it + const mockFormatter = jest.fn((...args) => new FormatterActual(...args)); + + return { + __esModule: true, + default: Object.assign(mockFormatter, { + makeColumnFormatMap, + makeColumnFormattingRule, + }), + }; +}); + +const { createMockProxy } = TestUtils; +const FormatterActual = jest.requireActual('./Formatter').default; function makeFormatter(...settings: ConstructorParameters) { return new Formatter(...settings); @@ -23,6 +50,46 @@ function makeFormattingRule( return { label, formatString, type }; } +describe('createFormatterFromSettings', () => { + const mockSettings = createMockProxy({ + formatter: [ + createMockProxy({ + format: createMockProxy(), + }), + ], + }); + + it.each([undefined, mockSettings])( + 'should create a formatter with the given settings: %s', + settings => { + const columnRules = getColumnFormats(settings); + const dateTimeOptions = getDateTimeFormatterOptions(settings); + + const { + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings, + } = settings ?? {}; + + const actual = createFormatterFromSettings(dh, settings); + + expect(Formatter).toHaveBeenCalledWith( + dh, + columnRules, + dateTimeOptions, + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings + ); + expect(actual).toBeInstanceOf(FormatterActual); + } + ); +}); + describe('isCustomColumnFormatDefined', () => { const columnFormats = [ Formatter.makeColumnFormattingRule( diff --git a/packages/jsapi-utils/src/FormatterUtils.ts b/packages/jsapi-utils/src/FormatterUtils.ts index 55e837fb89..d25957a56d 100644 --- a/packages/jsapi-utils/src/FormatterUtils.ts +++ b/packages/jsapi-utils/src/FormatterUtils.ts @@ -1,7 +1,44 @@ +import type { dh as DhType } from '@deephaven/jsapi-types'; import type { FormattingRule } from './Formatter'; import Formatter from './Formatter'; import { DateTimeColumnFormatter, TableColumnFormatter } from './formatters'; -import { ColumnFormatSettings, DateTimeFormatSettings } from './Settings'; +import Settings, { + ColumnFormatSettings, + DateTimeFormatSettings, +} from './Settings'; + +/** + * Instantiate a `Formatter` from the given settings. + * @param dh The `dh` object + * @param settings Optional settings to use + * @returns A new `Formatter` instance + */ +export function createFormatterFromSettings( + dh: typeof DhType, + settings?: Settings +): Formatter { + const columnRules = getColumnFormats(settings); + const dateTimeOptions = getDateTimeFormatterOptions(settings); + + const { + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings, + } = settings ?? {}; + + return new Formatter( + dh, + columnRules, + dateTimeOptions, + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings + ); +} export function getColumnFormats( settings?: ColumnFormatSettings @@ -45,6 +82,7 @@ export function isCustomColumnFormatDefined( } export default { + createFormatterFromSettings, getColumnFormats, getDateTimeFormatterOptions, isCustomColumnFormatDefined, diff --git a/packages/utils/src/ClassUtils.test.ts b/packages/utils/src/ClassUtils.test.ts index cf57debfc5..a5807d48f6 100644 --- a/packages/utils/src/ClassUtils.test.ts +++ b/packages/utils/src/ClassUtils.test.ts @@ -7,6 +7,11 @@ class Aaa { getAaa() { return this.nameA; } + + // eslint-disable-next-line class-methods-use-this + get someGetter() { + throw new Error('This should not get evaluated as a method'); + } } class Bbb extends Aaa { @@ -15,6 +20,11 @@ class Bbb extends Aaa { getBbb() { return this.nameB; } + + // eslint-disable-next-line class-methods-use-this + get someGetter() { + throw new Error('This should not get evaluated as a method'); + } } class Ccc extends Bbb { @@ -25,6 +35,11 @@ class Ccc extends Bbb { } getCcc2 = () => this.nameC; + + // eslint-disable-next-line class-methods-use-this + get someGetter() { + throw new Error('This should not get evaluated as a method'); + } } beforeEach(() => { diff --git a/packages/utils/src/ClassUtils.ts b/packages/utils/src/ClassUtils.ts index 19a70d1456..9b6e347c50 100644 --- a/packages/utils/src/ClassUtils.ts +++ b/packages/utils/src/ClassUtils.ts @@ -51,7 +51,9 @@ export function getAllMethodNames( for (const name of Object.getOwnPropertyNames(current)) { if ( name !== 'constructor' && - typeof current[name as keyof typeof current] === 'function' + // Ensure this is a method and not a getter + typeof Object.getOwnPropertyDescriptor(current, name)?.value === + 'function' ) { methodNames.add(name as MethodName); }