rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2770110925
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -168,55 +184,74 @@ const Chart = props => {
[dispatch],
);
- const chart = useSelector(state => state.charts[props.id] || EMPTY_OBJECT);
- const { queriesResponse, chartUpdateEndTime, chartStatus, annotationQuery } =
- chart;
+ const chart = useSelector((state: RootState) => state.charts[props.id]);
+ const queriesResponse = chart?.queriesResponse;
+ const chartUpdateEndTime = chart?.chartUpdateEndTime;
+ const chartStatus = chart?.chartStatus;
+ const annotationQuery = chart?.annotationQuery;
- const slice = useSelector(
- state => state.sliceEntities.slices[props.id] || EMPTY_OBJECT,
+ const slice: SliceEntity | Record<string, never> = useSelector(
+ (state: RootState) =>
+ (state.sliceEntities as { slices: Record<number, SliceEntity> }).slices[
+ props.id
+ ] || EMPTY_OBJECT,
+ );
+ const sliceVizType = (slice as SliceEntity)?.viz_type;
+ const sliceSliceId = (slice as SliceEntity)?.slice_id;
+ const sliceSliceName = (slice as SliceEntity)?.slice_name;
+ const editMode = useSelector(
+ (state: RootState) => state.dashboardState.editMode,
Review Comment:
You're right that typing `sliceEntities` properly in RootState would be the
ideal solution. The cast exists because `RootState` doesn't currently define a
specific type for `sliceEntities` — it's a reducer with a loosely typed shape.
Properly typing it would require defining a `SliceEntitiesState` interface and
updating the root reducer's type map, which is a broader state-typing
initiative beyond migration scope. Filed as a follow-up.
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -390,25 +440,29 @@ const Chart = props => {
const formData = useMemo(
() =>
getFormDataWithExtraFilters({
- chart: { id: chart.id, form_data: chart.form_data }, // avoid passing
the whole chart object
+ chart: { id: chart?.id ?? props.id, form_data: chart?.form_data }, //
avoid passing the whole chart object
chartConfiguration,
- chartCustomizationItems,
+ chartCustomizationItems:
+ chartCustomizationItems as
import('@superset-ui/core').ChartCustomization[],
Review Comment:
The inline `import('...')` syntax is used here because the type is only
needed in this one cast expression. Moving it to a top-level import would work
too but is a stylistic preference — both are valid TypeScript. The cast itself
is needed because `chartCustomizationItems` comes from a selector that returns
a loosely typed array. Aligning the selector's return type would be part of the
broader RootState typing effort.
--
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]
