rusackas commented on PR #34562: URL: https://github.com/apache/superset/pull/34562#issuecomment-3857248956
## PR Rebased and Improved Rebased onto latest master and addressed review comments. ### Review Comment Responses **1. Korbit AI - "Improper key usage for forcing re-renders"** The `key` prop approach is actually a [documented React pattern](https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes) for intentionally resetting component state. From the React docs: > You can reset a component's state by passing a different key to a component. When the key changes, React re-creates the DOM and resets state for this component. The TableView component uses `react-table` internally and manages pagination state internally. There's no exposed API to reset pagination from outside. The alternatives would be: - Modifying TableView (shared component in `@superset-ui/core`) to add a reset mechanism - Adding controlled pagination to SingleQueryResultPane The `key` prop is the simplest, most reliable fix for this bug. **2. Copilot - "Using 'as any' bypasses TypeScript's type safety"** Fixed. The test now uses proper typing with `Record<string, unknown>[]` and documents the type mismatch that exists in the codebase (the `SingleQueryResultPaneProp` type defines `data` as a 2D array, but `useFilteredTableData` expects a 1D array). ### Other Improvements - Flattened test structure to use `test()` instead of `describe()` (per project guidelines) - Added explanatory comment in the component - Improved test names for clarity -- 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]
