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(); + }); +});