From 0d7755cbb62669db74b1158b3322a075cf178bc1 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 27 Mar 2024 17:08:00 -0500 Subject: [PATCH 01/10] Formatter settings from context (#1889) --- .../components/FormatSettingsBootstrap.tsx | 25 ++++ packages/app-utils/src/components/index.ts | 1 + packages/code-studio/src/main/App.tsx | 7 +- packages/jsapi-components/src/index.ts | 2 + .../src/spectrum/Picker/Picker.tsx | 10 +- .../jsapi-components/src/useFormatSettings.ts | 14 +++ packages/jsapi-components/src/useFormatter.ts | 118 ++++++++++++++++++ packages/utils/src/ClassUtils.test.ts | 15 +++ packages/utils/src/ClassUtils.ts | 4 +- 9 files changed, 185 insertions(+), 11 deletions(-) create mode 100644 packages/app-utils/src/components/FormatSettingsBootstrap.tsx create mode 100644 packages/jsapi-components/src/useFormatSettings.ts create mode 100644 packages/jsapi-components/src/useFormatter.ts diff --git a/packages/app-utils/src/components/FormatSettingsBootstrap.tsx b/packages/app-utils/src/components/FormatSettingsBootstrap.tsx new file mode 100644 index 0000000000..c435af6ac3 --- /dev/null +++ b/packages/app-utils/src/components/FormatSettingsBootstrap.tsx @@ -0,0 +1,25 @@ +import { FormatSettingsContext } from '@deephaven/jsapi-components'; +import { getSettings, RootState } from '@deephaven/redux'; +import { useSelector } from 'react-redux'; + +export interface FormatSettingsBootstrapProps { + children: React.ReactNode; +} + +/** + * Get formatter settings from the Redux store and provide them via a + * FormatSettingsContext.Provider. + */ +export function FormatSettingsBootstrap({ + children, +}: FormatSettingsBootstrapProps): JSX.Element { + const settings = useSelector(getSettings); + + return ( + + {children} + + ); +} + +export default FormatSettingsBootstrap; diff --git a/packages/app-utils/src/components/index.ts b/packages/app-utils/src/components/index.ts index 270eb573cd..b6b3c402e4 100644 --- a/packages/app-utils/src/components/index.ts +++ b/packages/app-utils/src/components/index.ts @@ -4,6 +4,7 @@ export * from './ConnectionBootstrap'; export * from './ConnectionContext'; export * from './FontBootstrap'; export * from './FontsLoaded'; +export * from './FormatSettingsBootstrap'; export * from './PluginsBootstrap'; export * from './ThemeBootstrap'; export * from './useConnection'; diff --git a/packages/code-studio/src/main/App.tsx b/packages/code-studio/src/main/App.tsx index c9a717d0b4..b2c861722a 100644 --- a/packages/code-studio/src/main/App.tsx +++ b/packages/code-studio/src/main/App.tsx @@ -1,12 +1,15 @@ import React, { ReactElement } from 'react'; +import { FormatSettingsBootstrap } from '@deephaven/app-utils'; import { ContextMenuRoot } from '@deephaven/components'; import AppMainContainer from './AppMainContainer'; function App(): ReactElement { return (
- - + + + +
); } diff --git a/packages/jsapi-components/src/index.ts b/packages/jsapi-components/src/index.ts index 16f750045a..51a6f7ab9d 100644 --- a/packages/jsapi-components/src/index.ts +++ b/packages/jsapi-components/src/index.ts @@ -11,6 +11,8 @@ export { default as useDebouncedViewportSearch } from './useDebouncedViewportSea export * from './useDebouncedViewportSelectionFilter'; export * from './useFilterConditionFactories'; export * from './useFilteredItemsWithDefaultValue'; +export * from './useFormatSettings'; +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..dac6597e46 100644 --- a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx @@ -3,12 +3,11 @@ 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 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'; @@ -33,12 +32,7 @@ export function Picker({ selectedKey, ...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(); const keyColumn = useMemo( () => getPickerKeyColumn(table, keyColumnName), diff --git a/packages/jsapi-components/src/useFormatSettings.ts b/packages/jsapi-components/src/useFormatSettings.ts new file mode 100644 index 0000000000..b5e1bb6448 --- /dev/null +++ b/packages/jsapi-components/src/useFormatSettings.ts @@ -0,0 +1,14 @@ +import React from 'react'; +import { Settings } from '@deephaven/jsapi-utils'; +import { useContextOrThrow } from '@deephaven/react-hooks'; + +export const FormatSettingsContext = React.createContext(null); + +export function useFormatSettings(): Settings { + return useContextOrThrow( + FormatSettingsContext, + 'No format settings available in useFormatSettings. Was code wrapped in FormatSettingsContext.Provider?' + ); +} + +export default useFormatSettings; diff --git a/packages/jsapi-components/src/useFormatter.ts b/packages/jsapi-components/src/useFormatter.ts new file mode 100644 index 0000000000..9774f0ac3e --- /dev/null +++ b/packages/jsapi-components/src/useFormatter.ts @@ -0,0 +1,118 @@ +import { useApi } from '@deephaven/jsapi-bootstrap'; +import { + DateTimeColumnFormatterOptions, + DecimalColumnFormatterOptions, + Formatter, + FormatterUtils, + FormattingRule, + IntegerColumnFormatterOptions, +} from '@deephaven/jsapi-utils'; +import { bindAllMethods } from '@deephaven/utils'; +import { useMemo } from 'react'; +import { useFormatSettings } from './useFormatSettings'; + +export type UseFormatterResult = Pick< + Formatter, + | 'getColumnFormat' + | 'getColumnFormatMapForType' + | 'getColumnTypeFormatter' + | 'getFormattedString' + | 'timeZone' +>; + +export interface FormatterOptions { + columnRules?: FormattingRule[]; + dateTimeOptions?: DateTimeColumnFormatterOptions; + decimalFormatOptions?: DecimalColumnFormatterOptions; + integerFormatOptions?: IntegerColumnFormatterOptions; + truncateNumbersWithPound?: boolean; + showEmptyStrings?: boolean; + showNullStrings?: boolean; +} + +/** + * Returns a subset of `Formatter` instance members based on the current + * `FormatSettingsContext`. Methods are bound to a `Formatter` instance, so they + * are safe to destructure. Static methods can still be accessed statically from + * the `Formatter` class. + * @param columnRules Column formatting rules to use, if not provided, will use + * the rules from the `FormatSettingsContext` + * @param dateTimeOptions DateTime formatting options to use, if not provided, + * will use the options from the `FormatSettingsContext` + * @param decimalFormatOptions Decimal formatting options to use, if not provided, + * will use the options from the `FormatSettingsContext` + * @param integerFormatOptions Integer formatting options to use, if not provided, + * will use the options from the `FormatSettingsContext` + * @param truncateNumbersWithPound Determine if numbers should be truncated w/ + * repeating # instead of ellipsis at the end + * @param showEmptyStrings Determine if empty strings should be shown + * @param showNullStrings Determine if null strings should be shown + * @returns Formatter util functions + timeZone value + */ +export function useFormatter({ + columnRules: columnRulesOverrides, + dateTimeOptions: dateTimeOptionsOverrides, + decimalFormatOptions: decimalFormatOptionsOverrides, + integerFormatOptions: integerFormatOptionsOverrides, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings, +}: FormatterOptions = {}): UseFormatterResult { + const dh = useApi(); + const formatSettings = useFormatSettings(); + + const formatter = useMemo(() => { + const columnRules = + columnRulesOverrides ?? FormatterUtils.getColumnFormats(formatSettings); + + const dateTimeOptions = + dateTimeOptionsOverrides ?? + FormatterUtils.getDateTimeFormatterOptions(formatSettings); + + const { defaultDecimalFormatOptions, defaultIntegerFormatOptions } = + formatSettings; + + const instance = new Formatter( + dh, + columnRules, + dateTimeOptions, + decimalFormatOptionsOverrides ?? defaultDecimalFormatOptions, + integerFormatOptionsOverrides ?? defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings + ); + + // Bind all methods so we can destructure them + bindAllMethods(instance); + + return instance; + }, [ + columnRulesOverrides, + dateTimeOptionsOverrides, + decimalFormatOptionsOverrides, + dh, + formatSettings, + integerFormatOptionsOverrides, + showEmptyStrings, + showNullStrings, + truncateNumbersWithPound, + ]); + + const { + getColumnFormat, + getColumnFormatMapForType, + getColumnTypeFormatter, + getFormattedString, + } = formatter; + + return { + getColumnFormat, + getColumnFormatMapForType, + getColumnTypeFormatter, + getFormattedString, + timeZone: formatter.timeZone, + }; +} + +export default useFormatter; 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); } From c4f9e731eb57aa5e51f64ca20d08908cc39923f3 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 27 Mar 2024 17:15:06 -0500 Subject: [PATCH 02/10] Comments (#1889) --- packages/jsapi-components/src/useFormatter.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/jsapi-components/src/useFormatter.ts b/packages/jsapi-components/src/useFormatter.ts index 9774f0ac3e..46c6b7311d 100644 --- a/packages/jsapi-components/src/useFormatter.ts +++ b/packages/jsapi-components/src/useFormatter.ts @@ -31,10 +31,11 @@ export interface FormatterOptions { } /** - * Returns a subset of `Formatter` instance members based on the current - * `FormatSettingsContext`. Methods are bound to a `Formatter` instance, so they - * are safe to destructure. Static methods can still be accessed statically from - * the `Formatter` class. + * 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 columnRules Column formatting rules to use, if not provided, will use * the rules from the `FormatSettingsContext` * @param dateTimeOptions DateTime formatting options to use, if not provided, From 94cf75d0a808c25186056d66263587c2519cf509 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 28 Mar 2024 08:57:15 -0500 Subject: [PATCH 03/10] Simplified formatter settings (#1889) --- .../components/FormatSettingsBootstrap.tsx | 25 ------ packages/app-utils/src/components/index.ts | 1 - packages/code-studio/src/main/App.tsx | 7 +- packages/jsapi-components/src/index.ts | 1 - .../src/spectrum/Picker/Picker.tsx | 6 +- .../jsapi-components/src/useFormatSettings.ts | 14 ---- packages/jsapi-components/src/useFormatter.ts | 76 ++++--------------- 7 files changed, 22 insertions(+), 108 deletions(-) delete mode 100644 packages/app-utils/src/components/FormatSettingsBootstrap.tsx delete mode 100644 packages/jsapi-components/src/useFormatSettings.ts diff --git a/packages/app-utils/src/components/FormatSettingsBootstrap.tsx b/packages/app-utils/src/components/FormatSettingsBootstrap.tsx deleted file mode 100644 index c435af6ac3..0000000000 --- a/packages/app-utils/src/components/FormatSettingsBootstrap.tsx +++ /dev/null @@ -1,25 +0,0 @@ -import { FormatSettingsContext } from '@deephaven/jsapi-components'; -import { getSettings, RootState } from '@deephaven/redux'; -import { useSelector } from 'react-redux'; - -export interface FormatSettingsBootstrapProps { - children: React.ReactNode; -} - -/** - * Get formatter settings from the Redux store and provide them via a - * FormatSettingsContext.Provider. - */ -export function FormatSettingsBootstrap({ - children, -}: FormatSettingsBootstrapProps): JSX.Element { - const settings = useSelector(getSettings); - - return ( - - {children} - - ); -} - -export default FormatSettingsBootstrap; diff --git a/packages/app-utils/src/components/index.ts b/packages/app-utils/src/components/index.ts index b6b3c402e4..270eb573cd 100644 --- a/packages/app-utils/src/components/index.ts +++ b/packages/app-utils/src/components/index.ts @@ -4,7 +4,6 @@ export * from './ConnectionBootstrap'; export * from './ConnectionContext'; export * from './FontBootstrap'; export * from './FontsLoaded'; -export * from './FormatSettingsBootstrap'; export * from './PluginsBootstrap'; export * from './ThemeBootstrap'; export * from './useConnection'; diff --git a/packages/code-studio/src/main/App.tsx b/packages/code-studio/src/main/App.tsx index b2c861722a..c9a717d0b4 100644 --- a/packages/code-studio/src/main/App.tsx +++ b/packages/code-studio/src/main/App.tsx @@ -1,15 +1,12 @@ import React, { ReactElement } from 'react'; -import { FormatSettingsBootstrap } from '@deephaven/app-utils'; import { ContextMenuRoot } from '@deephaven/components'; import AppMainContainer from './AppMainContainer'; function App(): ReactElement { return (
- - - - + +
); } diff --git a/packages/jsapi-components/src/index.ts b/packages/jsapi-components/src/index.ts index 51a6f7ab9d..8606285751 100644 --- a/packages/jsapi-components/src/index.ts +++ b/packages/jsapi-components/src/index.ts @@ -11,7 +11,6 @@ export { default as useDebouncedViewportSearch } from './useDebouncedViewportSea export * from './useDebouncedViewportSelectionFilter'; export * from './useFilterConditionFactories'; export * from './useFilteredItemsWithDefaultValue'; -export * from './useFormatSettings'; export * from './useFormatter'; export * from './useGetItemIndexByValue'; export * from './useGetItemPosition'; diff --git a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx index dac6597e46..e1dafca69d 100644 --- a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx @@ -4,6 +4,7 @@ import { PickerProps as PickerPropsBase, } from '@deephaven/components'; import { dh as DhType } from '@deephaven/jsapi-types'; +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'; @@ -23,6 +24,8 @@ export interface PickerProps extends Omit { labelColumn?: string; // TODO #1890 : descriptionColumn, iconColumn + + settings?: Settings; } export function Picker({ @@ -30,9 +33,10 @@ export function Picker({ keyColumn: keyColumnName, labelColumn: labelColumnName, selectedKey, + settings, ...props }: PickerProps): JSX.Element { - const { getFormattedString: formatValue } = useFormatter(); + const { getFormattedString: formatValue } = useFormatter(settings); const keyColumn = useMemo( () => getPickerKeyColumn(table, keyColumnName), diff --git a/packages/jsapi-components/src/useFormatSettings.ts b/packages/jsapi-components/src/useFormatSettings.ts deleted file mode 100644 index b5e1bb6448..0000000000 --- a/packages/jsapi-components/src/useFormatSettings.ts +++ /dev/null @@ -1,14 +0,0 @@ -import React from 'react'; -import { Settings } from '@deephaven/jsapi-utils'; -import { useContextOrThrow } from '@deephaven/react-hooks'; - -export const FormatSettingsContext = React.createContext(null); - -export function useFormatSettings(): Settings { - return useContextOrThrow( - FormatSettingsContext, - 'No format settings available in useFormatSettings. Was code wrapped in FormatSettingsContext.Provider?' - ); -} - -export default useFormatSettings; diff --git a/packages/jsapi-components/src/useFormatter.ts b/packages/jsapi-components/src/useFormatter.ts index 46c6b7311d..02331ff130 100644 --- a/packages/jsapi-components/src/useFormatter.ts +++ b/packages/jsapi-components/src/useFormatter.ts @@ -1,15 +1,7 @@ import { useApi } from '@deephaven/jsapi-bootstrap'; -import { - DateTimeColumnFormatterOptions, - DecimalColumnFormatterOptions, - Formatter, - FormatterUtils, - FormattingRule, - IntegerColumnFormatterOptions, -} from '@deephaven/jsapi-utils'; +import { Formatter, FormatterUtils, Settings } from '@deephaven/jsapi-utils'; import { bindAllMethods } from '@deephaven/utils'; import { useMemo } from 'react'; -import { useFormatSettings } from './useFormatSettings'; export type UseFormatterResult = Pick< Formatter, @@ -20,65 +12,37 @@ export type UseFormatterResult = Pick< | 'timeZone' >; -export interface FormatterOptions { - columnRules?: FormattingRule[]; - dateTimeOptions?: DateTimeColumnFormatterOptions; - decimalFormatOptions?: DecimalColumnFormatterOptions; - integerFormatOptions?: IntegerColumnFormatterOptions; - truncateNumbersWithPound?: boolean; - showEmptyStrings?: boolean; - showNullStrings?: boolean; -} - /** * 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 columnRules Column formatting rules to use, if not provided, will use - * the rules from the `FormatSettingsContext` - * @param dateTimeOptions DateTime formatting options to use, if not provided, - * will use the options from the `FormatSettingsContext` - * @param decimalFormatOptions Decimal formatting options to use, if not provided, - * will use the options from the `FormatSettingsContext` - * @param integerFormatOptions Integer formatting options to use, if not provided, - * will use the options from the `FormatSettingsContext` - * @param truncateNumbersWithPound Determine if numbers should be truncated w/ - * repeating # instead of ellipsis at the end - * @param showEmptyStrings Determine if empty strings should be shown - * @param showNullStrings Determine if null strings should be shown - * @returns Formatter util functions + timeZone value + * @param settings Optional settings to use when constructing the `Formatter` */ -export function useFormatter({ - columnRules: columnRulesOverrides, - dateTimeOptions: dateTimeOptionsOverrides, - decimalFormatOptions: decimalFormatOptionsOverrides, - integerFormatOptions: integerFormatOptionsOverrides, - truncateNumbersWithPound, - showEmptyStrings, - showNullStrings, -}: FormatterOptions = {}): UseFormatterResult { +export function useFormatter(settings?: Settings): UseFormatterResult { const dh = useApi(); - const formatSettings = useFormatSettings(); const formatter = useMemo(() => { - const columnRules = - columnRulesOverrides ?? FormatterUtils.getColumnFormats(formatSettings); + const columnRules = FormatterUtils.getColumnFormats(settings); const dateTimeOptions = - dateTimeOptionsOverrides ?? - FormatterUtils.getDateTimeFormatterOptions(formatSettings); + FormatterUtils.getDateTimeFormatterOptions(settings); - const { defaultDecimalFormatOptions, defaultIntegerFormatOptions } = - formatSettings; + const { + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, + truncateNumbersWithPound, + showEmptyStrings, + showNullStrings, + } = settings ?? {}; const instance = new Formatter( dh, columnRules, dateTimeOptions, - decimalFormatOptionsOverrides ?? defaultDecimalFormatOptions, - integerFormatOptionsOverrides ?? defaultIntegerFormatOptions, + defaultDecimalFormatOptions, + defaultIntegerFormatOptions, truncateNumbersWithPound, showEmptyStrings, showNullStrings @@ -88,17 +52,7 @@ export function useFormatter({ bindAllMethods(instance); return instance; - }, [ - columnRulesOverrides, - dateTimeOptionsOverrides, - decimalFormatOptionsOverrides, - dh, - formatSettings, - integerFormatOptionsOverrides, - showEmptyStrings, - showNullStrings, - truncateNumbersWithPound, - ]); + }, [dh, settings]); const { getColumnFormat, From edd261e84e6845bd5668909015c2e281604cbb54 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 28 Mar 2024 11:56:50 -0500 Subject: [PATCH 04/10] Split out util function (#1889) --- packages/jsapi-components/src/useFormatter.ts | 30 +++----------- packages/jsapi-utils/src/FormatterUtils.ts | 40 ++++++++++++++++++- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/packages/jsapi-components/src/useFormatter.ts b/packages/jsapi-components/src/useFormatter.ts index 02331ff130..0900b0a8c8 100644 --- a/packages/jsapi-components/src/useFormatter.ts +++ b/packages/jsapi-components/src/useFormatter.ts @@ -1,5 +1,9 @@ import { useApi } from '@deephaven/jsapi-bootstrap'; -import { Formatter, FormatterUtils, Settings } from '@deephaven/jsapi-utils'; +import { + createFormatterFromSettings, + Formatter, + Settings, +} from '@deephaven/jsapi-utils'; import { bindAllMethods } from '@deephaven/utils'; import { useMemo } from 'react'; @@ -24,29 +28,7 @@ export function useFormatter(settings?: Settings): UseFormatterResult { const dh = useApi(); const formatter = useMemo(() => { - const columnRules = FormatterUtils.getColumnFormats(settings); - - const dateTimeOptions = - FormatterUtils.getDateTimeFormatterOptions(settings); - - const { - defaultDecimalFormatOptions, - defaultIntegerFormatOptions, - truncateNumbersWithPound, - showEmptyStrings, - showNullStrings, - } = settings ?? {}; - - const instance = new Formatter( - dh, - columnRules, - dateTimeOptions, - defaultDecimalFormatOptions, - defaultIntegerFormatOptions, - truncateNumbersWithPound, - showEmptyStrings, - showNullStrings - ); + const instance = createFormatterFromSettings(dh, settings); // Bind all methods so we can destructure them bindAllMethods(instance); 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, From de2aa6da06f27d32aa4b5f6c34376af0a06089af Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 28 Mar 2024 14:39:19 -0500 Subject: [PATCH 05/10] Tests (#1889) --- .../jsapi-components/src/useFormatter.test.ts | 69 ++++++++++++++++++ .../jsapi-utils/src/FormatterUtils.test.ts | 71 ++++++++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 packages/jsapi-components/src/useFormatter.test.ts 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-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( From fe09fa65333da4434cdef446587aeb7b37ad6ac7 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 28 Mar 2024 16:14:42 -0500 Subject: [PATCH 06/10] useGetItemIndexByValue error handling (#1889) --- .../src/useGetItemIndexByValue.test.ts | 16 ++++++++++++++++ .../src/useGetItemIndexByValue.ts | 13 ++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts index 8bbb81d22f..3e0deec62f 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts @@ -54,6 +54,22 @@ describe('useGetItemIndexByValue', () => { } ); + it('should return null if seekRow fails', async () => { + asMock(mockTable.seekRow).mockRejectedValue('Some error'); + + const { result } = renderHook(() => + useGetItemIndexByValue({ + columnName: 'mock.columnName', + table: mockTable, + value: 'mock.value', + }) + ); + + const actual = await result.current(); + + expect(actual).toBeNull(); + }); + 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..6d38ec5d2e 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.ts @@ -1,7 +1,10 @@ import { useCallback } from 'react'; import { dh } from '@deephaven/jsapi-types'; +import Log from '@deephaven/log'; import { useTableUtils } from './useTableUtils'; +const log = Log.module('useGetItemIndexByValue'); + /** * Returns a function that gets the index of the first row containing a column * value. @@ -30,9 +33,13 @@ export function useGetItemIndexByValue({ const column = table.findColumn(columnName); const columnValueType = tableUtils.getValueType(column.type); - const index = await table.seekRow(0, column, columnValueType, value); - - return index === -1 ? null : index; + try { + const index = await table.seekRow(0, column, columnValueType, value); + return index === -1 ? null : index; + } catch (err) { + log.debug('Error seeking row', { column, value, columnValueType }); + return null; + } }, [columnName, table, tableUtils, value]); } From c032c9b10a0dc4b4a6caaefebdefacf314d2b8fc Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 28 Mar 2024 16:48:24 -0500 Subject: [PATCH 07/10] Added settings to dependency array to update selection on settings change (#1889) --- packages/jsapi-components/src/spectrum/Picker/Picker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx index e1dafca69d..704dfc84d9 100644 --- a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx @@ -98,7 +98,7 @@ export function Picker({ isCanceled = true; }; }, - [getItemIndexByValue, setViewport] + [getItemIndexByValue, settings, setViewport] ); return ( From 903d03c2d9b2cecc01b7a642ae4fa09c8060bb81 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 10:18:54 -0500 Subject: [PATCH 08/10] Rethrow error (#1889) --- packages/jsapi-components/src/useGetItemIndexByValue.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.ts b/packages/jsapi-components/src/useGetItemIndexByValue.ts index 6d38ec5d2e..1149e48f54 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.ts @@ -38,7 +38,7 @@ export function useGetItemIndexByValue({ return index === -1 ? null : index; } catch (err) { log.debug('Error seeking row', { column, value, columnValueType }); - return null; + throw err; } }, [columnName, table, tableUtils, value]); } From 1e86dc67aa5a14e89ba2d9c3c0451627b1596e5c Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 10:35:45 -0500 Subject: [PATCH 09/10] Fixed test (#1889) --- .../jsapi-components/src/useGetItemIndexByValue.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts index 3e0deec62f..e21d200523 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.test.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.test.ts @@ -54,7 +54,7 @@ describe('useGetItemIndexByValue', () => { } ); - it('should return null if seekRow fails', async () => { + it('should throw if seekRow fails', async () => { asMock(mockTable.seekRow).mockRejectedValue('Some error'); const { result } = renderHook(() => @@ -65,9 +65,7 @@ describe('useGetItemIndexByValue', () => { }) ); - const actual = await result.current(); - - expect(actual).toBeNull(); + expect(result.current()).rejects.toEqual('Some error'); }); it.each([ From 416386273e355940486caca92a1c1d1fcda24be4 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 14:30:38 -0500 Subject: [PATCH 10/10] Removed catch + log (#1889) --- .../jsapi-components/src/useGetItemIndexByValue.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/jsapi-components/src/useGetItemIndexByValue.ts b/packages/jsapi-components/src/useGetItemIndexByValue.ts index 1149e48f54..ca2cc2008f 100644 --- a/packages/jsapi-components/src/useGetItemIndexByValue.ts +++ b/packages/jsapi-components/src/useGetItemIndexByValue.ts @@ -1,10 +1,7 @@ import { useCallback } from 'react'; import { dh } from '@deephaven/jsapi-types'; -import Log from '@deephaven/log'; import { useTableUtils } from './useTableUtils'; -const log = Log.module('useGetItemIndexByValue'); - /** * Returns a function that gets the index of the first row containing a column * value. @@ -33,13 +30,8 @@ export function useGetItemIndexByValue({ const column = table.findColumn(columnName); const columnValueType = tableUtils.getValueType(column.type); - try { - const index = await table.seekRow(0, column, columnValueType, value); - return index === -1 ? null : index; - } catch (err) { - log.debug('Error seeking row', { column, value, columnValueType }); - throw err; - } + const index = await table.seekRow(0, column, columnValueType, value); + return index === -1 ? null : index; }, [columnName, table, tableUtils, value]); }