EnxDev commented on code in PR #41754:
URL: https://github.com/apache/superset/pull/41754#discussion_r3529936471
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -113,26 +114,58 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
}
}, [columns]);
+ // A single effect owns every ownState write derived from render state.
+ // updateTableOwnState replaces ownState wholesale, so separate effects that
+ // each spread serverPaginationData in the same render would clobber one
+ // another's keys: clamping the current page, priming the raw-mode summary
+ // columns and nudging a re-query for missing totals must be one combined
+ // delta.
useEffect(() => {
- if (!serverPagination || !serverPaginationData || !rowCount) return;
+ const nextOwnState = { ...serverPaginationData };
+ let changed = false;
+
+ if (serverPagination && serverPaginationData && rowCount !== undefined) {
+ const currentPage = serverPaginationData.currentPage ?? 0;
+ const currentPageSize = serverPaginationData.pageSize ??
serverPageLength;
+ const totalPages = Math.ceil(rowCount / currentPageSize);
+ // An empty result set clamps to page zero; a shrunken one clamps to its
+ // last remaining page.
+ const clampedPage = Math.max(0, Math.min(currentPage, totalPages - 1));
+ if (clampedPage !== currentPage) {
+ nextOwnState.currentPage = clampedPage;
+ changed = true;
+ }
+ }
- const currentPage = serverPaginationData.currentPage ?? 0;
- const currentPageSize = serverPaginationData.pageSize ?? serverPageLength;
- const totalPages = Math.ceil(rowCount / currentPageSize);
+ const primed = (serverPaginationData?.rawSummaryColumns ?? []) as string[];
+ const requested = Boolean(serverPaginationData?.totalsRequested);
+ if (isRawRecords && showTotals && !isEqual(primed, rawSummaryColumns)) {
+ nextOwnState.rawSummaryColumns = rawSummaryColumns;
+ changed = true;
+ }
+ // A renderTrigger toggle re-renders without re-querying; requesting totals
+ // through ownState dispatches the standard re-query whose buildQuery
+ // carries the totals query for the active mode.
+ if (showTotals && totals === undefined && !requested) {
Review Comment:
yes, conscious call, with these bounds: a saved aggregate + summary chart
does zero extra work (totals ride the initial formData-driven query; the nudge
only fires in the renderTrigger gap). A saved raw + summary chart pays one
priming re-query on first load — the base query is a server-side cache hit,
only the totals query is new work. The toggle-off reset query is the deliberate
trade-off: the alternative (clearing the flag when data arrives) would fire an
extra query after every totals fetch, which is strictly worse; toggle-off stays
visually instant regardless because the grid gate hides the row before the
query returns.
--
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]
