From d983c0dc40f18121b51d6ad50065b717e82cf083 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 9 Jan 2026 21:15:25 +0300 Subject: [PATCH 1/4] fix(Tabs): prevent infinite rerenders with nested tabs --- .../src/dashboard/actions/dashboardState.js | 12 ++++++++-- .../components/gridComponents/Tab/Tab.jsx | 22 ++++++++++++++----- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 18b567e20f51..afda5554b670 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -603,13 +603,21 @@ export function onRefresh( force = false, interval = 0, dashboardId, + isLazyLoad = false, ) { return dispatch => { - dispatch({ type: ON_REFRESH }); + // Only dispatch ON_REFRESH for dashboard-level refreshes + // Skip it for lazy-loaded tabs to prevent infinite loops + if (!isLazyLoad) { + dispatch({ type: ON_REFRESH }); + } + refreshCharts(chartList, force, interval, dashboardId, dispatch).then( () => { dispatch(onRefreshSuccess()); - dispatch(onFiltersRefresh()); + if (!isLazyLoad) { + dispatch(onFiltersRefresh()); + } }, ); }; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx index 8902e84a4675..a52e416a5631 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Fragment, useCallback, memo, useEffect } from 'react'; +import { Fragment, useCallback, memo, useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { useDispatch, useSelector } from 'react-redux'; @@ -129,6 +129,9 @@ const Tab = props => { state => state.dashboardState.tabActivationTimes?.[props.id] || 0, ); const dashboardInfo = useSelector(state => state.dashboardInfo); + + // Track which refresh we've already handled to prevent duplicates + const handledRefreshRef = useRef(null); useEffect(() => { if (props.renderType === RENDER_TAB_CONTENT && props.isComponentVisible) { @@ -137,13 +140,20 @@ const Tab = props => { tabActivationTime && lastRefreshTime > tabActivationTime ) { - const chartIds = getChartIdsFromComponent(props.id, dashboardLayout); - if (chartIds.length > 0) { - requestAnimationFrame(() => { + // Create a unique key for this specific refresh + const refreshKey = `${props.id}-${lastRefreshTime}`; + + // Only proceed if we haven't already handled this refresh + if (handledRefreshRef.current !== refreshKey) { + handledRefreshRef.current = refreshKey; + + const chartIds = getChartIdsFromComponent(props.id, dashboardLayout); + if (chartIds.length > 0) { + // Use lazy load flag to avoid updating global refresh time setTimeout(() => { - dispatch(onRefresh(chartIds, true, 0, dashboardInfo.id)); + dispatch(onRefresh(chartIds, true, 0, dashboardInfo.id, true)); }, CHART_MOUNT_DELAY); - }); + } } } } From 21e8934aa57897050460b0f5cb2a7dda9fc1d029 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 9 Jan 2026 21:16:54 +0300 Subject: [PATCH 2/4] chore: update test --- .../src/dashboard/components/gridComponents/Tab/Tab.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx index a5072293ada2..e4e529eadeb9 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx @@ -500,6 +500,7 @@ test('Should refresh charts when tab becomes active after dashboard refresh', as true, // Force refresh 0, // Interval 23, // Dashboard ID + true, // isLazyLoad flag ); }); From 45bb64d08e441a16bcbd3041c9fd36af29edbc30 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 9 Jan 2026 21:20:27 +0300 Subject: [PATCH 3/4] test: add regression tests --- .../gridComponents/Tab/Tab.test.tsx | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx index e4e529eadeb9..7f0392fc5f48 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx @@ -541,3 +541,105 @@ test('Should not refresh charts when tab becomes active if no dashboard refresh expect(onRefresh).not.toHaveBeenCalled(); }); + +test('Should not cause infinite refresh loop with nested tabs - regression test', async () => { + jest.clearAllMocks(); + const getChartIdsFromComponent = require('src/dashboard/util/getChartIdsFromComponent'); + getChartIdsFromComponent.mockReset(); + getChartIdsFromComponent.mockReturnValue([201, 202]); + + const props = createProps(); + props.renderType = 'RENDER_TAB_CONTENT'; + props.isComponentVisible = false; + + const initialState = { + dashboardState: { + lastRefreshTime: Date.now() - 1000, // Dashboard was refreshed recently + tabActivationTimes: { + 'TAB-YT6eNksV-': Date.now() - 5000, // Tab was activated before refresh + }, + }, + dashboardInfo: { + id: 23, + dash_edit_perm: true, + }, + }; + + const { rerender } = render(, { + useRedux: true, + useDnd: true, + initialState, + }); + + // Initial state - no refresh should happen + expect(onRefresh).not.toHaveBeenCalled(); + + // Make tab visible - should trigger ONE refresh + rerender(); + + await waitFor( + () => { + expect(onRefresh).toHaveBeenCalledTimes(1); + }, + { timeout: 500 }, + ); + + // Clear the mock to track subsequent calls + jest.clearAllMocks(); + + // REGRESSION TEST: Multiple re-renders should NOT trigger additional refreshes + // This simulates the infinite loop scenario that was happening with nested tabs + for (let i = 0; i < 5; i++) { + rerender(); + await new Promise(resolve => setTimeout(resolve, 20)); + } + + expect(onRefresh).not.toHaveBeenCalled(); +}); + +test('Should use isLazyLoad flag for tab refreshes', async () => { + jest.clearAllMocks(); + const getChartIdsFromComponent = require('src/dashboard/util/getChartIdsFromComponent'); + getChartIdsFromComponent.mockReset(); + getChartIdsFromComponent.mockReturnValue([401, 402]); + + const props = createProps(); + props.renderType = 'RENDER_TAB_CONTENT'; + props.isComponentVisible = true; + + const initialState = { + dashboardState: { + lastRefreshTime: Date.now() - 1000, // Dashboard was refreshed recently + tabActivationTimes: { + 'TAB-YT6eNksV-': Date.now() - 5000, // Tab was activated before refresh + }, + }, + dashboardInfo: { + id: 42, + dash_edit_perm: true, + }, + }; + + render(, { + useRedux: true, + useDnd: true, + initialState, + }); + + // Tab should trigger refresh with isLazyLoad = true + await waitFor( + () => { + expect(onRefresh).toHaveBeenCalled(); + }, + { timeout: 500 }, + ); + + // Verify that isLazyLoad flag is set to true for tab refreshes + expect(onRefresh).toHaveBeenCalledWith( + [401, 402], + true, // force + 0, // interval + 42, // dashboardId + true, // isLazyLoad should be true to prevent infinite loops + ); +}); From 92e5cac7d42e5517b53f1614160c1a53ffb069ac Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Sat, 10 Jan 2026 17:40:47 +0300 Subject: [PATCH 4/4] fix: ci --- superset-frontend/src/dashboard/actions/dashboardState.js | 2 +- .../src/dashboard/components/gridComponents/Tab/Tab.jsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index afda5554b670..e19567d8c780 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -611,7 +611,7 @@ export function onRefresh( if (!isLazyLoad) { dispatch({ type: ON_REFRESH }); } - + refreshCharts(chartList, force, interval, dashboardId, dispatch).then( () => { dispatch(onRefreshSuccess()); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx index a52e416a5631..b56066af05e6 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx @@ -129,7 +129,7 @@ const Tab = props => { state => state.dashboardState.tabActivationTimes?.[props.id] || 0, ); const dashboardInfo = useSelector(state => state.dashboardInfo); - + // Track which refresh we've already handled to prevent duplicates const handledRefreshRef = useRef(null); @@ -142,11 +142,11 @@ const Tab = props => { ) { // Create a unique key for this specific refresh const refreshKey = `${props.id}-${lastRefreshTime}`; - + // Only proceed if we haven't already handled this refresh if (handledRefreshRef.current !== refreshKey) { handledRefreshRef.current = refreshKey; - + const chartIds = getChartIdsFromComponent(props.id, dashboardLayout); if (chartIds.length > 0) { // Use lazy load flag to avoid updating global refresh time