-
Notifications
You must be signed in to change notification settings - Fork 193
[JSX] Convert FilterableDataTable to Functional Component #10312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[JSX] Convert FilterableDataTable to Functional Component #10312
Conversation
| @@ -0,0 +1,4 @@ | |||
| declare module 'jsx/Filter'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having trouble making use of other jsx files that don't have typescript declarations (i.e. ProgressBar.js, Filter.js), and this was the solution I came upon.
I'm not sure why I included Datatable and Panel, they seem to have sidecar typescript declarations.
I can find another solution or alternatively convert those to .tsx in this PR as well.
| @@ -0,0 +1,66 @@ | |||
| import {ReactNode} from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea of having a different "types" in jsx.. is there an existing file that should own each of these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay! I was trying to follow the lead from the dataquery module where I noticed a types.js file
One of the reasons this seemed useful is because the Props for FilterableDatatable will essentially be a union of the Props for Datatable and Filter and a types.js file felt like a convenient central place to store them without having to import across files.
Do you suggest I just declare the types at the top of the respective components?
Description
This PR performs a migration for the
FilterableDataTablecomponent to class component.Key Changes
1. TypeScript Migration & Architecture
jsx/types.tsto centralize all table-related types.FilterableDataTablePropsas an intersection ofDataTableProps. This ensures the wrapper "englobes" all base table properties (likeactions,data, andgetFormattedCell) without duplication, ensuring synchronization between parent and child components.declarations.d.tsto provide module definitions for existing JavaScript components (Filter.js,ProgressBar.js, etc.), resolvingTS7016"Implicit Any" compiler errors.2. Logic & State Improvements
useEffect(300ms) to synchronize the filter state with the browser's URL query parameters viawindow.history.replaceState.validFiltersmemoization to ensure only fields marked for filtering are passed to the UI, preventing unnecessary re-renders.How to Test
?filters=...after the 300ms debounce.