bito-code-review[bot] commented on code in PR #36760:
URL: https://github.com/apache/superset/pull/36760#discussion_r2642209870
##########
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.tsx:
##########
@@ -58,7 +78,7 @@ class AdhocMetricOption extends PureComponent {
onMetricEdit,
columns,
savedMetricsOptions,
- savedMetric,
+ savedMetric = {} as SavedMetricTypeDef,
Review Comment:
<!-- Bito Reply -->
Yes, defaulting savedMetric to {} maintains JavaScript destructuring
behavior and ensures consistent handling. The check
`Object.keys(savedMetric).length > 0` will correctly identify an empty object
as no saved metric, triggering the appropriate code path without errors.
##########
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx:
##########
@@ -216,12 +216,12 @@ class AdhocMetricPopoverTrigger extends PureComponent<
adhocMetric={adhocMetric}
columns={columns}
savedMetricsOptions={savedMetricsOptions}
- savedMetric={savedMetric}
- datasource={datasource}
+ savedMetric={savedMetric as savedMetricType}
Review Comment:
<!-- Bito Reply -->
This explanation documents the type assertion as intentional technical debt
for the JS→TS migration, noting that parent components pass either a typed
savedMetric or an empty object. The eslint-disable comment marks it for future
cleanup. However, the bot's suggestion to pass undefined instead of asserting
an empty object to savedMetricType is safer to avoid potential runtime property
access errors.
--
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]
