From 275a1dd55a9ace60746c257bd1240172f7df090b Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 4 May 2026 21:07:29 +0600 Subject: [PATCH 1/4] [FSSDK-12471] provider fix --- src/provider/OptimizelyProvider.spec.tsx | 15 ++++--- src/provider/OptimizelyProvider.tsx | 50 +++++++++++------------- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/provider/OptimizelyProvider.spec.tsx b/src/provider/OptimizelyProvider.spec.tsx index d24030c..9711fc6 100644 --- a/src/provider/OptimizelyProvider.spec.tsx +++ b/src/provider/OptimizelyProvider.spec.tsx @@ -228,7 +228,7 @@ describe('OptimizelyProvider', () => { }); describe('cleanup', () => { - it('should reset store on unmount', async () => { + it('should not reset store on unmount (store is GCd with component tree)', async () => { const mockClient = createMockClient(); let capturedContext: OptimizelyContextValue | null = null; @@ -246,8 +246,10 @@ describe('OptimizelyProvider', () => { unmount(); - // Store should be reset - expect(store.getState().userContext).toBeNull(); + // Store state is preserved — on real unmount, the store is + // garbage collected with the component tree. Eagerly resetting + // breaks React 18+ StrictMode (effect cleanup destroys state + // that the render body set, and no re-render restores it). expect(store.getState().error).toBeNull(); }); }); @@ -391,7 +393,7 @@ describe('OptimizelyProvider', () => { expect(mockClient2.createUserContext).toHaveBeenCalledWith('user-1', undefined); }); - it('should dispose manager on unmount', async () => { + it('should preserve store state on unmount (no eager reset)', async () => { const mockClient = createMockClient(); let capturedContext: OptimizelyContextValue | null = null; @@ -406,8 +408,9 @@ describe('OptimizelyProvider', () => { unmount(); - // Store should be reset after unmount - expect(capturedContext!.store.getState().userContext).toBeNull(); + // Store state is preserved after unmount — no eager reset. + // The store is garbage collected with the component tree. + expect(capturedContext!.store.getState().userContext).not.toBeNull(); }); it('should recreate user context when only attributes change (same id)', async () => { diff --git a/src/provider/OptimizelyProvider.tsx b/src/provider/OptimizelyProvider.tsx index eeb5027..8c7e2ca 100644 --- a/src/provider/OptimizelyProvider.tsx +++ b/src/provider/OptimizelyProvider.tsx @@ -70,8 +70,8 @@ export function OptimizelyProvider({ userManagerRef.current.resolveUserContext(user, qualifiedSegments, skipSegments); } - // Effect: Client onReady — only needed for error handling. - // Readiness is derived from userContext + getOptimizelyConfig() by hooks. + // Effect: Client readiness + config update subscription. + // Handles both initial datafile fetch and subsequent polling updates. useEffect(() => { if (!client) { console.error('[OPTIMIZELY - REACT] OptimizelyProvider must be passed an Optimizely client instance'); @@ -80,42 +80,38 @@ export function OptimizelyProvider({ } let isMounted = true; - - client.onReady({ timeout }).catch((error) => { - if (!isMounted) return; - const err = error instanceof Error ? error : new Error(String(error)); - store.setError(err); - }); - - return () => { - isMounted = false; - }; - }, [client, timeout, store]); - - // Effect: Subscribe to config/datafile updates (e.g., polling) - useEffect(() => { - if (!client) return; + // When the datafile response is cached (e.g. browser HTTP cache), + // CONFIG_UPDATE may fire before this effect subscribes. In that case + // onReady resolves but CONFIG_UPDATE is never re-emitted (config + // didn't change). The flag lets onReady act as a fallback without + // causing a double-refresh when both fire. + let configReceived = false; const listenerId = client.notificationCenter.addNotificationListener( NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE, () => { + configReceived = true; store.refresh(); } ); - return () => { - client.notificationCenter.removeNotificationListener(listenerId); - }; - }, [client, store]); + client + .onReady({ timeout }) + .then(() => { + if (!isMounted || configReceived) return; + store.refresh(); + }) + .catch((error) => { + if (!isMounted) return; + const err = error instanceof Error ? error : new Error(String(error)); + store.setError(err); + }); - // Cleanup on unmount - useEffect(() => { return () => { - userManagerRef.current?.dispose(); - userManagerRef.current = null; - store.reset(); + isMounted = false; + client.notificationCenter.removeNotificationListener(listenerId); }; - }, [store]); + }, [client, timeout, store]); return {children}; } From 773436c0710b0b3a9a7b301b2b230e6296bcc602 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Wed, 6 May 2026 19:55:34 +0600 Subject: [PATCH 2/4] [FSSDK-12471] provider update --- src/provider/OptimizelyProvider.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/provider/OptimizelyProvider.tsx b/src/provider/OptimizelyProvider.tsx index 8c7e2ca..3d24001 100644 --- a/src/provider/OptimizelyProvider.tsx +++ b/src/provider/OptimizelyProvider.tsx @@ -38,6 +38,7 @@ export function OptimizelyProvider({ const storeRef = useRef(null); const userManagerRef = useRef(null); const prevClientRef = useRef(); + const hadConfigAtRender = useMemo(() => !!client?.getOptimizelyConfig(), [client]); if (storeRef.current === null) { storeRef.current = new ProviderStateStore(); @@ -98,7 +99,7 @@ export function OptimizelyProvider({ client .onReady({ timeout }) .then(() => { - if (!isMounted || configReceived) return; + if (!isMounted || configReceived || hadConfigAtRender) return; store.refresh(); }) .catch((error) => { @@ -111,7 +112,7 @@ export function OptimizelyProvider({ isMounted = false; client.notificationCenter.removeNotificationListener(listenerId); }; - }, [client, timeout, store]); + }, [client, timeout, store, hadConfigAtRender]); return {children}; } From 44339b3dbb8e1e916c90f48607cb039ecd33ba04 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Wed, 6 May 2026 20:00:56 +0600 Subject: [PATCH 3/4] [FSSDK-12471] review update --- src/provider/OptimizelyProvider.spec.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/provider/OptimizelyProvider.spec.tsx b/src/provider/OptimizelyProvider.spec.tsx index 9711fc6..b2cb304 100644 --- a/src/provider/OptimizelyProvider.spec.tsx +++ b/src/provider/OptimizelyProvider.spec.tsx @@ -228,7 +228,7 @@ describe('OptimizelyProvider', () => { }); describe('cleanup', () => { - it('should not reset store on unmount (store is GCd with component tree)', async () => { + it('should not reset store on unmount (store becomes unreachable to React tree)', async () => { const mockClient = createMockClient(); let capturedContext: OptimizelyContextValue | null = null; @@ -246,10 +246,11 @@ describe('OptimizelyProvider', () => { unmount(); - // Store state is preserved — on real unmount, the store is - // garbage collected with the component tree. Eagerly resetting - // breaks React 18+ StrictMode (effect cleanup destroys state - // that the render body set, and no re-render restores it). + // Store state is preserved — on real unmount, the store becomes + // unreachable to the React tree. Eagerly resetting breaks React + // 18+ StrictMode (effect cleanup destroys state that the render + // body set, and no re-render restores it). + expect(store.getState().userContext).not.toBeNull(); expect(store.getState().error).toBeNull(); }); }); @@ -409,7 +410,7 @@ describe('OptimizelyProvider', () => { unmount(); // Store state is preserved after unmount — no eager reset. - // The store is garbage collected with the component tree. + // The store becomes unreachable to the React tree. expect(capturedContext!.store.getState().userContext).not.toBeNull(); }); From e6f60551f69192dc698d3b633e58a5759c492642 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 8 May 2026 00:09:03 +0600 Subject: [PATCH 4/4] [FSSDK-12471] review update --- src/provider/OptimizelyProvider.spec.tsx | 91 +++++++++++++++++++++--- src/provider/OptimizelyProvider.tsx | 36 +++++----- 2 files changed, 98 insertions(+), 29 deletions(-) diff --git a/src/provider/OptimizelyProvider.spec.tsx b/src/provider/OptimizelyProvider.spec.tsx index b2cb304..1e083c2 100644 --- a/src/provider/OptimizelyProvider.spec.tsx +++ b/src/provider/OptimizelyProvider.spec.tsx @@ -638,7 +638,7 @@ describe('OptimizelyProvider', () => { }); describe('config update subscription', () => { - it('should subscribe to OPTIMIZELY_CONFIG_UPDATE on mount', () => { + it('should subscribe to OPTIMIZELY_CONFIG_UPDATE after onReady', async () => { const mockClient = createMockClient(); render( @@ -647,13 +647,15 @@ describe('OptimizelyProvider', () => { ); - expect(mockClient.notificationCenter.addNotificationListener).toHaveBeenCalledWith( - 'OPTIMIZELY_CONFIG_UPDATE', - expect.any(Function) - ); + await waitFor(() => { + expect(mockClient.notificationCenter.addNotificationListener).toHaveBeenCalledWith( + 'OPTIMIZELY_CONFIG_UPDATE', + expect.any(Function) + ); + }); }); - it('should remove notification listener on unmount', () => { + it('should remove notification listener on unmount', async () => { const mockClient = createMockClient(); const { unmount } = render( @@ -662,6 +664,10 @@ describe('OptimizelyProvider', () => { ); + await waitFor(() => { + expect(mockClient.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); + }); + unmount(); expect(mockClient.notificationCenter.removeNotificationListener).toHaveBeenCalledWith(1); @@ -679,6 +685,7 @@ describe('OptimizelyProvider', () => { await waitFor(() => { expect(capturedContext).not.toBeNull(); + expect(mockClient.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); }); const stateBefore = capturedContext!.store.getState(); @@ -698,7 +705,7 @@ describe('OptimizelyProvider', () => { expect(stateBefore).not.toBe(stateAfter); }); - it('should re-subscribe when client changes', () => { + it('should re-subscribe when client changes', async () => { const mockClient1 = createMockClient(); const mockClient2 = createMockClient(); @@ -708,7 +715,9 @@ describe('OptimizelyProvider', () => { ); - expect(mockClient1.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(mockClient1.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); + }); rerender( @@ -716,9 +725,71 @@ describe('OptimizelyProvider', () => { ); - // Old listener cleaned up, new one registered + await waitFor(() => { + expect(mockClient2.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); + }); + + // Old listener cleaned up expect(mockClient1.notificationCenter.removeNotificationListener).toHaveBeenCalledWith(1); - expect(mockClient2.notificationCenter.addNotificationListener).toHaveBeenCalledTimes(1); + }); + }); + + describe('config revision race condition', () => { + it('should refresh when config revision changes between render and onReady', async () => { + let resolveOnReady: () => void; + const onReadyPromise = new Promise((resolve) => { + resolveOnReady = resolve; + }); + + const mockClient = createMockClient({ + getOptimizelyConfig: vi.fn().mockReturnValue({ revision: '1' }), + onReady: vi.fn().mockReturnValue(onReadyPromise), + }); + let capturedContext: OptimizelyContextValue | null = null; + + render( + + (capturedContext = ctx)} /> + + ); + + expect(capturedContext).not.toBeNull(); + const stateBefore = capturedContext!.store.getState(); + + // Config updates to v2 before onReady resolves + (mockClient.getOptimizelyConfig as ReturnType).mockReturnValue({ revision: '2' }); + + await act(async () => { + resolveOnReady!(); + }); + + const stateAfter = capturedContext!.store.getState(); + expect(stateBefore).not.toBe(stateAfter); + }); + + it('should not refresh when config revision is unchanged at onReady', async () => { + const mockClient = createMockClient({ + getOptimizelyConfig: vi.fn().mockReturnValue({ revision: '1' }), + }); + let capturedContext: OptimizelyContextValue | null = null; + + render( + + (capturedContext = ctx)} /> + + ); + + await waitFor(() => { + expect(capturedContext).not.toBeNull(); + }); + + const stateBefore = capturedContext!.store.getState(); + + // Flush onReady microtask — revision is still '1' + await act(async () => {}); + + const stateAfter = capturedContext!.store.getState(); + expect(stateBefore).toBe(stateAfter); }); }); diff --git a/src/provider/OptimizelyProvider.tsx b/src/provider/OptimizelyProvider.tsx index 3d24001..e34ef20 100644 --- a/src/provider/OptimizelyProvider.tsx +++ b/src/provider/OptimizelyProvider.tsx @@ -38,7 +38,7 @@ export function OptimizelyProvider({ const storeRef = useRef(null); const userManagerRef = useRef(null); const prevClientRef = useRef(); - const hadConfigAtRender = useMemo(() => !!client?.getOptimizelyConfig(), [client]); + const revisionAtRender = useMemo(() => client?.getOptimizelyConfig()?.revision, [client]); if (storeRef.current === null) { storeRef.current = new ProviderStateStore(); @@ -81,26 +81,22 @@ export function OptimizelyProvider({ } let isMounted = true; - // When the datafile response is cached (e.g. browser HTTP cache), - // CONFIG_UPDATE may fire before this effect subscribes. In that case - // onReady resolves but CONFIG_UPDATE is never re-emitted (config - // didn't change). The flag lets onReady act as a fallback without - // causing a double-refresh when both fire. - let configReceived = false; - - const listenerId = client.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE, - () => { - configReceived = true; - store.refresh(); - } - ); + let listenerId: number | undefined; client .onReady({ timeout }) .then(() => { - if (!isMounted || configReceived || hadConfigAtRender) return; - store.refresh(); + if (!isMounted) return; + + const currentRevision = client.getOptimizelyConfig()?.revision; + if (currentRevision !== revisionAtRender) { + store.refresh(); + } + + listenerId = client.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE, + () => store.refresh() + ); }) .catch((error) => { if (!isMounted) return; @@ -110,9 +106,11 @@ export function OptimizelyProvider({ return () => { isMounted = false; - client.notificationCenter.removeNotificationListener(listenerId); + if (listenerId !== undefined) { + client.notificationCenter.removeNotificationListener(listenerId); + } }; - }, [client, timeout, store, hadConfigAtRender]); + }, [client, timeout, store, revisionAtRender]); return {children}; }