Skip to content

Commit c3ea867

Browse files
authored
perf: remove workspace dependancy from iris-grid-panel and memoize settings redux selector (#1982)
Discovered as part of #1977 investigation. Typing in a notebook triggers a workspace update, which was triggering all tables to re-render. Removed deprecated dependeny of workspace from iris-grid. Further settings is derived from workspace and returns a new settings object. Iris-grid depends on settings, but the object had no changes. Memoize the selector so it always returns the same object if settings haven't changed. Removes ~100ms of lag that was occuring 400ms after the debounce of notebook key press events. BREAKING CHANGE: getPluginContent deprecatedProps have been removed from iris-grid
1 parent dc29aa9 commit c3ea867

4 files changed

Lines changed: 50 additions & 45 deletions

File tree

package-lock.json

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/dashboard-core-plugins/src/panels/IrisGridPanel.tsx

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import {
4343
DehydratedIrisGridPanelState,
4444
ColumnHeaderGroup,
4545
IrisGridContextMenuData,
46-
IrisGridTableModel,
4746
PartitionConfig,
4847
} from '@deephaven/iris-grid';
4948
import {
@@ -54,10 +53,8 @@ import {
5453
} from '@deephaven/jsapi-utils';
5554
import Log from '@deephaven/log';
5655
import {
57-
CustomizableWorkspace,
5856
getSettings,
5957
getUser,
60-
getWorkspace,
6158
RootState,
6259
User,
6360
WorkspaceSettings,
@@ -67,10 +64,7 @@ import {
6764
CancelablePromise,
6865
PromiseUtils,
6966
} from '@deephaven/utils';
70-
import {
71-
ContextMenuRoot,
72-
ResolvableContextAction,
73-
} from '@deephaven/components';
67+
import { ResolvableContextAction } from '@deephaven/components';
7468
import type { dh } from '@deephaven/jsapi-types';
7569
import {
7670
GridState,
@@ -101,8 +95,6 @@ const log = Log.module('IrisGridPanel');
10195

10296
const DEBOUNCE_PANEL_STATE_UPDATE = 500;
10397

104-
const PLUGIN_COMPONENTS = { IrisGrid, IrisGridTableModel, ContextMenuRoot };
105-
10698
type ModelQueueFunction = (model: IrisGridModel) => void;
10799

108100
type ModelQueue = ModelQueueFunction[];
@@ -157,7 +149,6 @@ interface StateProps {
157149
tableColumn?: LinkColumn
158150
) => boolean;
159151
user: User;
160-
workspace: CustomizableWorkspace;
161152
settings: WorkspaceSettings;
162153
}
163154

@@ -420,8 +411,6 @@ export class IrisGridPanel extends PureComponent<
420411
(
421412
Plugin: TablePluginComponent | undefined,
422413
model: IrisGridModel | undefined,
423-
user: User,
424-
workspace: CustomizableWorkspace,
425414
pluginState: unknown
426415
) => {
427416
if (
@@ -433,19 +422,6 @@ export class IrisGridPanel extends PureComponent<
433422
return null;
434423
}
435424

436-
// The panel in the deprecated props makes an ugly dependency of the plugin on the panel
437-
// Since we didn't have TS when the old plugins would have been implemented,
438-
// just pass the deprecated props without type checking
439-
// so we can break the ugly dependency of plugin on panel
440-
const deprecatedProps = {
441-
panel: this,
442-
onFilter: this.handlePluginFilter,
443-
onFetchColumns: this.handlePluginFetchColumns,
444-
user,
445-
workspace,
446-
components: PLUGIN_COMPONENTS,
447-
};
448-
449425
return (
450426
<div className="iris-grid-plugin">
451427
<Plugin
@@ -456,8 +432,6 @@ export class IrisGridPanel extends PureComponent<
456432
table={model.table}
457433
onStateChange={this.handlePluginStateChange}
458434
pluginState={pluginState}
459-
// eslint-disable-next-line react/jsx-props-no-spreading
460-
{...deprecatedProps}
461435
/>
462436
</div>
463437
);
@@ -1217,7 +1191,6 @@ export class IrisGridPanel extends PureComponent<
12171191
metadata,
12181192
panelState,
12191193
user,
1220-
workspace,
12211194
settings,
12221195
theme,
12231196
} = this.props;
@@ -1266,8 +1239,7 @@ export class IrisGridPanel extends PureComponent<
12661239
const description = model?.description ?? undefined;
12671240
const pluginState = panelState?.pluginState ?? null;
12681241
const childrenContent =
1269-
children ??
1270-
this.getPluginContent(Plugin, model, user, workspace, pluginState);
1242+
children ?? this.getPluginContent(Plugin, model, pluginState);
12711243
const { permissions } = user;
12721244
const { canCopy, canDownloadCsv } = permissions;
12731245

@@ -1373,7 +1345,6 @@ const mapStateToProps = (
13731345
localDashboardId
13741346
),
13751347
user: getUser(state),
1376-
workspace: getWorkspace(state),
13771348
settings: getSettings(state),
13781349
});
13791350

packages/redux/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"@deephaven/log": "file:../log",
2828
"@deephaven/plugin": "file:../plugin",
2929
"fast-deep-equal": "^3.1.3",
30+
"proxy-memoize": "^3.0.0",
3031
"redux-thunk": "2.4.1"
3132
},
3233
"peerDependencies": {

packages/redux/src/selectors.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { UndoPartial } from '@deephaven/utils';
2+
import { memoize } from 'proxy-memoize';
23
import type { RootState, WorkspaceSettings } from './store';
34

45
const EMPTY_OBJECT = Object.freeze({});
@@ -55,22 +56,24 @@ export const getWorkspace = <State extends RootState>(
5556
};
5657

5758
// Settings
58-
export const getSettings = <State extends RootState>(
59-
store: State
60-
): UndoPartial<State['workspace']['data']['settings']> => {
61-
const customizedSettings = getWorkspace(store)?.data.settings ?? {};
62-
63-
const settings = { ...getDefaultWorkspaceSettings(store) };
64-
const keys = Object.keys(customizedSettings) as (keyof WorkspaceSettings)[];
65-
for (let i = 0; i < keys.length; i += 1) {
66-
const key = keys[i];
67-
if (customizedSettings[key] !== undefined) {
68-
// @ts-expect-error assign non-undefined customized settings to settings
69-
settings[key] = customizedSettings[key];
59+
export const getSettings = memoize(
60+
<State extends RootState>(
61+
store: State
62+
): UndoPartial<State['workspace']['data']['settings']> => {
63+
const customizedSettings = getWorkspace(store)?.data.settings ?? {};
64+
65+
const settings = { ...getDefaultWorkspaceSettings(store) };
66+
const keys = Object.keys(customizedSettings) as (keyof WorkspaceSettings)[];
67+
for (let i = 0; i < keys.length; i += 1) {
68+
const key = keys[i];
69+
if (customizedSettings[key] !== undefined) {
70+
// @ts-expect-error assign non-undefined customized settings to settings
71+
settings[key] = customizedSettings[key];
72+
}
7073
}
74+
return settings as UndoPartial<State['workspace']['data']['settings']>;
7175
}
72-
return settings as UndoPartial<State['workspace']['data']['settings']>;
73-
};
76+
);
7477

7578
export const getDefaultSettings = <State extends RootState>(
7679
store: State

0 commit comments

Comments
 (0)