diff --git a/src/provider/OptimizelyProvider.spec.tsx b/src/provider/OptimizelyProvider.spec.tsx index d24030c..b2cb304 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 becomes unreachable to React tree)', async () => { const mockClient = createMockClient(); let capturedContext: OptimizelyContextValue | null = null; @@ -246,8 +246,11 @@ describe('OptimizelyProvider', () => { unmount(); - // Store should be reset - expect(store.getState().userContext).toBeNull(); + // 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(); }); }); @@ -391,7 +394,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 +409,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 becomes unreachable to the React 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..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(); @@ -70,8 +71,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 +81,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 || hadConfigAtRender) 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, hadConfigAtRender]); return {children}; }