Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -22,30 +22,47 @@ import { FeatureFlagExtensionHookResolver } from './FeatureFlagExtensionHookReso

/**
* React hook that returns a stable {@link SetFeatureFlag} callback.
*
* 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.
*/
const useFeatureFlagController = () => {
export const useFeatureFlagController = () => {
const dispatch = useConsoleDispatch();
const flags = useConsoleSelector(({ FLAGS }) => FLAGS);

// Queue of flag updates to be dispatched after render
const pendingUpdatesRef = useRef<Map<string, boolean>>(new Map());
const flushScheduledRef = useRef(false);

// 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(() => {
pendingUpdatesRef.current.forEach((enabled, flag) => {
if (flags.get(flag) !== enabled) {
dispatch(setFlag(flag, enabled));
}
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;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of flushScheduledRef is as a lock to prevent multiple queued microtasks of flushPendingUpdates from being called in parallel right?

Then shouldn't the unlock, flushPendingUpdates.current=false, be placed after pendingUpdatesRef is accessed, instead of before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes — unlock after taking the batch. Order is now: detach pendingUpdatesRef → set flushScheduledRef.current = false → dispatch the detached updates. That way a reentrant scheduleFlush during dispatch can queue a follow-up microtask for the new map.

updates.forEach((enabled, flag) => {
dispatch(setFlag(flag, enabled));
});
pendingUpdatesRef.current.clear();
});
}, [dispatch]);

return useCallback<SetFeatureFlag>((flag, enabled) => {
// Queue the update to be processed after render
pendingUpdatesRef.current.set(flag, enabled);
}, []);
const scheduleFlush = useCallback(() => {
if (flushScheduledRef.current) {
return;
}
flushScheduledRef.current = true;
queueMicrotask(() => {
flushPendingUpdates();
});
}, [flushPendingUpdates]);

return useCallback<SetFeatureFlag>(
(flag, enabled) => {
pendingUpdatesRef.current.set(flag, enabled);
scheduleFlush();
},
[scheduleFlush],
);
};

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
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';

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<RootState>();
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));

await act(async () => {
await Promise.resolve();
});

expect(store.getState().FLAGS.get('SYNC_FLAG')).toBe(true);
});

it('applies async flag updates without waiting for another render', async () => {
const { store, result } = renderController();

await act(async () => {
result.current('ASYNC_FLAG', true);
await Promise.resolve();
});

expect(store.getState().FLAGS.get('ASYNC_FLAG')).toBe(true);
});

it('coalesces consecutive async updates to the latest value', async () => {
const { store, result } = renderController();

await act(async () => {
result.current('TOGGLE_FLAG', true);
result.current('TOGGLE_FLAG', false);
await Promise.resolve();
});

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