Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 9 additions & 6 deletions src/provider/OptimizelyProvider.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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).
Comment thread
junaed-optimizely marked this conversation as resolved.
Outdated
expect(store.getState().error).toBeNull();
Comment thread
junaed-optimizely marked this conversation as resolved.
Outdated
});
});
Expand Down Expand Up @@ -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;

Expand All @@ -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 () => {
Expand Down
50 changes: 23 additions & 27 deletions src/provider/OptimizelyProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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 <OptimizelyContext.Provider value={contextValue}>{children}</OptimizelyContext.Provider>;
}
Loading