rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2770062865
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -157,58 +223,76 @@ function convertToArray(
return result;
}
-export class TableRenderer extends Component {
- constructor(props) {
+export class TableRenderer extends Component<
+ TableRendererProps,
+ TableRendererState
+> {
+ sortCache: Map<string, string[][]>;
+ cachedProps: TableRendererProps | null;
+ cachedBasePivotSettings: PivotSettings | null;
+
+ static propTypes: Record<string, unknown>;
+ static defaultProps: Record<string, unknown>;
+
+ constructor(props: TableRendererProps) {
super(props);
// We need state to record which entries are collapsed and which aren't.
// This is an object with flat-keys indicating if the corresponding rows
// should be collapsed.
this.state = { collapsedRows: {}, collapsedCols: {}, sortingOrder: [] };
this.sortCache = new Map();
+ this.cachedProps = null;
+ this.cachedBasePivotSettings = null;
this.clickHeaderHandler = this.clickHeaderHandler.bind(this);
this.clickHandler = this.clickHandler.bind(this);
}
- getBasePivotSettings() {
+ getBasePivotSettings(): PivotSettings {
// One-time extraction of pivot settings that we'll use throughout the
render.
const { props } = this;
const colAttrs = props.cols;
const rowAttrs = props.rows;
- const tableOptions = {
+ const tableOptions: Record<string, unknown> = {
rowTotals: true,
colTotals: true,
...props.tableOptions,
};
- const rowTotals = tableOptions.rowTotals || colAttrs.length === 0;
- const colTotals = tableOptions.colTotals || rowAttrs.length === 0;
+ const rowTotals =
+ (tableOptions.rowTotals as boolean) || colAttrs.length === 0;
+ const colTotals =
+ (tableOptions.colTotals as boolean) || rowAttrs.length === 0;
Review Comment:
Agreed — defining a `TableOptions` interface would eliminate these casts.
This is pre-existing untyped code from the JS era; the TS migration preserves
existing behavior with casts rather than redesigning interfaces. I'll file a
follow-up issue to type `tableOptions` / `subtotalOptions` properly.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -157,58 +223,76 @@ function convertToArray(
return result;
}
-export class TableRenderer extends Component {
- constructor(props) {
+export class TableRenderer extends Component<
+ TableRendererProps,
+ TableRendererState
+> {
+ sortCache: Map<string, string[][]>;
+ cachedProps: TableRendererProps | null;
+ cachedBasePivotSettings: PivotSettings | null;
+
+ static propTypes: Record<string, unknown>;
+ static defaultProps: Record<string, unknown>;
+
+ constructor(props: TableRendererProps) {
super(props);
// We need state to record which entries are collapsed and which aren't.
// This is an object with flat-keys indicating if the corresponding rows
// should be collapsed.
this.state = { collapsedRows: {}, collapsedCols: {}, sortingOrder: [] };
this.sortCache = new Map();
+ this.cachedProps = null;
+ this.cachedBasePivotSettings = null;
this.clickHeaderHandler = this.clickHeaderHandler.bind(this);
this.clickHandler = this.clickHandler.bind(this);
}
- getBasePivotSettings() {
+ getBasePivotSettings(): PivotSettings {
// One-time extraction of pivot settings that we'll use throughout the
render.
const { props } = this;
const colAttrs = props.cols;
const rowAttrs = props.rows;
- const tableOptions = {
+ const tableOptions: Record<string, unknown> = {
rowTotals: true,
colTotals: true,
...props.tableOptions,
};
- const rowTotals = tableOptions.rowTotals || colAttrs.length === 0;
- const colTotals = tableOptions.colTotals || rowAttrs.length === 0;
+ const rowTotals =
+ (tableOptions.rowTotals as boolean) || colAttrs.length === 0;
+ const colTotals =
+ (tableOptions.colTotals as boolean) || rowAttrs.length === 0;
const namesMapping = props.namesMapping || {};
- const subtotalOptions = {
+ const subtotalOptions: Record<string, unknown> = {
arrowCollapsed: '\u25B2',
arrowExpanded: '\u25BC',
...props.subtotalOptions,
};
const colSubtotalDisplay = {
displayOnTop: false,
- enabled: tableOptions.colSubTotals,
+ enabled: tableOptions.colSubTotals as boolean | undefined,
hideOnExpand: false,
- ...subtotalOptions.colSubtotalDisplay,
+ ...(subtotalOptions.colSubtotalDisplay as
+ | Record<string, unknown>
+ | undefined),
};
const rowSubtotalDisplay = {
displayOnTop: false,
- enabled: tableOptions.rowSubTotals,
+ enabled: tableOptions.rowSubTotals as boolean | undefined,
hideOnExpand: false,
- ...subtotalOptions.rowSubtotalDisplay,
+ ...(subtotalOptions.rowSubtotalDisplay as
+ | Record<string, unknown>
+ | undefined),
};
- const pivotData = new PivotData(props, {
- rowEnabled: rowSubtotalDisplay.enabled,
- colEnabled: colSubtotalDisplay.enabled,
+ const pivotData = new PivotData(props as Record<string, unknown>, {
+ rowEnabled: rowSubtotalDisplay.enabled as boolean | undefined,
+ colEnabled: colSubtotalDisplay.enabled as boolean | undefined,
Review Comment:
Agreed — defining a `TableOptions` interface would eliminate these casts.
This is pre-existing untyped code from the JS era; the TS migration preserves
existing behavior with casts rather than redesigning interfaces. I'll file a
follow-up issue to type `tableOptions` / `subtotalOptions` properly.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -268,8 +355,8 @@ export class TableRenderer extends Component {
rowKeys,
rowTotals,
colTotals,
- arrowCollapsed: subtotalOptions.arrowCollapsed,
- arrowExpanded: subtotalOptions.arrowExpanded,
+ arrowCollapsed: subtotalOptions.arrowCollapsed as string,
+ arrowExpanded: subtotalOptions.arrowExpanded as string,
Review Comment:
Agreed — defining a `TableOptions` interface would eliminate these casts.
This is pre-existing untyped code from the JS era; the TS migration preserves
existing behavior with casts rather than redesigning interfaces. I'll file a
follow-up issue to type `tableOptions` / `subtotalOptions` properly.
--
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]
