rusackas commented on PR #34562:
URL: https://github.com/apache/superset/pull/34562#issuecomment-3857801391
### Response to CodeAnt AI Performance Suggestion
Thanks for the thoughtful review! I considered the suggested
`key={filterText ? 'filtered' : 'unfiltered'}` approach, but it wouldn't fully
solve the problem:
**Why the simpler toggle approach doesn't work:**
If you're on page 3 of 3 with filter "Action" and type more to get "ActionX"
which yields only 1 page, you'd still get the "currentPage > totalPages" error
because the key didn't change ('filtered' → 'filtered').
**Why `key={filterText}` is the right trade-off:**
1. **Correctness**: It prevents the error in all cases
2. **Simplicity**: Avoids adding controlled pagination to the shared
TableView component
3. **Acceptable performance**: Tables in View as Table are reasonably sized
(paginated results, not huge datasets)
4. **React-idiomatic**: This is a [documented React
pattern](https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes)
I've updated the inline comment to document this trade-off for future
maintainers.
--
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]
