Skip to content

Commit 6a4c42f

Browse files
authored
fix(react-sdk): double evaluation on client side init (#1229)
<!-- CURSOR_SUMMARY --> > [!NOTE] > **Medium Risk** > Changes how variation hooks evaluate flags by introducing readiness gating via a new `LDReactClient.isReady()` API, which could affect initial render values and evaluation timing across apps (especially around bootstrap/initialization transitions). Test coverage was expanded, but behavior changes are user-visible. > > **Overview** > Prevents variation hooks from evaluating flags before the client is ready, avoiding extra/double evaluations during initialization. `useVariationCore` now returns the default (or a caller-provided not-ready default) until `client.isReady()` becomes true, then evaluates once and continues to re-evaluate on context/key changes and `change:<flag>` events. > > Adds `LDReactClient.isReady()` (implemented in `createClient`, including bootstrap awareness, and in the server-side noop client) and updates variation *detail* hooks to return a `CLIENT_NOT_READY` evaluation reason when gated. Tests and mocks are updated accordingly, and the client-only example UI now hides flag output until initialization completes. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f401bc0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/launchdarkly/js-core/pull/1229" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
1 parent c67c5f6 commit 6a4c42f

12 files changed

Lines changed: 289 additions & 55 deletions

File tree

packages/sdk/react/__tests__/client/deprecated-hooks/renderHelpers.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function makeWrapper(
1010
) {
1111
const contextValue: LDReactClientContextValue = {
1212
client: mockClient,
13-
initializedState: 'initializing',
13+
initializedState: 'complete',
1414
...contextOverrides,
1515
};
1616

packages/sdk/react/__tests__/client/hooks/useVariation.test.tsx

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function makeWrapper(
2020
) {
2121
const contextValue: LDReactClientContextValue = {
2222
client: mockClient,
23-
initializedState: 'initializing',
23+
initializedState: 'complete',
2424
...contextOverrides,
2525
};
2626

@@ -563,3 +563,135 @@ it('useJsonVariation calls variation again when context changes', () => {
563563

564564
expect((mockClient.jsonVariation as jest.Mock).mock.calls.length).toBeGreaterThan(callsBefore);
565565
});
566+
567+
// ─── ready-gating tests ──────────────────────────────────────────────────────
568+
569+
it('does not call variation when not ready', () => {
570+
const mockClient = makeMockClient();
571+
(mockClient.isReady as jest.Mock).mockReturnValue(false);
572+
573+
function FlagConsumer() {
574+
useBoolVariation('my-flag', false);
575+
return null;
576+
}
577+
578+
const Wrapper = makeWrapper(mockClient, { initializedState: 'initializing' });
579+
render(
580+
<Wrapper>
581+
<FlagConsumer />
582+
</Wrapper>,
583+
);
584+
585+
expect(mockClient.boolVariation).not.toHaveBeenCalled();
586+
});
587+
588+
it('returns defaultValue when not ready', () => {
589+
const mockClient = makeMockClient();
590+
(mockClient.isReady as jest.Mock).mockReturnValue(false);
591+
(mockClient.boolVariation as jest.Mock).mockReturnValue(true);
592+
593+
const captured: boolean[] = [];
594+
595+
function FlagConsumer() {
596+
const value = useBoolVariation('my-flag', false);
597+
captured.push(value);
598+
return null;
599+
}
600+
601+
const Wrapper = makeWrapper(mockClient, { initializedState: 'initializing' });
602+
render(
603+
<Wrapper>
604+
<FlagConsumer />
605+
</Wrapper>,
606+
);
607+
608+
expect(captured[0]).toBe(false);
609+
});
610+
611+
it('evaluates once when initialization completes', () => {
612+
const mockClient = makeMockClient();
613+
(mockClient.isReady as jest.Mock).mockReturnValue(false);
614+
(mockClient.boolVariation as jest.Mock).mockReturnValue(true);
615+
616+
let setContextValue!: React.Dispatch<React.SetStateAction<LDReactClientContextValue>>;
617+
const captured: boolean[] = [];
618+
619+
function StatefulWrapper({ children }: { children: React.ReactNode }) {
620+
const [ctxValue, setCtx] = React.useState<LDReactClientContextValue>({
621+
client: mockClient,
622+
initializedState: 'initializing',
623+
});
624+
setContextValue = setCtx;
625+
return <LDReactContext.Provider value={ctxValue}>{children}</LDReactContext.Provider>;
626+
}
627+
628+
function FlagConsumer() {
629+
const value = useBoolVariation('my-flag', false);
630+
captured.push(value);
631+
return null;
632+
}
633+
634+
render(
635+
<StatefulWrapper>
636+
<FlagConsumer />
637+
</StatefulWrapper>,
638+
);
639+
640+
expect(mockClient.boolVariation).not.toHaveBeenCalled();
641+
expect(captured[captured.length - 1]).toBe(false);
642+
643+
(mockClient.isReady as jest.Mock).mockReturnValue(true);
644+
645+
act(() => {
646+
setContextValue({
647+
client: mockClient,
648+
initializedState: 'complete',
649+
context: { kind: 'user', key: 'user-1' },
650+
});
651+
});
652+
653+
expect(mockClient.boolVariation).toHaveBeenCalledTimes(1);
654+
expect(captured[captured.length - 1]).toBe(true);
655+
});
656+
657+
it('evaluates exactly once on mount when already initialized (bootstrap)', () => {
658+
const mockClient = makeMockClient();
659+
(mockClient.boolVariation as jest.Mock).mockReturnValue(true);
660+
661+
function FlagConsumer() {
662+
useBoolVariation('my-flag', false);
663+
return null;
664+
}
665+
666+
const Wrapper = makeWrapper(mockClient, { initializedState: 'complete' });
667+
render(
668+
<Wrapper>
669+
<FlagConsumer />
670+
</Wrapper>,
671+
);
672+
673+
expect(mockClient.boolVariation).toHaveBeenCalledTimes(1);
674+
});
675+
676+
it('evaluates when initialization failed (client returns defaults on failure)', () => {
677+
const mockClient = makeMockClient();
678+
(mockClient.boolVariation as jest.Mock).mockReturnValue(false);
679+
680+
const captured: boolean[] = [];
681+
682+
function FlagConsumer() {
683+
const value = useBoolVariation('my-flag', false);
684+
captured.push(value);
685+
return null;
686+
}
687+
688+
const Wrapper = makeWrapper(mockClient, { initializedState: 'failed' });
689+
render(
690+
<Wrapper>
691+
<FlagConsumer />
692+
</Wrapper>,
693+
);
694+
695+
expect(mockClient.boolVariation).toHaveBeenCalledTimes(1);
696+
expect(captured[0]).toBe(false);
697+
});

packages/sdk/react/__tests__/client/hooks/useVariationDetail.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { makeMockClient } from '../mockClient';
1919
function makeWrapper(mockClient: ReturnType<typeof makeMockClient>) {
2020
const contextValue: LDReactClientContextValue = {
2121
client: mockClient,
22-
initializedState: 'initializing',
22+
initializedState: 'complete',
2323
};
2424

2525
return function Wrapper({ children }: { children: React.ReactNode }) {

packages/sdk/react/__tests__/client/mockClient.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ export function makeMockClient(options: MockClientOptions = {}): MockClient {
8888
// @ts-ignore
8989
waitForInitialization: jest.fn(() => Promise.resolve({ status: 'complete' as const })),
9090
addHook: jest.fn(),
91+
isReady: jest.fn(() => true),
9192
shouldUseCamelCaseFlagKeys: jest.fn(() => true),
9293
} as unknown as LDReactClient;
9394

packages/sdk/react/__tests__/client/provider/LDReactProvider.test.tsx

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,66 @@ it('includes error in initial state when client was already failed before mount'
167167
expect(firstRender?.error).toBe(preFailedError);
168168
});
169169

170+
it('initializedState is complete when client is already initialized at mount (bootstrap)', () => {
171+
const client = makeMockClient({ initialState: 'complete' });
172+
const contextValues: LDReactClientContextValue[] = [];
173+
174+
function Consumer() {
175+
const value = useContext(LDReactContext);
176+
contextValues.push(value);
177+
return null;
178+
}
179+
180+
const Provider = createLDReactProviderWithClient(client);
181+
182+
render(
183+
<Provider>
184+
<Consumer />
185+
</Provider>,
186+
);
187+
188+
expect(contextValues[0]?.initializedState).toBe('complete');
189+
});
190+
191+
it('initializedState stays complete after context changes from identify', async () => {
192+
const client = makeMockClient();
193+
const contextValues: LDReactClientContextValue[] = [];
194+
195+
function Consumer() {
196+
const value = useContext(LDReactContext);
197+
contextValues.push(value);
198+
return null;
199+
}
200+
201+
const Provider = createLDReactProviderWithClient(client);
202+
203+
render(
204+
<Provider>
205+
<Consumer />
206+
</Provider>,
207+
);
208+
209+
await act(async () => {
210+
client.fireInitStatusChange('complete');
211+
});
212+
213+
await waitFor(() => {
214+
expect(contextValues[contextValues.length - 1]?.initializedState).toBe('complete');
215+
});
216+
217+
await act(async () => {
218+
client.fireContextChange({ kind: 'user', key: 'new-user' });
219+
});
220+
221+
await waitFor(() => {
222+
expect(contextValues[contextValues.length - 1]?.context).toEqual({
223+
kind: 'user',
224+
key: 'new-user',
225+
});
226+
});
227+
expect(contextValues[contextValues.length - 1]?.initializedState).toBe('complete');
228+
});
229+
170230
// ─── createLDReactProvider (convenience factory) ─────────────────────────────
171231

172232
describe('createLDReactProvider (convenience factory)', () => {

packages/sdk/react/__tests__/client/provider/multipleEnvironments.test.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ it('useBoolVariation reads from each respective client when using separate conte
1616
const ContextA = initLDReactContext();
1717
const ContextB = initLDReactContext();
1818

19-
const clientA = makeMockClient();
20-
const clientB = makeMockClient();
19+
const clientA = makeMockClient({ initialState: 'complete' });
20+
const clientB = makeMockClient({ initialState: 'complete' });
2121

2222
(clientA.boolVariation as jest.Mock).mockReturnValue(true);
2323
(clientB.boolVariation as jest.Mock).mockReturnValue(false);
@@ -52,8 +52,8 @@ it('useBoolVariation calls boolVariation on the correct client for each context'
5252
const ContextA = initLDReactContext();
5353
const ContextB = initLDReactContext();
5454

55-
const clientA = makeMockClient();
56-
const clientB = makeMockClient();
55+
const clientA = makeMockClient({ initialState: 'complete' });
56+
const clientB = makeMockClient({ initialState: 'complete' });
5757

5858
(clientA.boolVariation as jest.Mock).mockReturnValue(true);
5959
(clientB.boolVariation as jest.Mock).mockReturnValue(false);
@@ -88,8 +88,8 @@ it('useLDClient returns the respective client for each context', () => {
8888
const ContextA = initLDReactContext();
8989
const ContextB = initLDReactContext();
9090

91-
const clientA = makeMockClient();
92-
const clientB = makeMockClient();
91+
const clientA = makeMockClient({ initialState: 'complete' });
92+
const clientB = makeMockClient({ initialState: 'complete' });
9393

9494
const ProviderA = createLDReactProviderWithClient(clientA, ContextA);
9595
const ProviderB = createLDReactProviderWithClient(clientB, ContextB);

packages/sdk/react/__tests__/client/provider/namedContexts.test.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { makeMockClient } from '../mockClient';
1414
it('hook using a named context reads from the named client, not the global one', () => {
1515
const NamedContext = initLDReactContext();
1616

17-
const globalClient = makeMockClient();
18-
const namedClient = makeMockClient();
17+
const globalClient = makeMockClient({ initialState: 'complete' });
18+
const namedClient = makeMockClient({ initialState: 'complete' });
1919

2020
(globalClient.boolVariation as jest.Mock).mockReturnValue(true);
2121
(namedClient.boolVariation as jest.Mock).mockReturnValue(false);
@@ -46,8 +46,8 @@ it('hook using a named context reads from the named client, not the global one',
4646
it('hook with no context arg reads from the global LDReactContext provider', () => {
4747
const NamedContext = initLDReactContext();
4848

49-
const globalClient = makeMockClient();
50-
const namedClient = makeMockClient();
49+
const globalClient = makeMockClient({ initialState: 'complete' });
50+
const namedClient = makeMockClient({ initialState: 'complete' });
5151

5252
(globalClient.boolVariation as jest.Mock).mockReturnValue(true);
5353
(namedClient.boolVariation as jest.Mock).mockReturnValue(false);
@@ -78,7 +78,7 @@ it('hook with no context arg reads from the global LDReactContext provider', ()
7878
it('a named-context provider does not populate the global LDReactContext', () => {
7979
const NamedContext = initLDReactContext();
8080

81-
const namedClient = makeMockClient();
81+
const namedClient = makeMockClient({ initialState: 'complete' });
8282
const NamedProvider = createLDReactProviderWithClient(namedClient, NamedContext);
8383

8484
let globalContextValue: LDReactClientContextValue | undefined;
@@ -102,8 +102,8 @@ it('a named-context provider does not populate the global LDReactContext', () =>
102102
it('useBoolVariation and useLDClient return data from the named context when supplied', () => {
103103
const NamedContext = initLDReactContext();
104104

105-
const globalClient = makeMockClient();
106-
const namedClient = makeMockClient();
105+
const globalClient = makeMockClient({ initialState: 'complete' });
106+
const namedClient = makeMockClient({ initialState: 'complete' });
107107

108108
(namedClient.boolVariation as jest.Mock).mockReturnValue(true);
109109
(globalClient.boolVariation as jest.Mock).mockReturnValue(false);

packages/sdk/react/examples/client-only/src/App.tsx

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,41 +58,46 @@ function App() {
5858
statusMessage = 'Initializing…';
5959
}
6060

61-
const headerBgColor = flagValue ? '#00844B' : '#373841';
61+
const ready = status !== 'initializing';
62+
const headerBgColor = ready && flagValue ? '#00844B' : '#373841';
6263

6364
return (
6465
<div className="App">
6566
<header className="App-header" style={{ backgroundColor: headerBgColor }}>
66-
<p>{statusMessage}</p>
67-
<p>{`The ${flagKey} feature flag evaluates to ${flagValue ? 'true' : 'false'}.`}</p>
68-
<form onSubmit={handleSubmit}>
69-
<input
70-
type="text"
71-
value={inputValue}
72-
onChange={(e) => setInputValue(e.target.value)}
73-
placeholder="Flag key"
74-
aria-label="Flag key"
75-
/>
76-
<button type="submit">Update flag</button>
77-
</form>
78-
<div style={{ display: 'flex', gap: '8px' }}>
79-
{PRESET_CONTEXTS.map((preset) => {
80-
const isActive = activeContextKey === (preset.key as string);
81-
return (
82-
<button
83-
key={preset.key as string}
84-
onClick={() => handleSelectContext(preset)}
85-
disabled={identifyPending}
86-
style={{
87-
fontWeight: isActive ? 'bold' : 'normal',
88-
outline: isActive ? '2px solid white' : 'none',
89-
}}
90-
>
91-
{preset.name as string}
92-
</button>
93-
);
94-
})}
95-
</div>
67+
{ready && (
68+
<>
69+
<p>{statusMessage}</p>
70+
<p>{`The ${flagKey} feature flag evaluates to ${flagValue ? 'true' : 'false'}.`}</p>
71+
<form onSubmit={handleSubmit}>
72+
<input
73+
type="text"
74+
value={inputValue}
75+
onChange={(e) => setInputValue(e.target.value)}
76+
placeholder="Flag key"
77+
aria-label="Flag key"
78+
/>
79+
<button type="submit">Update flag</button>
80+
</form>
81+
<div style={{ display: 'flex', gap: '8px' }}>
82+
{PRESET_CONTEXTS.map((preset) => {
83+
const isActive = activeContextKey === (preset.key as string);
84+
return (
85+
<button
86+
key={preset.key as string}
87+
onClick={() => handleSelectContext(preset)}
88+
disabled={identifyPending}
89+
style={{
90+
fontWeight: isActive ? 'bold' : 'normal',
91+
outline: isActive ? '2px solid white' : 'none',
92+
}}
93+
>
94+
{preset.name as string}
95+
</button>
96+
);
97+
})}
98+
</div>
99+
</>
100+
)}
96101
</header>
97102
</div>
98103
);

0 commit comments

Comments
 (0)