fix(Tabs): prevent infinite rerenders with nested tabs#37018
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
🎪 Showtime deployed environment on GHA for 45bb64d • Environment: http://18.236.111.15:8080 (admin/admin) |
Code Review Agent Run #998926Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
🎪 Showtime deployed environment on GHA for 92e5cac • Environment: http://34.209.227.96:8080 (admin/admin) |
|
Bito Automatic Review Skipped – PR Already Merged |
(cherry picked from commit 0294c30)
(cherry picked from commit 0294c30)
- Add AutoRefreshContext mock hooks to Tab.test.tsx so tests can simulate isRefreshInFlight/isAutoRefreshing state independently. - Add three regression tests covering gaps in the existing infinite-loop fix (apache#37018): * dedup key resets correctly when a second dashboard refresh produces a new lastRefreshTime, allowing exactly one additional lazy refresh. * Tab does NOT dispatch a lazy refresh while a dashboard refresh is already in flight. * Tab does NOT dispatch a lazy refresh while an auto-refresh cycle is active. - Spread jest.requireActual for the dashboardState actions mock so the reducer keeps its action-type constants in tests. - Clear the CHART_MOUNT_DELAY setTimeout on unmount / dependency change in Tab.tsx to avoid stale dispatches.
Harden the lazy-refresh effect in Tab.tsx to prevent stale dispatches and stranded dedup keys when a chart lives inside nested tabs. 1. Return a cleanup from the useEffect that clears the CHART_MOUNT_DELAY setTimeout. A tab that unmounts (or re-renders with different deps) within the 100ms window no longer fires a stale lazy-refresh dispatch. 2. Move the handledRefreshRef dedup-key claim inside the setTimeout callback so the key is only claimed after the dispatch actually fires. If a guard (isAutoRefreshing / isRefreshInFlight) short- circuits the effect, the key is not stranded, and the subsequent effect run can still schedule the refresh. Adds four regression tests in Tab.test.tsx covering: - isRefreshInFlight and isAutoRefreshing short-circuit paths - setTimeout cleanup on unmount - Dedup key reschedule after a guarded short-circuit clears Related: apache#39439, apache#37018 Co-Authored-By: bot_apk <apk@cognition.ai>
SUMMARY
#35265 introduced a bug where refreshing a dashboard with nested tabs would cause infinite rerenders. This pr fixes that bug by introducing a lazy option to dashboard refresh action that doesn't update the global refresh time.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
tab-refresh-before.mp4
After:
tab-refresh-after.mp4
TESTING INSTRUCTIONS
This should not cause infinite rerenders.
ADDITIONAL INFORMATION