Skip to content

Commit 07cded8

Browse files
[fssdk-12294] provider config update listener setting
1 parent a3e621b commit 07cded8

8 files changed

Lines changed: 39 additions & 155 deletions

src/hooks/useDecide.spec.tsx

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -221,25 +221,6 @@ describe('useDecide', () => {
221221
expect(result.current.decision).toBe(MOCK_DECISION);
222222
});
223223

224-
it('should re-evaluate when setClientReady fire', async () => {
225-
const mockUserContext = createMockUserContext();
226-
store.setUserContext(mockUserContext);
227-
// Client has no config yet
228-
const wrapper = createWrapper(store, mockClient);
229-
const { result } = renderHook(() => useDecide('flag_1'), { wrapper });
230-
231-
expect(result.current.isLoading).toBe(true);
232-
233-
// Simulate config becoming available when onReady resolves
234-
(mockClient.getOptimizelyConfig as ReturnType<typeof vi.fn>).mockReturnValue({ revision: '1' });
235-
await act(async () => {
236-
store.setClientReady(true);
237-
});
238-
239-
expect(result.current.isLoading).toBe(false);
240-
expect(result.current.decision).toBe(MOCK_DECISION);
241-
});
242-
243224
it('should return error from store with isLoading: false', async () => {
244225
const wrapper = createWrapper(store, mockClient);
245226
const { result } = renderHook(() => useDecide('flag_1'), { wrapper });
@@ -363,33 +344,6 @@ describe('useDecide', () => {
363344
expect(result.current.decision).toBeNull();
364345
});
365346

366-
it('should re-call decide() when setClientReady fires after sync decision was already served', async () => {
367-
// Sync datafile scenario: config + userContext available before onReady
368-
mockClient = createMockClient(true);
369-
const mockUserContext = createMockUserContext();
370-
store.setUserContext(mockUserContext);
371-
372-
const wrapper = createWrapper(store, mockClient);
373-
const { result } = renderHook(() => useDecide('flag_1'), { wrapper });
374-
375-
// Decision already served
376-
expect(result.current.isLoading).toBe(false);
377-
expect(result.current.decision).toBe(MOCK_DECISION);
378-
expect(mockUserContext.decide).toHaveBeenCalledTimes(1);
379-
380-
// onReady() resolves → setClientReady(true) fires → store state changes →
381-
// useSyncExternalStore re-renders → useMemo recomputes → decide() called again.
382-
// This is a redundant call since config + userContext haven't changed,
383-
// but it's a one-time cost per flag per page load.
384-
await act(async () => {
385-
store.setClientReady(true);
386-
});
387-
388-
expect(mockUserContext.decide).toHaveBeenCalledTimes(2);
389-
expect(result.current.isLoading).toBe(false);
390-
expect(result.current.decision).toBe(MOCK_DECISION);
391-
});
392-
393347
it('should re-evaluate decision when OPTIMIZELY_CONFIG_UPDATE fires from the client', async () => {
394348
const mockUserContext = createMockUserContext();
395349
const { wrapper, fireConfigUpdate } = createProviderWrapper(mockUserContext);
@@ -402,6 +356,7 @@ describe('useDecide', () => {
402356
});
403357

404358
expect(result.current.decision).toBe(MOCK_DECISION);
359+
405360
const callCountBeforeUpdate = (mockUserContext.decide as ReturnType<typeof vi.fn>).mock.calls.length;
406361

407362
// Simulate a new datafile with a different decision

src/hooks/useOptimizelyClient.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,12 @@ describe('useOptimizelyClient', () => {
8181

8282
// Trigger store state changes that should NOT cause useOptimizelyClient to re-render
8383
act(() => {
84-
store.setClientReady(true);
84+
store.setError(new Error('test'));
8585
});
8686
expect(capturedRenderCount).toBe(initialRenderCount);
8787

8888
act(() => {
89-
store.setError(new Error('test'));
89+
store.refresh();
9090
});
9191
expect(capturedRenderCount).toBe(initialRenderCount);
9292
});

src/hooks/useOptimizelyUserContext.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,11 @@ describe('useOptimizelyUserContext', () => {
206206

207207
const initialRenderCount = capturedRenderCount;
208208

209-
// Changing isClientReady triggers a store notification,
209+
// Triggering a store notification via setState,
210210
// but since the derived result hasn't changed, useMemo returns
211211
// the same reference and React bails out
212212
act(() => {
213-
store.setClientReady(true);
213+
store.refresh();
214214
});
215215

216216
expect(capturedRenderCount).toBe(initialRenderCount);

src/provider/OptimizelyProvider.spec.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ describe('OptimizelyProvider', () => {
154154
expect(mockClient.onReady).toHaveBeenCalledWith({ timeout: 5000 });
155155
});
156156

157-
it('should set isClientReady to true when onReady succeeds', async () => {
157+
it('should not set error when onReady succeeds', async () => {
158158
const mockClient = createMockClient({
159159
onReady: vi.fn().mockResolvedValue(undefined),
160160
});
@@ -168,13 +168,12 @@ describe('OptimizelyProvider', () => {
168168

169169
await waitFor(() => {
170170
expect(capturedContext).not.toBeNull();
171-
expect(capturedContext!.store.getState().isClientReady).toBe(true);
172171
});
173172

174173
expect(capturedContext!.store.getState().error).toBeNull();
175174
});
176175

177-
it('should set isClientReady to false and set error when onReady rejects', async () => {
176+
it('should set error when onReady rejects', async () => {
178177
const testError = new Error('Client initialization failed');
179178
const mockClient = createMockClient({
180179
onReady: vi.fn().mockRejectedValue(testError),
@@ -191,9 +190,6 @@ describe('OptimizelyProvider', () => {
191190
expect(capturedContext).not.toBeNull();
192191
expect(capturedContext!.store.getState().error).toBe(testError);
193192
});
194-
195-
// Client is NOT ready when onReady rejects
196-
expect(capturedContext!.store.getState().isClientReady).toBe(false);
197193
});
198194

199195
it('should set error when onReady times out (rejects)', async () => {
@@ -213,8 +209,6 @@ describe('OptimizelyProvider', () => {
213209
expect(capturedContext).not.toBeNull();
214210
expect(capturedContext!.store.getState().error).toBe(timeoutError);
215211
});
216-
217-
expect(capturedContext!.store.getState().isClientReady).toBe(false);
218212
});
219213
});
220214

@@ -246,15 +240,13 @@ describe('OptimizelyProvider', () => {
246240

247241
await waitFor(() => {
248242
expect(capturedContext).not.toBeNull();
249-
expect(capturedContext!.store.getState().isClientReady).toBe(true);
250243
});
251244

252245
const store = capturedContext!.store;
253246

254247
unmount();
255248

256249
// Store should be reset
257-
expect(store.getState().isClientReady).toBe(false);
258250
expect(store.getState().userContext).toBeNull();
259251
expect(store.getState().error).toBeNull();
260252
});
@@ -572,8 +564,8 @@ describe('OptimizelyProvider', () => {
572564
resolveOnReady!();
573565
});
574566

575-
// Store was reset on unmount, and onReady resolution should not set isClientReady
576-
expect(store.getState().isClientReady).toBe(false);
567+
// Store was reset on unmount, onReady resolution should not affect store
568+
expect(store.getState().error).toBeNull();
577569
});
578570

579571
it('should call onReady again when client changes', async () => {

src/provider/OptimizelyProvider.tsx

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export function OptimizelyProvider({
7777
userManagerRef.current.resolveUserContext(user, qualifiedSegments, skipSegments);
7878
}
7979

80-
// Effect: Client onReady
80+
// Effect: Client onReady — only needed for error handling.
81+
// Readiness is derived from userContext + getOptimizelyConfig() by hooks.
8182
useEffect(() => {
8283
if (!client) {
8384
logger?.error('OptimizelyProvider must be passed an Optimizely client instance');
@@ -87,24 +88,11 @@ export function OptimizelyProvider({
8788

8889
let isMounted = true;
8990

90-
const waitForClientReady = async (): Promise<void> => {
91-
try {
92-
await client.onReady({ timeout });
93-
94-
if (!isMounted) return;
95-
96-
store.setClientReady(true);
97-
} catch (error) {
98-
if (!isMounted) return;
99-
const err = error instanceof Error ? error : new Error(String(error));
100-
store.setState({
101-
isClientReady: false,
102-
error: err,
103-
});
104-
}
105-
};
106-
107-
waitForClientReady();
91+
client.onReady({ timeout }).catch((error) => {
92+
if (!isMounted) return;
93+
const err = error instanceof Error ? error : new Error(String(error));
94+
store.setError(err);
95+
});
10896

10997
return () => {
11098
isMounted = false;
@@ -118,7 +106,7 @@ export function OptimizelyProvider({
118106
const listenerId = client.notificationCenter.addNotificationListener(
119107
NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE,
120108
() => {
121-
store.setState({});
109+
store.refresh();
122110
}
123111
);
124112

0 commit comments

Comments
 (0)