Copilot commented on code in PR #44955:
URL: https://github.com/apache/superset/pull/44955#discussion_r4186908127
##########
superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx:
##########
@@ -147,7 +148,7 @@ export default function ColumnConfigControl<T extends
ColumnConfig>({
<ColumnConfigItem
key={col.name}
Review Comment:
React list keys should be stable and unique. Using `col.name` (a display
alias) can create collisions (e.g., multiple columns could share a label) and
can change across aliasing/renames, which may cause incorrect reuse/remount of
items. Use the real column identifier (`col.key`) for the React `key` prop
instead.
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts:
##########
@@ -341,10 +344,18 @@ export function normalizeColumnConfigKeys<T>(
return {};
}
const colnamesSet = new Set(colnames);
+ const aliasToKey = new Map(
+ colnames
+ .filter(col => COLUMN_NAME_ALIASES[col])
+ .map(col => [COLUMN_NAME_ALIASES[col], col]),
+ );
Review Comment:
The alias-to-key map silently drops collisions if two columns share the same
display alias, since later entries overwrite earlier ones in `Map`. If
collisions are possible in `COLUMN_NAME_ALIASES`, consider detecting duplicates
(and either refusing to remap ambiguous aliases or preferring a deterministic
choice) to avoid remapping a config onto the wrong column.
--
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]
