Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: TypeScript Type Improvements #1056

Merged
merged 11 commits into from
Mar 13, 2023
2 changes: 1 addition & 1 deletion packages/chart/src/ChartModelFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class ChartModelFactory {
* This causes TS issues in 1 or 2 spots. Once this is TS it can be returned to just FigureChartModel
*/
static async makeModel(
settings: Record<string, unknown> | undefined,
settings: ChartModelSettings | undefined,
figure: Figure,
theme = ChartTheme
): Promise<ChartModel> {
Expand Down
19 changes: 11 additions & 8 deletions packages/chart/src/ChartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ import ChartTheme from './ChartTheme';

export interface ChartModelSettings {
hiddenSeries?: string[];
type: keyof SeriesPlotStyle;
series: string[];
xAxis: string;
type?: keyof SeriesPlotStyle;
series?: string[];
xAxis?: string;
title?: string;
}

Expand Down Expand Up @@ -1803,18 +1803,21 @@ class ChartUtils {
*/
static hydrateSettings(
settings: ChartModelSettings
): Omit<ChartModelSettings, 'type'> & { type: SeriesPlotStyle } {
): Omit<ChartModelSettings, 'type'> & { type?: SeriesPlotStyle } {
return {
...settings,
type: dh.plot.SeriesPlotStyle[settings.type],
type:
settings.type != null
? dh.plot.SeriesPlotStyle[settings.type]
: undefined,
};
}

static titleFromSettings(settings: ChartModelSettings): string {
const {
series,
xAxis,
title = `${series.join(', ')} by ${xAxis}`,
title = `${(series ?? []).join(', ')} by ${xAxis}`,
} = settings;

return title;
Expand Down Expand Up @@ -1870,13 +1873,13 @@ class ChartUtils {
{
chartType: `${dh.plot.ChartType.XY}`,
axes: [xAxis, yAxis],
series: series.map(name => ({
series: (series ?? []).map(name => ({
plotStyle: `${type}`,
name,
dataSources: [
{
type: `${dh.plot.SourceType.X}`,
columnName: settingsAxis,
columnName: settingsAxis ?? '',
axis: xAxis,
table,
},
Expand Down
3 changes: 1 addition & 2 deletions packages/code-studio/src/main/WidgetUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ export const createChartModel = async (
};
const figure = await connection.getObject(definition);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
return ChartModelFactory.makeModel(settings as any, figure);
return ChartModelFactory.makeModel(settings, figure);
}

const definition = {
Expand Down
12 changes: 7 additions & 5 deletions packages/code-studio/src/redux/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import { RootState, ServerConfigValues } from '@deephaven/redux';
import { RootState } from '@deephaven/redux';
import LayoutStorage from '../storage/LayoutStorage';

/**
* Get the layout storage used by the app
* @param store The redux store
* @returns The layout storage instance
*/
export const getLayoutStorage = (store: RootState): LayoutStorage =>
store.layoutStorage as LayoutStorage;
export const getLayoutStorage = <State extends RootState = RootState>(
store: State
): LayoutStorage => store.layoutStorage as LayoutStorage;

/**
* Get the configuration values of the server
* @param store The redux store
* @returns The layout storage instance
*/
export const getServerConfigValues = (store: RootState): ServerConfigValues =>
store.serverConfigValues;
export const getServerConfigValues = <State extends RootState = RootState>(
store: State
): State['serverConfigValues'] => store.serverConfigValues;
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ export class ColumnSpecificSectionContent extends PureComponent<
isNewRule: true,
};
return {
formatSettings:
formatSettings != null ? [...formatSettings, newFormat] : [newFormat],
formatSettings: [...formatSettings, newFormat],
formatRulesChanged: true,
};
});
Expand Down
2 changes: 1 addition & 1 deletion packages/code-studio/src/settings/SettingsUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type FormatOption = {
export type FormatterItem = {
columnType: string;
columnName: string;
format: Partial<TableColumnFormat>;
format: TableColumnFormat | Record<string, never>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think would be cleaner to do Partial<TableColumnFormat>

id?: number;
isNewRule?: boolean;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/navigation/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function isSwitchMenuItemType(item: MenuItemDef): item is SwitchMenuItemDef {
}

export type MenuItemProps = {
item: MenuItemDef;
item: MenuItemDef | SwitchMenuItemDef;
Zhou-Ziheng marked this conversation as resolved.
Show resolved Hide resolved
onSelect?: () => void;
'data-testid'?: string;
};
Expand Down
3 changes: 1 addition & 2 deletions packages/dashboard-core-plugins/src/ChartBuilderPlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ export function ChartBuilderPlugin(
}) => {
const { settings } = metadata;
const makeModel = () =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ChartModelFactory.makeModelFromSettings(settings as any, table);
ChartModelFactory.makeModelFromSettings(settings, table);
const title = ChartUtils.titleFromSettings(settings);

const config = {
Expand Down
31 changes: 23 additions & 8 deletions packages/iris-grid/src/IrisGridUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ type HydratedIrisGridState = Pick<
metrics: Pick<GridMetrics, 'userColumnWidths' | 'userRowHeights'>;
};

export interface TableSettings {
quickFilters?: readonly [
number,
{
text: string;
}
][];
advancedFilters?: readonly [
number,
{
options: AdvancedFilterOptions;
}
][];
inputFilters?: readonly InputFilter[];
sorts?: readonly {
column: ModelIndex;
isAbs: boolean;
direction: SortDirection;
Zhou-Ziheng marked this conversation as resolved.
Show resolved Hide resolved
}[];
partition?: unknown;
partitionColumn?: ColumnName;
}
export type DehydratedPendingDataMap<T> = [number, { data: [string, T][] }][];

export type DehydratedAdvancedFilter = [
Expand Down Expand Up @@ -864,14 +886,7 @@ class IrisGridUtils {
*/
static applyTableSettings(
table: Table,
tableSettings: {
quickFilters?: readonly DehydratedQuickFilter[];
advancedFilters?: readonly DehydratedAdvancedFilter[];
inputFilters?: readonly InputFilter[];
sorts?: readonly DehydratedSort[];
partition?: unknown;
partitionColumn?: ColumnName;
},
tableSettings: TableSettings,
timeZone: string
): void {
const { columns } = table;
Expand Down
2 changes: 1 addition & 1 deletion packages/jsapi-types/src/dh.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export interface Figure extends Evented {
readonly EVENT_SERIES_ADDED: string;

/** Given a client-created figure descriptor, generate a figure that can be subscribed to */
create(figure: Partial<FigureDescriptor>): Figure;
create(figure: Partial<FigureDescriptor>): Promise<Figure>;

readonly title: string;
readonly titleFont: string;
Expand Down
165 changes: 94 additions & 71 deletions packages/redux/src/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,90 +1,113 @@
import type {
RootState,
User,
Storage,
Workspace,
WorkspaceSettings,
} from './store';
import type { RootState, Storage, WorkspaceSettings } from './store';

const EMPTY_OBJECT = Object.freeze({});

const EMPTY_MAP: ReadonlyMap<unknown, unknown> = new Map();

type Selector<R> = (state: RootState) => R;

export type Selector<State extends RootState, R> = (store: State) => R;
// User
export const getUser: Selector<User> = store => store.user;
export const getUser = <State extends RootState = RootState>(
store: State
): State['user'] => store.user;

export const getUserName: Selector<User['name']> = store => getUser(store).name;
export const getUserName = <State extends RootState = RootState>(
store: State
): State['user']['name'] => getUser(store).name;

export const getUserGroups: Selector<User['groups']> = store =>
getUser(store).groups;
export const getUserGroups = <State extends RootState = RootState>(
store: State
): State['user']['groups'] => getUser(store).groups;

// Storage
export const getStorage: Selector<Storage> = store => store.storage;
export const getStorage = <State extends RootState = RootState>(
store: State
): State['storage'] => store.storage;

export const getCommandHistoryStorage: Selector<
Storage['commandHistoryStorage']
> = store => getStorage(store).commandHistoryStorage;
export const getCommandHistoryStorage = <State extends RootState = RootState>(
store: State
): State['storage']['commandHistoryStorage'] =>
getStorage(store).commandHistoryStorage;

export const getFileStorage: Selector<Storage['fileStorage']> = store =>
getStorage(store).fileStorage;
export const getFileStorage = <State extends RootState = RootState>(
store: State
): State['storage']['fileStorage'] => getStorage(store).fileStorage;

export const getWorkspaceStorage: Selector<
Storage['workspaceStorage']
> = store => getStorage(store).workspaceStorage;
export const getWorkspaceStorage = <State extends RootState = RootState>(
store: State
): Storage['workspaceStorage'] => getStorage(store).workspaceStorage;

// Workspace
export const getWorkspace: Selector<Workspace> = store => store.workspace;
export const getWorkspace = <State extends RootState = RootState>(
store: State
): RootState['workspace'] => store.workspace;

// Settings
export const getSettings: Selector<WorkspaceSettings> = store =>
export const getSettings = <State extends RootState = RootState>(
store: State
): RootState['workspace']['data']['settings'] =>
getWorkspace(store).data.settings;

export const getDefaultDateTimeFormat: Selector<
WorkspaceSettings['defaultDateTimeFormat']
> = store => getSettings(store).defaultDateTimeFormat;

export const getDefaultDecimalFormatOptions: Selector<
WorkspaceSettings['defaultDecimalFormatOptions']
> = store => getSettings(store).defaultDecimalFormatOptions ?? EMPTY_OBJECT;

export const getDefaultIntegerFormatOptions: Selector<
WorkspaceSettings['defaultIntegerFormatOptions']
> = store => getSettings(store).defaultIntegerFormatOptions ?? EMPTY_OBJECT;

export const getFormatter: Selector<WorkspaceSettings['formatter']> = store =>
getSettings(store).formatter;

export const getTimeZone: Selector<WorkspaceSettings['timeZone']> = store =>
getSettings(store).timeZone;

export const getShowTimeZone: Selector<
WorkspaceSettings['showTimeZone']
> = store => getSettings(store).showTimeZone;

export const getShowTSeparator: Selector<
WorkspaceSettings['showTSeparator']
> = store => getSettings(store).showTSeparator;

export const getTruncateNumbersWithPound: Selector<
WorkspaceSettings['truncateNumbersWithPound']
> = store => getSettings(store).truncateNumbersWithPound;

export const getDisableMoveConfirmation: Selector<
WorkspaceSettings['disableMoveConfirmation']
> = store => getSettings(store).disableMoveConfirmation || false;

export const getShortcutOverrides: Selector<
WorkspaceSettings['shortcutOverrides']
> = store => getSettings(store).shortcutOverrides;

export const getDefaultNotebookSettings: Selector<
WorkspaceSettings['defaultNotebookSettings']
> = store => getSettings(store).defaultNotebookSettings ?? EMPTY_OBJECT;

export const getActiveTool: Selector<RootState['activeTool']> = store =>
store.activeTool;

export const getPlugins: Selector<RootState['plugins']> = store =>
store.plugins ?? EMPTY_MAP;
export const getDefaultDateTimeFormat = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['defaultDateTimeFormat'] =>
getSettings(store).defaultDateTimeFormat;

export const getDefaultDecimalFormatOptions = <
State extends RootState = RootState
>(
store: State
): WorkspaceSettings['defaultDecimalFormatOptions'] =>
getSettings(store).defaultDecimalFormatOptions ?? EMPTY_OBJECT;

export const getDefaultIntegerFormatOptions = <
State extends RootState = RootState
>(
store: State
): WorkspaceSettings['defaultIntegerFormatOptions'] =>
getSettings(store).defaultIntegerFormatOptions ?? EMPTY_OBJECT;

export const getFormatter = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['formatter'] => getSettings(store).formatter;

export const getTimeZone = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['timeZone'] => getSettings(store).timeZone;

export const getShowTimeZone = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['showTimeZone'] => getSettings(store).showTimeZone;

export const getShowTSeparator = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['showTSeparator'] => getSettings(store).showTSeparator;

export const getTruncateNumbersWithPound = <
State extends RootState = RootState
>(
store: State
): WorkspaceSettings['truncateNumbersWithPound'] =>
getSettings(store).truncateNumbersWithPound;

export const getDisableMoveConfirmation = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['disableMoveConfirmation'] =>
getSettings(store).disableMoveConfirmation || false;

export const getShortcutOverrides = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['shortcutOverrides'] =>
getSettings(store).shortcutOverrides;

export const getDefaultNotebookSettings = <State extends RootState = RootState>(
store: State
): WorkspaceSettings['defaultNotebookSettings'] =>
getSettings(store).defaultNotebookSettings ?? EMPTY_OBJECT;

export const getActiveTool = <State extends RootState = RootState>(
store: State
): RootState['activeTool'] => store.activeTool;

export const getPlugins = <State extends RootState = RootState>(
store: State
): RootState['plugins'] => store.plugins ?? EMPTY_MAP;