From 4cc32a8c066948e5da2e69dd05383d74c545c4a4 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Tue, 11 Aug 2026 18:17:59 +0530 Subject: [PATCH 1/6] Flush async feature flag updates immediately Deferring all setFeatureFlag calls until the next render left flag-gated plugin routes and nav items missing for ~10s after an async console.flag/hookProvider resolved. Keep render-time updates deferred, but dispatch async updates right away. Fixes https://github.com/openshift/console/issues/16922 Co-authored-by: Cursor --- .../flags/FeatureFlagExtensionLoader.tsx | 39 +++++++--- .../FeatureFlagExtensionLoader.spec.tsx | 75 +++++++++++++++++++ 2 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx diff --git a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx index b3d001f8d4c..3fb59497c73 100644 --- a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx +++ b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx @@ -22,30 +22,49 @@ import { FeatureFlagExtensionHookResolver } from './FeatureFlagExtensionHookReso /** * React hook that returns a stable {@link SetFeatureFlag} callback. + * + * Sync calls during render are deferred until after layout effects so we avoid + * "Cannot update a component while rendering" with react-redux 8.x (handlers may + * invoke this while rendering). Async calls (e.g. after a fetch in a + * console.flag/hookProvider) flush immediately so flag-gated extensions update + * without waiting for an unrelated re-render. */ -const useFeatureFlagController = () => { +export const useFeatureFlagController = () => { const dispatch = useConsoleDispatch(); const flags = useConsoleSelector(({ FLAGS }) => FLAGS); + const flagsRef = useRef(flags); + flagsRef.current = flags; // Queue of flag updates to be dispatched after render const pendingUpdatesRef = useRef>(new Map()); + const isRenderingRef = useRef(true); - // Process pending flag updates after render completes. - // This avoids "Cannot update a component while rendering" errors with react-redux 8.x - // because handlers are called during render (they use hooks) but dispatches happen after. - useLayoutEffect(() => { + // Mark the render phase; cleared in the layout effect below. + isRenderingRef.current = true; + + const flushPendingUpdates = useCallback(() => { pendingUpdatesRef.current.forEach((enabled, flag) => { - if (flags.get(flag) !== enabled) { + if (flagsRef.current.get(flag) !== enabled) { dispatch(setFlag(flag, enabled)); } }); pendingUpdatesRef.current.clear(); + }, [dispatch]); + + useLayoutEffect(() => { + isRenderingRef.current = false; + flushPendingUpdates(); }); - return useCallback((flag, enabled) => { - // Queue the update to be processed after render - pendingUpdatesRef.current.set(flag, enabled); - }, []); + return useCallback( + (flag, enabled) => { + pendingUpdatesRef.current.set(flag, enabled); + if (!isRenderingRef.current) { + flushPendingUpdates(); + } + }, + [flushPendingUpdates], + ); }; /** diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx new file mode 100644 index 00000000000..f165393a1b0 --- /dev/null +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -0,0 +1,75 @@ +import { act, renderHook } from '@testing-library/react'; +import { Map as ImmutableMap } from 'immutable'; +import { setFlag } from '@console/internal/actions/flags'; +import { useConsoleDispatch } from '@console/shared/src/hooks/useConsoleDispatch'; +import { useConsoleSelector } from '@console/shared/src/hooks/useConsoleSelector'; +import { useFeatureFlagController } from '../FeatureFlagExtensionLoader'; + +jest.mock('@console/shared/src/hooks/useConsoleSelector', () => ({ + useConsoleSelector: jest.fn(), +})); + +jest.mock('@console/shared/src/hooks/useConsoleDispatch', () => ({ + useConsoleDispatch: jest.fn(), +})); + +jest.mock('@console/internal/actions/flags', () => ({ + ...jest.requireActual('@console/internal/actions/flags'), + setFlag: jest.fn((flag: string, value: boolean) => ({ + type: 'setFlag', + payload: { flag, value }, + })), +})); + +const mockDispatch = jest.fn(); +const mockUseSelector = useConsoleSelector as jest.Mock; +const mockUseDispatch = useConsoleDispatch as jest.Mock; +const mockSetFlag = setFlag as jest.MockedFunction; + +describe('useFeatureFlagController', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseDispatch.mockReturnValue(mockDispatch); + mockUseSelector.mockReturnValue(ImmutableMap()); + }); + + it('defers flag updates made during render until after layout effects', () => { + const { result } = renderHook(() => { + const setFeatureFlag = useFeatureFlagController(); + // Simulate console.flag/hookProvider handlers that set flags during render. + setFeatureFlag('SYNC_FLAG', true); + return setFeatureFlag; + }); + + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockSetFlag).toHaveBeenCalledWith('SYNC_FLAG', true); + expect(result.current).toEqual(expect.any(Function)); + }); + + it('dispatches async flag updates immediately without waiting for another render', () => { + const { result } = renderHook(() => useFeatureFlagController()); + + mockDispatch.mockClear(); + mockSetFlag.mockClear(); + + act(() => { + result.current('ASYNC_FLAG', true); + }); + + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockSetFlag).toHaveBeenCalledWith('ASYNC_FLAG', true); + }); + + it('does not redispatch when the flag already has the requested value', () => { + mockUseSelector.mockReturnValue(ImmutableMap({ EXISTING_FLAG: true })); + const { result } = renderHook(() => useFeatureFlagController()); + + mockDispatch.mockClear(); + + act(() => { + result.current('EXISTING_FLAG', true); + }); + + expect(mockDispatch).not.toHaveBeenCalled(); + }); +}); From 086170d836a33965d5cefdbe00118401e5b8ddf4 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Mon, 17 Aug 2026 16:36:59 +0530 Subject: [PATCH 2/6] Keep feature flag snapshot in sync across consecutive updates Update flagsRef when dispatching so consecutive async setFeatureFlag calls (e.g. true then false before Redux re-renders) are not skipped against a stale selector snapshot. Strengthen unit coverage. Co-authored-by: Cursor --- .../flags/FeatureFlagExtensionLoader.tsx | 3 +++ .../FeatureFlagExtensionLoader.spec.tsx | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx index 3fb59497c73..d804e6e3c9d 100644 --- a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx +++ b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx @@ -46,6 +46,9 @@ export const useFeatureFlagController = () => { pendingUpdatesRef.current.forEach((enabled, flag) => { if (flagsRef.current.get(flag) !== enabled) { dispatch(setFlag(flag, enabled)); + // Keep the local snapshot in sync so consecutive async updates (e.g. true + // then false before Redux re-renders) are not skipped against a stale value. + flagsRef.current = flagsRef.current.set(flag, enabled); } }); pendingUpdatesRef.current.clear(); diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx index f165393a1b0..2aae8079a86 100644 --- a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -34,13 +34,16 @@ describe('useFeatureFlagController', () => { }); it('defers flag updates made during render until after layout effects', () => { + let dispatchCountDuringRender = 0; const { result } = renderHook(() => { const setFeatureFlag = useFeatureFlagController(); // Simulate console.flag/hookProvider handlers that set flags during render. setFeatureFlag('SYNC_FLAG', true); + dispatchCountDuringRender = mockDispatch.mock.calls.length; return setFeatureFlag; }); + expect(dispatchCountDuringRender).toBe(0); expect(mockDispatch).toHaveBeenCalledTimes(1); expect(mockSetFlag).toHaveBeenCalledWith('SYNC_FLAG', true); expect(result.current).toEqual(expect.any(Function)); @@ -60,6 +63,23 @@ describe('useFeatureFlagController', () => { expect(mockSetFlag).toHaveBeenCalledWith('ASYNC_FLAG', true); }); + it('dispatches consecutive async updates before the selector re-renders', () => { + mockUseSelector.mockReturnValue(ImmutableMap({ TOGGLE_FLAG: false })); + const { result } = renderHook(() => useFeatureFlagController()); + + mockDispatch.mockClear(); + mockSetFlag.mockClear(); + + act(() => { + result.current('TOGGLE_FLAG', true); + result.current('TOGGLE_FLAG', false); + }); + + expect(mockDispatch).toHaveBeenCalledTimes(2); + expect(mockSetFlag).toHaveBeenNthCalledWith(1, 'TOGGLE_FLAG', true); + expect(mockSetFlag).toHaveBeenNthCalledWith(2, 'TOGGLE_FLAG', false); + }); + it('does not redispatch when the flag already has the requested value', () => { mockUseSelector.mockReturnValue(ImmutableMap({ EXISTING_FLAG: true })); const { result } = renderHook(() => useFeatureFlagController()); From 5d1c45a5916e50bb32f3c73123f687f5d309af24 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Tue, 18 Aug 2026 08:09:18 +0530 Subject: [PATCH 3/6] Avoid shadowing FLAGS with a local flagsRef Always dispatch pending setFeatureFlag updates instead of syncing a local FLAGS snapshot. Flags are also set elsewhere and read via useFlag, so a locally mutated copy can go stale. Immutable Map.set is already a no-op when the value is unchanged. --- .../flags/FeatureFlagExtensionLoader.tsx | 14 +++++-------- .../FeatureFlagExtensionLoader.spec.tsx | 20 +------------------ 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx index d804e6e3c9d..26f8828eb86 100644 --- a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx +++ b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx @@ -31,9 +31,6 @@ import { FeatureFlagExtensionHookResolver } from './FeatureFlagExtensionHookReso */ export const useFeatureFlagController = () => { const dispatch = useConsoleDispatch(); - const flags = useConsoleSelector(({ FLAGS }) => FLAGS); - const flagsRef = useRef(flags); - flagsRef.current = flags; // Queue of flag updates to be dispatched after render const pendingUpdatesRef = useRef>(new Map()); @@ -42,14 +39,13 @@ export const useFeatureFlagController = () => { // Mark the render phase; cleared in the layout effect below. isRenderingRef.current = true; + // Always dispatch pending values. Do not keep a local FLAGS snapshot for + // change-detection: flags are also updated elsewhere (e.g. detectFeatures / + // setFlag consumers read via useFlag), so a shadow copy can go stale. + // Immutable Map.set is a no-op when the value is unchanged. const flushPendingUpdates = useCallback(() => { pendingUpdatesRef.current.forEach((enabled, flag) => { - if (flagsRef.current.get(flag) !== enabled) { - dispatch(setFlag(flag, enabled)); - // Keep the local snapshot in sync so consecutive async updates (e.g. true - // then false before Redux re-renders) are not skipped against a stale value. - flagsRef.current = flagsRef.current.set(flag, enabled); - } + dispatch(setFlag(flag, enabled)); }); pendingUpdatesRef.current.clear(); }, [dispatch]); diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx index 2aae8079a86..d0b5e44d29a 100644 --- a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -1,8 +1,6 @@ import { act, renderHook } from '@testing-library/react'; -import { Map as ImmutableMap } from 'immutable'; import { setFlag } from '@console/internal/actions/flags'; import { useConsoleDispatch } from '@console/shared/src/hooks/useConsoleDispatch'; -import { useConsoleSelector } from '@console/shared/src/hooks/useConsoleSelector'; import { useFeatureFlagController } from '../FeatureFlagExtensionLoader'; jest.mock('@console/shared/src/hooks/useConsoleSelector', () => ({ @@ -22,7 +20,6 @@ jest.mock('@console/internal/actions/flags', () => ({ })); const mockDispatch = jest.fn(); -const mockUseSelector = useConsoleSelector as jest.Mock; const mockUseDispatch = useConsoleDispatch as jest.Mock; const mockSetFlag = setFlag as jest.MockedFunction; @@ -30,7 +27,6 @@ describe('useFeatureFlagController', () => { beforeEach(() => { jest.clearAllMocks(); mockUseDispatch.mockReturnValue(mockDispatch); - mockUseSelector.mockReturnValue(ImmutableMap()); }); it('defers flag updates made during render until after layout effects', () => { @@ -63,8 +59,7 @@ describe('useFeatureFlagController', () => { expect(mockSetFlag).toHaveBeenCalledWith('ASYNC_FLAG', true); }); - it('dispatches consecutive async updates before the selector re-renders', () => { - mockUseSelector.mockReturnValue(ImmutableMap({ TOGGLE_FLAG: false })); + it('dispatches consecutive async updates before Redux re-renders', () => { const { result } = renderHook(() => useFeatureFlagController()); mockDispatch.mockClear(); @@ -79,17 +74,4 @@ describe('useFeatureFlagController', () => { expect(mockSetFlag).toHaveBeenNthCalledWith(1, 'TOGGLE_FLAG', true); expect(mockSetFlag).toHaveBeenNthCalledWith(2, 'TOGGLE_FLAG', false); }); - - it('does not redispatch when the flag already has the requested value', () => { - mockUseSelector.mockReturnValue(ImmutableMap({ EXISTING_FLAG: true })); - const { result } = renderHook(() => useFeatureFlagController()); - - mockDispatch.mockClear(); - - act(() => { - result.current('EXISTING_FLAG', true); - }); - - expect(mockDispatch).not.toHaveBeenCalled(); - }); }); From 096d00179a8dd6c22232fdf6d0cb92c887313ca1 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Wed, 19 Aug 2026 21:49:04 +0530 Subject: [PATCH 4/6] Defer feature flag flushes via microtask Replace render-phase isRenderingRef tracking with a coalesced queueMicrotask flush so child FeatureFlagExtensionHookResolver re-renders cannot dispatch during render. Drop the stale flagsRef comment and exercise the controller through renderHookWithProviders against a real Redux store. --- .../flags/FeatureFlagExtensionLoader.tsx | 41 ++++---- .../FeatureFlagExtensionLoader.spec.tsx | 94 +++++++++---------- 2 files changed, 61 insertions(+), 74 deletions(-) diff --git a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx index 26f8828eb86..2a1fe5493b8 100644 --- a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx +++ b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx @@ -1,5 +1,5 @@ import type { FC } from 'react'; -import { useCallback, useRef, useEffect, useLayoutEffect } from 'react'; +import { useCallback, useRef, useEffect } from 'react'; import type { FeatureFlagHookProvider, ModelFeatureFlag, @@ -23,46 +23,41 @@ import { FeatureFlagExtensionHookResolver } from './FeatureFlagExtensionHookReso /** * React hook that returns a stable {@link SetFeatureFlag} callback. * - * Sync calls during render are deferred until after layout effects so we avoid - * "Cannot update a component while rendering" with react-redux 8.x (handlers may - * invoke this while rendering). Async calls (e.g. after a fetch in a - * console.flag/hookProvider) flush immediately so flag-gated extensions update - * without waiting for an unrelated re-render. + * Updates are always flushed on a microtask so handlers invoked during render + * (including child FeatureFlagExtensionHookResolver re-renders) never dispatch + * synchronously, while async callers (e.g. after a fetch in a + * console.flag/hookProvider) still update without waiting for an unrelated re-render. */ export const useFeatureFlagController = () => { const dispatch = useConsoleDispatch(); - // Queue of flag updates to be dispatched after render const pendingUpdatesRef = useRef>(new Map()); - const isRenderingRef = useRef(true); + const flushScheduledRef = useRef(false); - // Mark the render phase; cleared in the layout effect below. - isRenderingRef.current = true; - - // Always dispatch pending values. Do not keep a local FLAGS snapshot for - // change-detection: flags are also updated elsewhere (e.g. detectFeatures / - // setFlag consumers read via useFlag), so a shadow copy can go stale. - // Immutable Map.set is a no-op when the value is unchanged. const flushPendingUpdates = useCallback(() => { + flushScheduledRef.current = false; pendingUpdatesRef.current.forEach((enabled, flag) => { dispatch(setFlag(flag, enabled)); }); pendingUpdatesRef.current.clear(); }, [dispatch]); - useLayoutEffect(() => { - isRenderingRef.current = false; - flushPendingUpdates(); - }); + const scheduleFlush = useCallback(() => { + if (flushScheduledRef.current) { + return; + } + flushScheduledRef.current = true; + queueMicrotask(() => { + flushPendingUpdates(); + }); + }, [flushPendingUpdates]); return useCallback( (flag, enabled) => { pendingUpdatesRef.current.set(flag, enabled); - if (!isRenderingRef.current) { - flushPendingUpdates(); - } + scheduleFlush(); }, - [flushPendingUpdates], + [scheduleFlush], ); }; diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx index d0b5e44d29a..5d3ed60b52b 100644 --- a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -1,77 +1,69 @@ -import { act, renderHook } from '@testing-library/react'; -import { setFlag } from '@console/internal/actions/flags'; -import { useConsoleDispatch } from '@console/shared/src/hooks/useConsoleDispatch'; +import { act } from '@testing-library/react'; +import { useStore } from 'react-redux'; +import type { RootState } from '@console/internal/redux'; +import { renderHookWithProviders } from '@console/shared/src/test-utils/unit-test-utils'; import { useFeatureFlagController } from '../FeatureFlagExtensionLoader'; -jest.mock('@console/shared/src/hooks/useConsoleSelector', () => ({ - useConsoleSelector: jest.fn(), -})); - -jest.mock('@console/shared/src/hooks/useConsoleDispatch', () => ({ - useConsoleDispatch: jest.fn(), -})); - -jest.mock('@console/internal/actions/flags', () => ({ - ...jest.requireActual('@console/internal/actions/flags'), - setFlag: jest.fn((flag: string, value: boolean) => ({ - type: 'setFlag', - payload: { flag, value }, - })), -})); - -const mockDispatch = jest.fn(); -const mockUseDispatch = useConsoleDispatch as jest.Mock; -const mockSetFlag = setFlag as jest.MockedFunction; +jest.mock('@console/internal/plugins', () => { + // Avoid loading real local plugins / schema validation in unit tests. + const { TestPluginStore } = jest.requireActual('@openshift/dynamic-plugin-sdk'); + return { + pluginStore: new TestPluginStore({ + autoEnableLoadedPlugins: true, + loader: { + loadPluginManifest: async () => { + throw new Error('unused'); + }, + transformPluginManifest: (manifest) => manifest, + loadPlugin: async () => ({ success: true as const, loadedExtensions: [] }), + }, + }), + featureFlagMiddleware: () => (next) => (action) => next(action), + }; +}); describe('useFeatureFlagController', () => { - beforeEach(() => { - jest.clearAllMocks(); - mockUseDispatch.mockReturnValue(mockDispatch); - }); - - it('defers flag updates made during render until after layout effects', () => { - let dispatchCountDuringRender = 0; - const { result } = renderHook(() => { + it('defers flag updates made during render until after the render completes', async () => { + let flagDuringRender: boolean | undefined; + const { store, result } = renderHookWithProviders(() => { + const reduxStore = useStore(); const setFeatureFlag = useFeatureFlagController(); // Simulate console.flag/hookProvider handlers that set flags during render. setFeatureFlag('SYNC_FLAG', true); - dispatchCountDuringRender = mockDispatch.mock.calls.length; + flagDuringRender = reduxStore.getState().FLAGS.get('SYNC_FLAG'); return setFeatureFlag; }); - expect(dispatchCountDuringRender).toBe(0); - expect(mockDispatch).toHaveBeenCalledTimes(1); - expect(mockSetFlag).toHaveBeenCalledWith('SYNC_FLAG', true); + expect(flagDuringRender).toBeUndefined(); expect(result.current).toEqual(expect.any(Function)); - }); - it('dispatches async flag updates immediately without waiting for another render', () => { - const { result } = renderHook(() => useFeatureFlagController()); + await act(async () => { + await Promise.resolve(); + }); - mockDispatch.mockClear(); - mockSetFlag.mockClear(); + expect(store.getState().FLAGS.get('SYNC_FLAG')).toBe(true); + }); - act(() => { + it('applies async flag updates without waiting for another render', async () => { + const { store, result } = renderHookWithProviders(() => useFeatureFlagController()); + + await act(async () => { result.current('ASYNC_FLAG', true); + await Promise.resolve(); }); - expect(mockDispatch).toHaveBeenCalledTimes(1); - expect(mockSetFlag).toHaveBeenCalledWith('ASYNC_FLAG', true); + expect(store.getState().FLAGS.get('ASYNC_FLAG')).toBe(true); }); - it('dispatches consecutive async updates before Redux re-renders', () => { - const { result } = renderHook(() => useFeatureFlagController()); - - mockDispatch.mockClear(); - mockSetFlag.mockClear(); + it('coalesces consecutive async updates to the latest value', async () => { + const { store, result } = renderHookWithProviders(() => useFeatureFlagController()); - act(() => { + await act(async () => { result.current('TOGGLE_FLAG', true); result.current('TOGGLE_FLAG', false); + await Promise.resolve(); }); - expect(mockDispatch).toHaveBeenCalledTimes(2); - expect(mockSetFlag).toHaveBeenNthCalledWith(1, 'TOGGLE_FLAG', true); - expect(mockSetFlag).toHaveBeenNthCalledWith(2, 'TOGGLE_FLAG', false); + expect(store.getState().FLAGS.get('TOGGLE_FLAG')).toBe(false); }); }); From 27d10d39548808ccbfa263ea938ac3706876a481 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Wed, 19 Aug 2026 22:00:52 +0530 Subject: [PATCH 5/6] Detach pending flag map before flush for safe reentrancy Unlock after taking the batch so reentrant setFeatureFlag during dispatch can schedule a follow-up microtask. Use createTestPluginStore in tests and cover the reentrant-update path. --- .../flags/FeatureFlagExtensionLoader.tsx | 8 ++- .../FeatureFlagExtensionLoader.spec.tsx | 65 ++++++++++++------- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx index 2a1fe5493b8..e24a08b7a44 100644 --- a/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx +++ b/frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx @@ -35,11 +35,15 @@ export const useFeatureFlagController = () => { const flushScheduledRef = useRef(false); const flushPendingUpdates = useCallback(() => { + // Detach the current batch first so reentrant setFeatureFlag calls during + // dispatch (e.g. Redux subscribers) write into a fresh map and can schedule a + // follow-up flush instead of being cleared with this batch. + const updates = pendingUpdatesRef.current; + pendingUpdatesRef.current = new Map(); flushScheduledRef.current = false; - pendingUpdatesRef.current.forEach((enabled, flag) => { + updates.forEach((enabled, flag) => { dispatch(setFlag(flag, enabled)); }); - pendingUpdatesRef.current.clear(); }, [dispatch]); const scheduleFlush = useCallback(() => { diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx index 5d3ed60b52b..4e6d1741ae4 100644 --- a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -2,37 +2,40 @@ import { act } from '@testing-library/react'; import { useStore } from 'react-redux'; import type { RootState } from '@console/internal/redux'; import { renderHookWithProviders } from '@console/shared/src/test-utils/unit-test-utils'; +import { createTestPluginStore } from '../../console-operator/__tests__/pluginTestUtils'; import { useFeatureFlagController } from '../FeatureFlagExtensionLoader'; +// unit-test-utils / the FLAGS reducer import @console/internal/plugins at module load. +// Provide a TestPluginStore via the shared helper so that import succeeds in Jest. jest.mock('@console/internal/plugins', () => { - // Avoid loading real local plugins / schema validation in unit tests. - const { TestPluginStore } = jest.requireActual('@openshift/dynamic-plugin-sdk'); + const { createTestPluginStore: createStore } = jest.requireActual( + '../../console-operator/__tests__/pluginTestUtils', + ); return { - pluginStore: new TestPluginStore({ - autoEnableLoadedPlugins: true, - loader: { - loadPluginManifest: async () => { - throw new Error('unused'); - }, - transformPluginManifest: (manifest) => manifest, - loadPlugin: async () => ({ success: true as const, loadedExtensions: [] }), - }, - }), + pluginStore: createStore(), featureFlagMiddleware: () => (next) => (action) => next(action), }; }); +const renderController = () => + renderHookWithProviders(() => useFeatureFlagController(), { + pluginStore: createTestPluginStore(), + }); + describe('useFeatureFlagController', () => { it('defers flag updates made during render until after the render completes', async () => { let flagDuringRender: boolean | undefined; - const { store, result } = renderHookWithProviders(() => { - const reduxStore = useStore(); - const setFeatureFlag = useFeatureFlagController(); - // Simulate console.flag/hookProvider handlers that set flags during render. - setFeatureFlag('SYNC_FLAG', true); - flagDuringRender = reduxStore.getState().FLAGS.get('SYNC_FLAG'); - return setFeatureFlag; - }); + const { store, result } = renderHookWithProviders( + () => { + const reduxStore = useStore(); + const setFeatureFlag = useFeatureFlagController(); + // Simulate console.flag/hookProvider handlers that set flags during render. + setFeatureFlag('SYNC_FLAG', true); + flagDuringRender = reduxStore.getState().FLAGS.get('SYNC_FLAG'); + return setFeatureFlag; + }, + { pluginStore: createTestPluginStore() }, + ); expect(flagDuringRender).toBeUndefined(); expect(result.current).toEqual(expect.any(Function)); @@ -45,7 +48,7 @@ describe('useFeatureFlagController', () => { }); it('applies async flag updates without waiting for another render', async () => { - const { store, result } = renderHookWithProviders(() => useFeatureFlagController()); + const { store, result } = renderController(); await act(async () => { result.current('ASYNC_FLAG', true); @@ -56,7 +59,7 @@ describe('useFeatureFlagController', () => { }); it('coalesces consecutive async updates to the latest value', async () => { - const { store, result } = renderHookWithProviders(() => useFeatureFlagController()); + const { store, result } = renderController(); await act(async () => { result.current('TOGGLE_FLAG', true); @@ -66,4 +69,22 @@ describe('useFeatureFlagController', () => { expect(store.getState().FLAGS.get('TOGGLE_FLAG')).toBe(false); }); + + it('preserves flag updates made reentrantly during flush', async () => { + const { store, result } = renderController(); + + await act(async () => { + const unsubscribe = store.subscribe(() => { + if (store.getState().FLAGS.get('REENTRANT_FLAG') === true) { + result.current('REENTRANT_FLAG', false); + unsubscribe(); + } + }); + result.current('REENTRANT_FLAG', true); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(store.getState().FLAGS.get('REENTRANT_FLAG')).toBe(false); + }); }); From 309946d0dde12425bafd6a87b3ef0f98c5a6e220 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Wed, 19 Aug 2026 22:11:46 +0530 Subject: [PATCH 6/6] Drop unnecessary @console/internal/plugins mock from flag tests createTestPluginStore passed into renderHookWithProviders is enough; sibling console-app tests rely on the same pattern without mocking plugins. --- .../__tests__/FeatureFlagExtensionLoader.spec.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx index 4e6d1741ae4..c5d60d4c379 100644 --- a/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx +++ b/frontend/packages/console-app/src/components/flags/__tests__/FeatureFlagExtensionLoader.spec.tsx @@ -5,18 +5,6 @@ import { renderHookWithProviders } from '@console/shared/src/test-utils/unit-tes import { createTestPluginStore } from '../../console-operator/__tests__/pluginTestUtils'; import { useFeatureFlagController } from '../FeatureFlagExtensionLoader'; -// unit-test-utils / the FLAGS reducer import @console/internal/plugins at module load. -// Provide a TestPluginStore via the shared helper so that import succeeds in Jest. -jest.mock('@console/internal/plugins', () => { - const { createTestPluginStore: createStore } = jest.requireActual( - '../../console-operator/__tests__/pluginTestUtils', - ); - return { - pluginStore: createStore(), - featureFlagMiddleware: () => (next) => (action) => next(action), - }; -}); - const renderController = () => renderHookWithProviders(() => useFeatureFlagController(), { pluginStore: createTestPluginStore(),