rusackas commented on code in PR #42535:
URL: https://github.com/apache/superset/pull/42535#discussion_r3706250214
##########
superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx:
##########
@@ -342,38 +366,53 @@ export function MultiEnumControl(props: ControlProps) {
(arraySchema.items as Record<string, unknown>) ??
({} as Record<string, unknown>);
- const enumValues = (itemsSchema.enum as unknown[]) ?? [];
- const enumNames =
- (itemsSchema['x-enumNames'] as string[]) ?? enumValues.map(String);
+ // No fallback allocations out here: a fresh ``[]`` per render would give
+ // the memo new deps every time. Fallbacks live inside the memo.
+ const enumValues = Array.isArray(itemsSchema.enum)
+ ? (itemsSchema.enum as unknown[])
+ : undefined;
+ const enumNames = Array.isArray(itemsSchema['x-enumNames'])
+ ? (itemsSchema['x-enumNames'] as string[])
+ : undefined;
- const options = enumValues.map((value, index) => ({
- value: value as string | number,
- label: enumNames[index] ?? String(value),
- }));
+ // Memoized: the host form passes a fresh ``config`` to every control on
+ // each change, so without the memo a catalog of N options is rebuilt on
+ // every one of the user's M selections (O(N·M)).
+ const options = useMemo(
+ () =>
+ (enumValues ?? []).map((value, index) => ({
+ value: value as string | number,
+ label: enumNames?.[index] ?? String(value),
+ })),
+ [enumValues, enumNames],
+ );
Review Comment:
That cast predates this PR, MultiEnumControl already asserted `string |
number` before the migration. This control only ever renders semantic-layer
metric/dimension catalogs, which are always strings, so I don't think it's
worth chasing down here.
##########
superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx:
##########
@@ -342,38 +366,53 @@ export function MultiEnumControl(props: ControlProps) {
(arraySchema.items as Record<string, unknown>) ??
({} as Record<string, unknown>);
- const enumValues = (itemsSchema.enum as unknown[]) ?? [];
- const enumNames =
- (itemsSchema['x-enumNames'] as string[]) ?? enumValues.map(String);
+ // No fallback allocations out here: a fresh ``[]`` per render would give
+ // the memo new deps every time. Fallbacks live inside the memo.
+ const enumValues = Array.isArray(itemsSchema.enum)
+ ? (itemsSchema.enum as unknown[])
+ : undefined;
+ const enumNames = Array.isArray(itemsSchema['x-enumNames'])
+ ? (itemsSchema['x-enumNames'] as string[])
+ : undefined;
- const options = enumValues.map((value, index) => ({
- value: value as string | number,
- label: enumNames[index] ?? String(value),
- }));
+ // Memoized: the host form passes a fresh ``config`` to every control on
+ // each change, so without the memo a catalog of N options is rebuilt on
+ // every one of the user's M selections (O(N·M)).
+ const options = useMemo(
+ () =>
+ (enumValues ?? []).map((value, index) => ({
+ value: value as string | number,
+ label: enumNames?.[index] ?? String(value),
+ })),
+ [enumValues, enumNames],
+ );
const value = Array.isArray(props.data) ? (props.data as unknown[]) : [];
const tooltip = (props.uischema?.options as Record<string, unknown>)
?.tooltip as string | undefined;
return (
- <Form.Item label={props.label} tooltip={tooltip}>
+ <FormItem label={props.label} tooltip={tooltip}>
<Select
+ ariaLabel={props.label || undefined}
mode="multiple"
value={value as (string | number)[]}
onChange={next => props.handleChange(props.path, next)}
options={options}
- style={{ width: '100%' }}
disabled={!props.enabled}
loading={!!refreshingSchema}
allowClear
- optionFilterProp="label"
+ // Stability fix only: the wrapped Select's bulk select-all/clear
+ // affordance is out of scope for this control (sc-107832).
+ allowSelectAll={false}
+ optionFilterProps={['label']}
Review Comment:
The prior antd Select had `optionFilterProp="label"` too, so this isn't a
behavior change, just the wrapped Select's array-based API for the same
label-only filter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
