Copilot commented on code in PR #37522:
URL: https://github.com/apache/superset/pull/37522#discussion_r2737955634
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -322,7 +339,17 @@ export default function transformProps(
data1,
currencyCodeColumn,
);
- const customFormattersSecondary = buildCustomFormatters(
+ const customFormattersSecondary = (
+ buildCustomFormatters as (
+ metrics: QueryFormMetric | QueryFormMetric[] | undefined,
+ savedCurrencyFormats: Record<string, Currency>,
+ savedColumnFormats: Record<string, string>,
+ d3Format: string | undefined,
+ currencyFormat: Currency | undefined | null,
+ data?: TimeseriesDataRecord[],
+ currencyCodeColumn?: string,
+ ) => ReturnType<typeof buildCustomFormatters>
+ )(
Review Comment:
The type assertion for buildCustomFormatters is unnecessary. The function
signature is already compatible with how it's being called. TypeScript should
be able to infer this correctly without the explicit cast. The type assertion
makes the code harder to maintain and could mask actual type errors.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -297,7 +303,17 @@ export default function transformProps(
currency: resolvedCurrency,
})
: getNumberFormatter(yAxisFormat);
- const customFormatters = buildCustomFormatters(
+ const customFormatters = (
+ buildCustomFormatters as (
+ metrics: QueryFormMetric | QueryFormMetric[] | undefined,
+ savedCurrencyFormats: Record<string, Currency>,
+ savedColumnFormats: Record<string, string>,
+ d3Format: string | undefined,
+ currencyFormat: Currency | undefined | null,
+ data?: TimeseriesDataRecord[],
+ currencyCodeColumn?: string,
+ ) => ReturnType<typeof buildCustomFormatters>
+ )(
Review Comment:
The type assertion for buildCustomFormatters is unnecessary. The function
signature is already compatible with how it's being called. TypeScript should
be able to infer this correctly without the explicit cast. The type assertion
makes the code harder to maintain and could mask actual type errors.
```suggestion
const customFormatters = buildCustomFormatters(
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -313,7 +320,17 @@ export default function transformProps(
currency: resolvedCurrencySecondary,
})
: getNumberFormatter(yAxisFormatSecondary);
- const customFormatters = buildCustomFormatters(
+ const customFormatters = (
+ buildCustomFormatters as (
+ metrics: QueryFormMetric | QueryFormMetric[] | undefined,
+ savedCurrencyFormats: Record<string, Currency>,
+ savedColumnFormats: Record<string, string>,
+ d3Format: string | undefined,
+ currencyFormat: Currency | undefined | null,
+ data?: TimeseriesDataRecord[],
+ currencyCodeColumn?: string,
+ ) => ReturnType<typeof buildCustomFormatters>
+ )(
Review Comment:
The type assertion for buildCustomFormatters is unnecessary. The function
signature is already compatible with how it's being called. TypeScript should
be able to infer this correctly without the explicit cast. The type assertion
makes the code harder to maintain and could mask actual type errors.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -663,7 +687,11 @@ export default function transformProps(
if (seriesType === EchartsTimeseriesSeriesType.Bar && showValue) {
padding.right = Math.max(
padding.right || 0,
- TIMESERIES_CONSTANTS.horizontalBarLabelRightPadding,
+ (
+ TIMESERIES_CONSTANTS as typeof TIMESERIES_CONSTANTS & {
+ horizontalBarLabelRightPadding: number;
+ }
+ ).horizontalBarLabelRightPadding,
Review Comment:
The type assertion for TIMESERIES_CONSTANTS is unnecessary. This property
already exists in the TIMESERIES_CONSTANTS object as defined in constants.ts.
TypeScript should be able to access this property without the assertion. The
type assertion makes the code harder to maintain and could mask actual type
errors.
```suggestion
TIMESERIES_CONSTANTS.horizontalBarLabelRightPadding,
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -327,11 +343,13 @@ export default function transformProps(
// - "metric__1 day ago" pattern (via hasTimeOffset)
// - "1 day ago, groupby" pattern (via hasTimeOffset)
// - exact match "1 day ago" (via seriesName parameter)
- const derivedSeries = isDerivedSeries(
- entry,
- chartProps.rawFormData,
- seriesName,
- );
+ const derivedSeries = (
+ isDerivedSeries as (
+ series: typeof entry,
+ formData: typeof chartProps.rawFormData,
+ seriesName?: string,
+ ) => boolean
+ )(entry, chartProps.rawFormData, seriesName);
Review Comment:
The type assertion for isDerivedSeries is unnecessary. The function
signature is already compatible with how it's being called. TypeScript should
be able to infer this correctly without the explicit cast. The type assertion
makes the code harder to maintain and could mask actual type errors.
```suggestion
const derivedSeries = isDerivedSeries(
entry,
chartProps.rawFormData,
seriesName,
);
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -371,7 +389,13 @@ export default function transformProps(
if (derivedSeries && array.includes(seriesName)) {
const originalSeries = rawSeries.find(
s =>
- !isDerivedSeries(
+ !(
+ isDerivedSeries as (
+ series: typeof s,
+ formData: typeof chartProps.rawFormData,
+ seriesName?: string,
+ ) => boolean
+ )(
Review Comment:
The type assertion for isDerivedSeries is unnecessary. The function
signature is already compatible with how it's being called. TypeScript should
be able to infer this correctly without the explicit cast. The type assertion
makes the code harder to maintain and could mask actual type 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]
