Skip to content

Commit a1fd13c

Browse files
Mike Bridgeclaude
andcommitted
perf(datasource-editor): fix typing lag and scrambling in form fields
Typing into any Dataset Editor text field (Settings → Basic Description, column descriptions, metric labels, SQL expressions) was visibly slow on realistic datasets because every keystroke re-rendered all seven tabs' contents and ran synchronous O(n) validation inside the setState callback. On fast typing or East-Asian IME input, characters landed in the DOM in the order renders finished rather than the order typed, manifesting as "scrambled" input. See sc-104866 for the full diagnosis. Four coordinated changes in two files: 1. Debounce validate + flush on blur. Validation (duplicate-name checks, currency format, folder consistency) is moved off the keystroke hot path via lodash debounce (300 ms window). A new flushValidation method is exposed as onBlur on the DatasourceContainer wrapper so focus leaving any field flushes pending validation synchronously, keeping the Save-button error gate accurate (errors.length > 0 disables save). componentWillUnmount cancels the pending invocation to avoid setState-on-unmounted warnings. 2. Lazy-mount hidden tabs via destroyInactiveTabPane so only the visible tab's children are mounted. Eliminates the largest source of per-keystroke render cost on wide datasets where ColumnCollectionTable and MetricsTab would otherwise re-render on every setState. 3. Memoise derived arrays (sortedMetrics, datetimeColumns, stringColumns) with one-pair (lastInput, lastOutput) caches so downstream PureComponent / React.memo children stop re-rendering on unrelated state changes. Also hoist three inline itemGenerator={() => ({...})} lambdas to stable class-field arrow functions (newSpatialItem, newMetricItem, newCalculatedColumnItem). 4. IME composition guards in TextAreaControl. Track isComposing state via onCompositionStart / onCompositionEnd, skip handleChange while composing, propagate once on compositionend. Reduces per-IME-word work from N candidate events to one commit. Also converts handleChange to an arrow class field so onChange={this.handleChange} has stable identity across renders (no more .bind(this) in render). Validation semantics are preserved (FR-008): same validators, same error messages, same Save-button gate — only the schedule changes. Existing 34 Jest tests across DatasourceEditor, DatasourceEditorCurrency, and TextAreaControl pass unmodified. Playwright coverage for SC-001, SC-003, and SC-005 deferred to a follow-up commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4c4905f commit a1fd13c

2 files changed

Lines changed: 157 additions & 33 deletions

File tree

superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx

Lines changed: 125 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
import rison from 'rison';
2020
import { PureComponent, useCallback, type ReactNode } from 'react';
21+
import { debounce } from 'lodash';
2122
import { connect, ConnectedProps } from 'react-redux';
2223
import type { JsonObject } from '@superset-ui/core';
2324
import { type SupersetTheme } from '@apache-superset/core/theme';
@@ -895,6 +896,25 @@ class DatasourceEditor extends PureComponent<
895896

896897
private abortControllers: AbortControllers;
897898

899+
private debouncedValidateAndChange!: ReturnType<typeof debounce<() => void>>;
900+
901+
private _sortedMetricsCache: {
902+
input: Metric[];
903+
output: Metric[];
904+
} | null = null;
905+
906+
private _datetimeColumnsCache: {
907+
dbCols: Column[];
908+
calcCols: Column[];
909+
output: { value: string; label: string }[];
910+
} | null = null;
911+
912+
private _stringColumnsCache: {
913+
dbCols: Column[];
914+
calcCols: Column[];
915+
output: { value: string; label: string }[];
916+
} | null = null;
917+
898918
static defaultProps = {
899919
onChange: () => {},
900920
setIsEditing: () => {},
@@ -990,6 +1010,15 @@ class DatasourceEditor extends PureComponent<
9901010
this.formatSql = this.formatSql.bind(this);
9911011
this.fetchUsageData = this.fetchUsageData.bind(this);
9921012
this.handleFoldersChange = this.handleFoldersChange.bind(this);
1013+
1014+
// Validation is moved off the keystroke hot path. A 300 ms window keeps
1015+
// feedback inside the typical between-word pause while letting React
1016+
// commit each keystroke's state update without blocking on O(n)
1017+
// duplicate/currency/folder checks.
1018+
this.debouncedValidateAndChange = debounce(
1019+
() => this.validate(this.onChange),
1020+
300,
1021+
);
9931022
}
9941023

