This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 08694d64cb8 fix(plugin-chart-table): declare DataTable hooks before
the early return (#43150)
08694d64cb8 is described below
commit 08694d64cb84076e3ef29b263dead028b2d4b293
Author: Chandan P <[email protected]>
AuthorDate: Tue Sep 8 22:43:17 2026 +0530
fix(plugin-chart-table): declare DataTable hooks before the early return
(#43150)
---
.../plugin-chart-table/src/DataTable/DataTable.tsx | 154 ++++++++++-----------
.../test/DataTable/DataTable.test.tsx | 129 +++++++++++++++++
2 files changed, 205 insertions(+), 78 deletions(-)
diff --git
a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
index 59d627eaec4..cf7ae61f03b 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
@@ -103,6 +103,42 @@ const sortTypes = {
alphanumeric: sortAlphanumericCaseInsensitive,
};
+// Prefer a stable identifier from original row data; otherwise use a
deterministic
+// concatenation of visible values (keys sorted so the result does not depend
on
+// column order).
+function stableRowKey<D extends object>(r: Row<D>): string {
+ const orig = r.original as Record<string, unknown> | undefined;
+ if (orig) {
+ const idLike = orig.id ?? orig.ID ?? orig.key ?? orig.uuid;
+ if (idLike != null) return String(idLike);
+ }
+
+ // Fallback: derive from row.values, sorting the keys so that reordering the
+ // columns does not change the key.
+ const v = r.values as Record<string, unknown>;
+ const keys = Object.keys(v).sort();
+ return keys.map(k => String(v[k] ?? '')).join('|');
+}
+
+// Very small, fast hash for strings (no crypto dependency).
+function hashString(s: string): string {
+ let h = 0;
+ for (let i = 0; i < s.length; i += 1) {
+ // oxlint-disable-next-line unicorn/prefer-math-trunc -- | 0 is
intentional for 32-bit integer wrapping in hash
+ h = (h * 31 + s.charCodeAt(i)) | 0;
+ }
+ return String(h);
+}
+
+function signatureOfRows<D extends object>(rs: Row<D>[]): string {
+ const keys = rs.map(stableRowKey);
+ const len = keys.length;
+ const first = keys[0] ?? '';
+ const last = keys[len - 1] ?? '';
+ const digest = hashString(keys.join('\u0001')); // non-printable separator
to avoid collisions
+ return `${len}|${first}|${last}|${digest}`;
+}
+
// Be sure to pass our updateMyData and the skipReset option
export default typedMemo(function DataTable<D extends object>({
tableClassName,
@@ -289,6 +325,46 @@ export default typedMemo(function DataTable<D extends
object>({
onFilteredDataChange(rowsRef.current, searchText);
}, [filterValue, onFilteredDataChange, rowSignature]);
+ // Emit filtered rows to parent in client-side mode (debounced via RAF)
+ const isMountedRef = useRef(true);
+ useEffect(() => {
+ isMountedRef.current = true;
+ return () => {
+ isMountedRef.current = false;
+ };
+ }, []);
+
+ const rafRef = useRef<number | null>(null);
+ const lastSigRef = useRef<string>('');
+
+ useEffect(() => {
+ if (serverPagination || typeof onFilteredRowsChange !== 'function') {
+ return;
+ }
+
+ const sig = signatureOfRows(rows);
+
+ if (sig !== lastSigRef.current) {
+ lastSigRef.current = sig;
+ if (rafRef.current != null) {
+ cancelAnimationFrame(rafRef.current);
+ }
+ rafRef.current = requestAnimationFrame(() => {
+ if (isMountedRef.current) {
+ // Only emit originals when the signature truly changed
+ onFilteredRowsChange(rows.map(r => r.original as D));
+ }
+ });
+ }
+
+ return () => {
+ if (rafRef.current != null) {
+ cancelAnimationFrame(rafRef.current);
+ rafRef.current = null;
+ }
+ };
+ }, [rows, serverPagination, onFilteredRowsChange]);
+
const handleSearchChange = useCallback(
(query: string) => {
if (manualSearch && onSearchChange) {
@@ -472,84 +548,6 @@ export default typedMemo(function DataTable<D extends
object>({
onServerPaginationChange(pageNumber, serverPageSize);
}
- // Emit filtered rows to parent in client-side mode (debounced via RAF)
- const isMountedRef = useRef(true);
- useEffect(() => {
- isMountedRef.current = true;
- return () => {
- isMountedRef.current = false;
- };
- }, []);
-
- const rafRef = useRef<number | null>(null);
- const lastSigRef = useRef<string>('');
-
- // Prefer a stable identifier from original row data; otherwise use a
deterministic
- // concatenation of visible values (keys sorted so column order changes are
detected).
- function stableRowKey<D extends object>(r: Row<D>): string {
- const orig = r.original as Record<string, unknown> | undefined;
- if (orig) {
- const idLike =
- (orig as any).id ??
- (orig as any).ID ??
- (orig as any).key ??
- (orig as any).uuid;
- if (idLike != null) return String(idLike);
- }
-
- // Fallback: derive from row.values, but make it stable against column
order changes.
- const v = r.values as Record<string, unknown>;
- const keys = Object.keys(v).sort(); // detect column order changes
- return keys.map(k => String(v[k] ?? '')).join('|');
- }
-
- // Very small, fast hash for strings (no crypto dependency).
- function hashString(s: string): string {
- let h = 0;
- for (let i = 0; i < s.length; i += 1) {
- // oxlint-disable-next-line unicorn/prefer-math-trunc -- | 0 is
intentional for 32-bit integer wrapping in hash
- h = (h * 31 + s.charCodeAt(i)) | 0;
- }
- return String(h);
- }
-
- function signatureOfRows<D extends object>(rs: Row<D>[]): string {
- const keys = rs.map(stableRowKey);
- const len = keys.length;
- const first = keys[0] ?? '';
- const last = keys[len - 1] ?? '';
- const digest = hashString(keys.join('\u0001')); // non-printable separator
to avoid collisions
- return `${len}|${first}|${last}|${digest}`;
- }
-
- useEffect(() => {
- if (serverPagination || typeof onFilteredRowsChange !== 'function') {
- return;
- }
-
- const sig = signatureOfRows(rows);
-
- if (sig !== lastSigRef.current) {
- lastSigRef.current = sig;
- if (rafRef.current != null) {
- cancelAnimationFrame(rafRef.current);
- }
- rafRef.current = requestAnimationFrame(() => {
- if (isMountedRef.current) {
- // Only emit originals when the signature truly changed
- onFilteredRowsChange(rows.map(r => r.original as D));
- }
- });
- }
-
- return () => {
- if (rafRef.current != null) {
- cancelAnimationFrame(rafRef.current);
- rafRef.current = null;
- }
- };
- }, [rows, serverPagination, onFilteredRowsChange]);
-
return (
<div
ref={wrapperRef}
diff --git
a/superset-frontend/plugins/plugin-chart-table/test/DataTable/DataTable.test.tsx
b/superset-frontend/plugins/plugin-chart-table/test/DataTable/DataTable.test.tsx
new file mode 100644
index 00000000000..9932d97640d
--- /dev/null
+++
b/superset-frontend/plugins/plugin-chart-table/test/DataTable/DataTable.test.tsx
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import '@testing-library/jest-dom';
+import { Component, type ReactNode } from 'react';
+import { render, screen } from '@superset-ui/core/spec';
+import { CellProps, Column, HeaderProps } from 'react-table';
+import DataTable from '../../src/DataTable/DataTable';
+import { ProviderWrapper } from '../testHelpers';
+
+type DataRow = {
+ city: string;
+ firstName: string;
+};
+
+interface RenderErrorBoundaryProps {
+ children: ReactNode;
+}
+
+interface RenderErrorBoundaryState {
+ hasError: boolean;
+}
+
+const columns: Column<DataRow>[] = [
+ {
+ Header: ({ column }: HeaderProps<DataRow>) => (
+ <th data-column-name={column.id}>First name</th>
+ ),
+ Cell: ({ value }: CellProps<DataRow>) => <td>{value}</td>,
+ id: 'firstName',
+ accessor: 'firstName' as never,
+ },
+ {
+ Header: ({ column }: HeaderProps<DataRow>) => (
+ <th data-column-name={column.id}>City</th>
+ ),
+ Cell: ({ value }: CellProps<DataRow>) => <td>{value}</td>,
+ id: 'city',
+ accessor: 'city' as never,
+ },
+];
+
+const data: DataRow[] = [
+ { firstName: 'Michael', city: 'Paris' },
+ { firstName: 'Jordan', city: 'London' },
+];
+
+// Turns a render-phase throw, such as a Rules of Hooks violation, into a
+// readable assertion instead of an unhandled error.
+class RenderErrorBoundary extends Component<
+ RenderErrorBoundaryProps,
+ RenderErrorBoundaryState
+> {
+ state: RenderErrorBoundaryState = {
+ hasError: false,
+ };
+
+ static getDerivedStateFromError() {
+ return { hasError: true };
+ }
+
+ render() {
+ if (this.state.hasError) {
+ // data-test is the configured testIdAttribute (see
spec/helpers/setup.ts),
+ // so *ByTestId('render-error') resolves this node.
+ return <div data-test="render-error">Render error</div>;
+ }
+
+ return this.props.children;
+ }
+}
+
+const renderDataTable = (tableColumns: Column<DataRow>[]) => (
+ <ProviderWrapper>
+ <RenderErrorBoundary>
+ <DataTable<DataRow>
+ columns={tableColumns}
+ data={data}
+ rowCount={data.length}
+ serverPagination={false}
+ serverPaginationData={{}}
+ onServerPaginationChange={jest.fn()}
+ handleSortByChange={jest.fn()}
+ sortByFromParent={[]}
+ onSearchColChange={jest.fn()}
+ searchOptions={[]}
+ onFilteredRowsChange={jest.fn()}
+ sticky={false}
+ />
+ </RenderErrorBoundary>
+ </ProviderWrapper>
+);
+
+test('keeps the hook order stable when the columns disappear', () => {
+ const { rerender } = render(renderDataTable(columns));
+
+ expect(screen.getByText('Michael')).toBeInTheDocument();
+
+ rerender(renderDataTable([]));
+
+ expect(screen.queryByTestId('render-error')).not.toBeInTheDocument();
+ expect(screen.getByText('No data found')).toBeInTheDocument();
+});
+
+test('keeps the hook order stable when the columns appear', () => {
+ const { rerender } = render(renderDataTable([]));
+
+ expect(screen.getByText('No data found')).toBeInTheDocument();
+
+ rerender(renderDataTable(columns));
+
+ expect(screen.queryByTestId('render-error')).not.toBeInTheDocument();
+ expect(screen.getByText('Michael')).toBeInTheDocument();
+});