9951024
onChange() {
@@ -1081,9 +1110,13 @@ class DatasourceEditor extends PureComponent<
10811110
}
10821111

10831112
validateAndChange() {
1084-
this.validate(this.onChange);
1113+
this.debouncedValidateAndChange();
10851114
}
10861115

1116+
flushValidation = () => {
1117+
this.debouncedValidateAndChange.flush();
1118+
};
1119+
10871120
async onQueryRun() {
10881121
const databaseId = this.state.datasource.database?.id;
10891122
const { sql } = this.state.datasource;
@@ -1464,21 +1497,56 @@ class DatasourceEditor extends PureComponent<
14641497
);
14651498
}
14661499

1467-
renderDefaultColumnSettings() {
1468-
const { datasource, databaseColumns, calculatedColumns } = this.state;
1469-
const { theme } = this.props;
1470-
const allColumns = [...databaseColumns, ...calculatedColumns];
1500+
// Cached getters for derived arrays. Each returns a stable reference while
1501+
// its inputs' identities are unchanged, so downstream PureComponent /
1502+
// React.memo children stop re-rendering on unrelated state changes.
1503+
1504+
getSortedMetrics(metrics: Metric[] | undefined): Metric[] {
1505+
const safeMetrics = metrics ?? [];
1506+
if (this._sortedMetricsCache?.input === safeMetrics) {
1507+
return this._sortedMetricsCache.output;
1508+
}
1509+
const output = safeMetrics.length
1510+
? [...safeMetrics].sort(
1511+
({ id: a }: { id?: number }, { id: b }: { id?: number }) =>
1512+
(b ?? 0) - (a ?? 0),
1513+
)
1514+
: [];
1515+
this._sortedMetricsCache = { input: safeMetrics, output };
1516+
return output;
1517+
}
14711518

1472-
// Get datetime-compatible columns for the default datetime dropdown
1473-
const datetimeColumns = allColumns
1519+
getDatetimeColumns(
1520+
dbCols: Column[],
1521+
calcCols: Column[],
1522+
): { value: string; label: string }[] {
1523+
if (
1524+
this._datetimeColumnsCache?.dbCols === dbCols &&
1525+
this._datetimeColumnsCache?.calcCols === calcCols
1526+
) {
1527+
return this._datetimeColumnsCache.output;
1528+
}
1529+
const output = [...dbCols, ...calcCols]
14741530
.filter(col => col.is_dttm)
14751531
.map(col => ({
14761532
value: col.column_name,
14771533
label: col.verbose_name || col.column_name,
14781534
}));
1535+
this._datetimeColumnsCache = { dbCols, calcCols, output };
1536+
return output;
1537+
}
14791538

1480-
// String columns + untyped calculated columns for the currency code dropdown
1481-
const stringColumns = allColumns
1539+
getStringColumns(
1540+
dbCols: Column[],
1541+
calcCols: Column[],
1542+
): { value: string; label: string }[] {
1543+
if (
1544+
this._stringColumnsCache?.dbCols === dbCols &&
1545+
this._stringColumnsCache?.calcCols === calcCols
1546+
) {
1547+
return this._stringColumnsCache.output;
1548+
}
1549+
const output = [...dbCols, ...calcCols]
14821550
.filter(
14831551
col =>
14841552
col.type_generic === GenericDataType.String ||
@@ -1488,6 +1556,42 @@ class DatasourceEditor extends PureComponent<
14881556
value: col.column_name,
14891557
label: col.verbose_name || col.column_name,
14901558
}));
1559+
this._stringColumnsCache = { dbCols, calcCols, output };
1560+
return output;
1561+
}
1562+
1563+
newSpatialItem = () => ({
1564+
name: t('<new spatial>'),
1565+
type: t('<no type>'),
1566+
config: null,
1567+
});
1568+
1569+
newMetricItem = () => ({
1570+
metric_name: t('<new metric>'),
1571+
verbose_name: '',
1572+
expression: '',
1573+
});
1574+
1575+
newCalculatedColumnItem = () => ({
1576+
column_name: t('<new column>'),
1577+
filterable: true,
1578+
groupby: true,
1579+
expression: t('<enter SQL expression here>'),
1580+
expanded: true,
1581+
});
1582+
1583+
renderDefaultColumnSettings() {
1584+
const { datasource, databaseColumns, calculatedColumns } = this.state;
1585+
const { theme } = this.props;
1586+
1587+
const datetimeColumns = this.getDatetimeColumns(
1588+
databaseColumns,
1589+
calculatedColumns,
1590+
);
1591+
const stringColumns = this.getStringColumns(
1592+
databaseColumns,
1593+
calculatedColumns,
1594+
);
14911595

14921596
return (
14931597
<DefaultColumnSettingsContainer data-test="default-column-settings">
@@ -1708,11 +1812,7 @@ class DatasourceEditor extends PureComponent<
17081812
tableColumns={['name', 'config']}
17091813
sortColumns={['name']}
17101814
onChange={this.onDatasourcePropChange.bind(this, 'spatials')}
1711-
itemGenerator={() => ({
1712-
name: t('<new spatial>'),
1713-
type: t('<no type>'),
1714-
config: null,
1715-
})}
1815+
itemGenerator={this.newSpatialItem}
17161816
collection={spatials ?? []}
17171817
allowDeletes
17181818
itemRenderers={{
@@ -2148,7 +2248,7 @@ class DatasourceEditor extends PureComponent<
21482248
renderMetricCollection() {
21492249
const { datasource, metricSearchTerm } = this.state;
21502250
const { metrics } = datasource;
2151-
const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : [];
2251+
const sortedMetrics = this.getSortedMetrics(metrics);
21522252
return (
21532253
<div>
21542254
<Input.Search
@@ -2271,11 +2371,7 @@ class DatasourceEditor extends PureComponent<
22712371
collection={sortedMetrics}
22722372
allowAddItem
22732373
onChange={this.onDatasourcePropChange.bind(this, 'metrics')}
2274-
itemGenerator={() => ({
2275-
metric_name: t('<new metric>'),
2276-
verbose_name: '',
2277-
expression: '',
2278-
})}
2374+
itemGenerator={this.newMetricItem}
22792375
itemCellProps={{
22802376
expression: () => ({
22812377
style: {
@@ -2349,10 +2445,13 @@ class DatasourceEditor extends PureComponent<
23492445
render() {
23502446
const { datasource, activeTabKey } = this.state;
23512447
const { metrics } = datasource;
2352-
const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : [];
2448+
const sortedMetrics = this.getSortedMetrics(metrics);
23532449

23542450
return (
2355-
<DatasourceContainer data-test="datasource-editor">
2451+
<DatasourceContainer
2452+
data-test="datasource-editor"
2453+
onBlur={this.flushValidation}
2454+
>
23562455
{this.renderErrors()}
23572456
<Alert
23582457
css={theme => ({ marginBottom: theme.sizeUnit * 4 })}
@@ -2372,6 +2471,7 @@ class DatasourceEditor extends PureComponent<
23722471
data-test="edit-dataset-tabs"
23732472
onChange={this.handleTabSelect}
23742473
defaultActiveKey={activeTabKey}
2474+
destroyInactiveTabPane
23752475
items={[
23762476
{
23772477
key: TABS_KEYS.SOURCE,
@@ -2479,13 +2579,7 @@ class DatasourceEditor extends PureComponent<
24792579
showExpression
24802580
allowAddItem
24812581
allowEditDataType
2482-
itemGenerator={() => ({
2483-
column_name: t('<new column>'),
2484-
filterable: true,
2485-
groupby: true,
2486-
expression: t('<enter SQL expression here>'),
2487-
expanded: true,
2488-
})}
2582+
itemGenerator={this.newCalculatedColumnItem}
24892583
/>
24902584
</StyledTableTabWrapper>
24912585
),
@@ -2627,6 +2721,8 @@ class DatasourceEditor extends PureComponent<
26272721
componentWillUnmount() {
26282722
this.isComponentMounted = false;
26292723

2724+
this.debouncedValidateAndChange.cancel();
2725+
26302726
// Abort all pending requests
26312727
Object.values(this.abortControllers).forEach(controller => {
26322728
if (controller) controller.abort();

superset-frontend/src/explore/components/controls/TextAreaControl.tsx

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,36 @@ class TextAreaControl extends Component<TextAreaControlProps> {
101101
| ReturnType<typeof debounce<(value: string) => void>>
102102
| undefined;
103103

104+
// East-Asian IMEs fire onChange per candidate character during composition.
105+
// Suppressing them reduces the per-IME-word cost from N to 1; only the
106+
// committed composition (on compositionend) propagates to the parent.
107+
private isComposing = false;
108+
104109
constructor(props: TextAreaControlProps) {
105110
super(props);
106111
if (props.debounceDelay && props.onChange) {
107112
this.debouncedOnChange = debounce(props.onChange, props.debounceDelay);
108113
}
109114
}
110115

116+
handleCompositionStart = () => {
117+
this.isComposing = true;
118+
};
119+
120+
handleCompositionEnd = (
121+
e: React.CompositionEvent<HTMLTextAreaElement | HTMLInputElement>,
122+
) => {
123+
this.isComposing = false;
124+
const target = e.target as HTMLTextAreaElement | HTMLInputElement;
125+
if (target?.value != null) {
126+
if (this.debouncedOnChange) {
127+
this.debouncedOnChange(target.value);
128+
} else {
129+
this.props.onChange?.(target.value);
130+
}
131+
}
132+
};
133+
111134
componentDidUpdate(prevProps: TextAreaControlProps) {
112135
if (
113136
this.props.onChange !== prevProps.onChange &&
@@ -124,14 +147,17 @@ class TextAreaControl extends Component<TextAreaControlProps> {
124147
}
125148
}
126149

127-
handleChange(value: string | { target: { value: string } }) {
150+
handleChange = (value: string | { target: { value: string } }) => {
151+
// Skip intermediate IME candidate events. The final committed value is
152+
// propagated once from handleCompositionEnd.
153+
if (this.isComposing) return;
128154
const finalValue = typeof value === 'object' ? value.target.value : value;
129155
if (this.debouncedOnChange) {
130156
this.debouncedOnChange(finalValue);
131157
} else {
132158
this.props.onChange?.(finalValue);
133159
}
134-
}
160+
};
135161

136162
componentWillUnmount() {
137163
if (this.debouncedOnChange) {
@@ -211,7 +237,7 @@ class TextAreaControl extends Component<TextAreaControlProps> {
211237
readOnly={readOnly}
212238
key={name}
213239
{...editorProps}
214-
onChange={this.handleChange.bind(this)}
240+
onChange={this.handleChange}
215241
/>
216242
</div>
217243
);
@@ -226,7 +252,9 @@ class TextAreaControl extends Component<TextAreaControlProps> {
226252
<div>
227253
<Input.TextArea
228254
placeholder={t('textarea')}
229-
onChange={this.handleChange.bind(this)}
255+
onChange={this.handleChange}
256+
onCompositionStart={this.handleCompositionStart}
257+
onCompositionEnd={this.handleCompositionEnd}
230258
defaultValue={this.props.initialValue}
231259
disabled={this.props.readOnly}
232260
style={{ height: this.props.height }}

0 commit comments

Comments
 (0)