diff --git a/src/hooks/types.ts b/src/hooks/types.ts index 71f25df..f286498 100644 --- a/src/hooks/types.ts +++ b/src/hooks/types.ts @@ -22,10 +22,10 @@ export interface UseDecideConfig { export type UseDecideResult = | { isLoading: true; error: null; decision: null } - | { isLoading: false; error: Error; decision: null } + | { isLoading: false; error: Error; decision: OptimizelyDecision | null } | { isLoading: false; error: null; decision: OptimizelyDecision }; export type UseDecideMultiResult = | { isLoading: true; error: null; decisions: Record } - | { isLoading: false; error: Error; decisions: Record } + | { isLoading: false; error: Error; decisions: Record } | { isLoading: false; error: null; decisions: Record }; diff --git a/src/hooks/useAsyncDecision.ts b/src/hooks/useAsyncDecision.ts index 6a8728f..5c464c8 100644 --- a/src/hooks/useAsyncDecision.ts +++ b/src/hooks/useAsyncDecision.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import type { OptimizelyUserContext } from '@optimizely/optimizely-sdk'; import type { Client } from '@optimizely/optimizely-sdk'; @@ -52,34 +52,40 @@ export function useAsyncDecision( isLoading: true, }); + const prevUserContextRef = useRef(null); + useEffect(() => { const { userContext, error } = state; const hasConfig = client.getOptimizelyConfig() !== null; - // Store-level error — no async call needed - if (error) { - setAsyncState({ result: emptyResult, error, isLoading: false }); + // No decision possible — surface store error or stay loading + if (!hasConfig || userContext === null) { + if (error) { + setAsyncState({ result: emptyResult, error, isLoading: false }); + } else { + setAsyncState((prev) => { + if (prev.isLoading) return prev; + return { result: emptyResult, error: null, isLoading: true }; + }); + } return; } - // Ensure loading state (skip if already loading to avoid re-render) + const userContextChanged = userContext !== prevUserContextRef.current; + prevUserContextRef.current = userContext; + setAsyncState((prev) => { if (prev.isLoading) return prev; - return { result: emptyResult, error: null, isLoading: true }; + return { result: userContextChanged ? emptyResult : prev.result, error: null, isLoading: true }; }); - // Store not ready — wait for config/user context - if (!hasConfig || userContext === null) { - return; - } - - // Store is ready — fire async decision + // Config + userContext available — fire async decision even if store has error let cancelled = false; execute(userContext).then( (result) => { if (!cancelled) { - setAsyncState({ result, error: null, isLoading: false }); + setAsyncState({ result, error, isLoading: false }); } }, (err) => { diff --git a/src/hooks/useDecide.spec.tsx b/src/hooks/useDecide.spec.tsx index 4e36b55..92988b8 100644 --- a/src/hooks/useDecide.spec.tsx +++ b/src/hooks/useDecide.spec.tsx @@ -129,7 +129,7 @@ describe('useDecide', () => { expect(result.current.decision).toBe(MOCK_DECISION); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecide('flag_1'), { wrapper }); @@ -145,6 +145,28 @@ describe('useDecide', () => { expect(result.current.decision).toBeNull(); }); + it('should return stale decision alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecide('flag_1'), { wrapper }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.decision).toBe(MOCK_DECISION); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.error).toBe(testError); + expect(result.current.decision).toBe(MOCK_DECISION); + }); + it('should re-evaluate when flagKey changes', () => { mockClient = createMockClient(true); const mockUserContext = createMockUserContext(); diff --git a/src/hooks/useDecide.ts b/src/hooks/useDecide.ts index 79807a0..0312ee2 100644 --- a/src/hooks/useDecide.ts +++ b/src/hooks/useDecide.ts @@ -52,15 +52,15 @@ export function useDecide(flagKey: string, config?: UseDecideConfig): UseDecideR const { userContext, error } = state; const hasConfig = client.getOptimizelyConfig() !== null; - if (error) { - return { decision: null, isLoading: false, error }; + if (hasConfig && userContext !== null) { + const decision = userContext.decide(flagKey, decideOptions); + return { decision, isLoading: false, error }; } - if (!hasConfig || userContext === null) { - return { decision: null, isLoading: true, error: null }; + if (error) { + return { decision: null, isLoading: false, error }; } - const decision = userContext.decide(flagKey, decideOptions); - return { decision, isLoading: false, error: null }; + return { decision: null, isLoading: true, error: null }; }, [fdVersion, state, client, flagKey, decideOptions]); } diff --git a/src/hooks/useDecideAll.spec.tsx b/src/hooks/useDecideAll.spec.tsx index bbbe11f..709497f 100644 --- a/src/hooks/useDecideAll.spec.tsx +++ b/src/hooks/useDecideAll.spec.tsx @@ -112,7 +112,7 @@ describe('useDecideAll', () => { expect(mockUserContext.decideAll).toHaveBeenCalledWith(decideOptions); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecideAll(), { wrapper }); @@ -128,6 +128,28 @@ describe('useDecideAll', () => { expect(result.current.decisions).toEqual({}); }); + it('should return stale decisions alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideAll(), { wrapper }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.error).toBe(testError); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + }); + it('should not call decideAll() while loading', () => { const mockUserContext = createMockUserContext(); store.setUserContext(mockUserContext); diff --git a/src/hooks/useDecideAll.ts b/src/hooks/useDecideAll.ts index 1cabf07..f04389a 100644 --- a/src/hooks/useDecideAll.ts +++ b/src/hooks/useDecideAll.ts @@ -15,6 +15,7 @@ */ import { useEffect, useMemo, useState } from 'react'; +import type { OptimizelyDecision } from '@optimizely/optimizely-sdk'; import { useOptimizelyContext } from './useOptimizelyContext'; import { useProviderState } from './useProviderState'; @@ -47,15 +48,15 @@ export function useDecideAll(config?: UseDecideConfig): UseDecideMultiResult { const { userContext, error } = state; const hasConfig = client.getOptimizelyConfig() !== null; - if (error) { - return { decisions: {} as Record, isLoading: false as const, error }; + if (hasConfig && userContext !== null) { + const decisions = userContext.decideAll(decideOptions); + return { decisions, isLoading: false as const, error }; } - if (!hasConfig || userContext === null) { - return { decisions: {} as Record, isLoading: true as const, error: null }; + if (error) { + return { decisions: {} as Record, isLoading: false as const, error }; } - const decisions = userContext.decideAll(decideOptions); - return { decisions, isLoading: false as const, error: null }; + return { decisions: {} as Record, isLoading: true as const, error: null }; }, [fdVersion, state, client, decideOptions]); } diff --git a/src/hooks/useDecideAllAsync.spec.tsx b/src/hooks/useDecideAllAsync.spec.tsx index f6a0083..0582bce 100644 --- a/src/hooks/useDecideAllAsync.spec.tsx +++ b/src/hooks/useDecideAllAsync.spec.tsx @@ -68,7 +68,7 @@ describe('useDecideAllAsync', () => { expect(result.current.error).toBeNull(); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecideAllAsync(), { wrapper }); @@ -84,6 +84,34 @@ describe('useDecideAllAsync', () => { expect(result.current.decisions).toEqual({}); }); + it('should return stale decisions alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideAllAsync(), { wrapper }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.error).toBe(testError); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + }); + it('should return isLoading: true while async call is in-flight', () => { mockClient = createMockClient(true); const mockUserContext = createMockUserContext(); diff --git a/src/hooks/useDecideAsync.spec.tsx b/src/hooks/useDecideAsync.spec.tsx index 7b22677..aeeeea1 100644 --- a/src/hooks/useDecideAsync.spec.tsx +++ b/src/hooks/useDecideAsync.spec.tsx @@ -68,7 +68,7 @@ describe('useDecideAsync', () => { expect(result.current.error).toBeNull(); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecideAsync('flag_1'), { wrapper }); @@ -84,6 +84,60 @@ describe('useDecideAsync', () => { expect(result.current.decision).toBeNull(); }); + it('should return stale decision alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideAsync('flag_1'), { wrapper }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.decision).toBe(MOCK_DECISION); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.error).toBe(testError); + expect(result.current.decision).toBe(MOCK_DECISION); + }); + + it('should preserve previous decision during re-execute when only store error changes', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideAsync('flag_1'), { wrapper }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + expect(result.current.decision).toBe(MOCK_DECISION); + + // Make next async call hang so we can observe intermediate state + (mockUserContext.decideAsync as ReturnType).mockReturnValue(new Promise(() => {})); + + const testError = new Error('CDN refresh failed'); + await act(async () => { + store.setError(testError); + }); + + // Should be loading but still have the previous decision, not null + expect(result.current.isLoading).toBe(true); + expect(result.current.decision).toBe(MOCK_DECISION); + }); + it('should return isLoading: true while async call is in-flight', () => { mockClient = createMockClient(true); const mockUserContext = createMockUserContext(); diff --git a/src/hooks/useDecideForKeys.spec.tsx b/src/hooks/useDecideForKeys.spec.tsx index faf70ca..69ee655 100644 --- a/src/hooks/useDecideForKeys.spec.tsx +++ b/src/hooks/useDecideForKeys.spec.tsx @@ -112,7 +112,7 @@ describe('useDecideForKeys', () => { expect(mockUserContext.decideForKeys).toHaveBeenCalledWith(['flag_1'], decideOptions); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecideForKeys(['flag_1']), { wrapper }); @@ -128,6 +128,28 @@ describe('useDecideForKeys', () => { expect(result.current.decisions).toEqual({}); }); + it('should return stale decisions alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideForKeys(['flag_1', 'flag_2']), { wrapper }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.error).toBe(testError); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + }); + it('should not call decideForKeys() while loading', () => { const mockUserContext = createMockUserContext(); store.setUserContext(mockUserContext); diff --git a/src/hooks/useDecideForKeys.ts b/src/hooks/useDecideForKeys.ts index 7e43fb9..0580349 100644 --- a/src/hooks/useDecideForKeys.ts +++ b/src/hooks/useDecideForKeys.ts @@ -15,6 +15,8 @@ */ import { useEffect, useMemo, useState } from 'react'; +import type { OptimizelyDecision } from '@optimizely/optimizely-sdk'; + import { useOptimizelyContext } from './useOptimizelyContext'; import { useProviderState } from './useProviderState'; import { useStableArray } from './useStableArray'; @@ -50,15 +52,15 @@ export function useDecideForKeys(flagKeys: string[], config?: UseDecideConfig): const { userContext, error } = state; const hasConfig = client.getOptimizelyConfig() !== null; - if (error) { - return { decisions: {}, isLoading: false, error }; + if (hasConfig && userContext !== null) { + const decisions = userContext.decideForKeys(stableKeys, decideOptions); + return { decisions, isLoading: false as const, error }; } - if (!hasConfig || userContext === null) { - return { decisions: {}, isLoading: true, error: null }; + if (error) { + return { decisions: {} as Record, isLoading: false as const, error }; } - const decisions = userContext.decideForKeys(stableKeys, decideOptions); - return { decisions, isLoading: false as const, error: null }; + return { decisions: {} as Record, isLoading: true as const, error: null }; }, [fdVersion, state, client, stableKeys, decideOptions]); } diff --git a/src/hooks/useDecideForKeysAsync.spec.tsx b/src/hooks/useDecideForKeysAsync.spec.tsx index af3bbf2..5cb731f 100644 --- a/src/hooks/useDecideForKeysAsync.spec.tsx +++ b/src/hooks/useDecideForKeysAsync.spec.tsx @@ -68,7 +68,7 @@ describe('useDecideForKeysAsync', () => { expect(result.current.error).toBeNull(); }); - it('should return error from store with isLoading: false', async () => { + it('should return error from store with isLoading: false when no decision is possible', async () => { const wrapper = createWrapper(store, mockClient); const { result } = renderHook(() => useDecideForKeysAsync(['flag_1']), { wrapper }); @@ -84,6 +84,34 @@ describe('useDecideForKeysAsync', () => { expect(result.current.decisions).toEqual({}); }); + it('should return stale decisions alongside error when config and user context are available', async () => { + mockClient = createMockClient(true); + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store, mockClient); + const { result } = renderHook(() => useDecideForKeysAsync(['flag_1', 'flag_2']), { wrapper }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + expect(result.current.error).toBe(testError); + expect(result.current.decisions).toEqual(MOCK_DECISIONS); + }); + it('should return isLoading: true while async call is in-flight', () => { mockClient = createMockClient(true); const mockUserContext = createMockUserContext(); diff --git a/src/hooks/useOptimizelyUserContext.spec.tsx b/src/hooks/useOptimizelyUserContext.spec.tsx index 55acdcd..b216abd 100644 --- a/src/hooks/useOptimizelyUserContext.spec.tsx +++ b/src/hooks/useOptimizelyUserContext.spec.tsx @@ -150,7 +150,7 @@ describe('useOptimizelyUserContext', () => { expect(result.current.userContext).toBeNull(); }); - it('should return error with isLoading: false when store has error', async () => { + it('should return error with isLoading: false when store has error and no user context', async () => { const wrapper = createWrapper(store); const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper }); @@ -166,6 +166,27 @@ describe('useOptimizelyUserContext', () => { expect(result.current.userContext).toBeNull(); }); + it('should return stale user context alongside error when user context was already set', async () => { + const mockUserContext = createMockUserContext(); + store.setUserContext(mockUserContext); + + const wrapper = createWrapper(store); + const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.userContext).toBe(mockUserContext); + expect(result.current.error).toBeNull(); + + const testError = new Error('CDN datafile fetch failed'); + await act(async () => { + store.setError(testError); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.error).toBe(testError); + expect(result.current.userContext).toBe(mockUserContext); + }); + it('should unsubscribe from store on unmount', () => { const unsubscribeSpy = vi.fn(); const subscribeSpy = vi.spyOn(store, 'subscribe').mockReturnValue(unsubscribeSpy); diff --git a/src/hooks/useOptimizelyUserContext.ts b/src/hooks/useOptimizelyUserContext.ts index a03a32c..576f781 100644 --- a/src/hooks/useOptimizelyUserContext.ts +++ b/src/hooks/useOptimizelyUserContext.ts @@ -22,7 +22,7 @@ import { useOptimizelyContext } from './useOptimizelyContext'; export type UseOptimizelyUserContextResult = | { isLoading: true; error: null; userContext: null } - | { isLoading: false; error: Error; userContext: null } + | { isLoading: false; error: Error; userContext: OptimizelyUserContext | null } | { isLoading: false; error: null; userContext: OptimizelyUserContext }; /** @@ -42,14 +42,14 @@ export function useOptimizelyUserContext(): UseOptimizelyUserContextResult { return useMemo(() => { const { userContext, error } = state; - if (error) { - return { userContext: null, isLoading: false, error }; + if (userContext !== null) { + return { userContext, isLoading: false, error }; } - if (userContext === null) { - return { userContext: null, isLoading: true, error: null }; + if (error) { + return { userContext: null, isLoading: false, error }; } - return { userContext, isLoading: false, error: null }; + return { userContext: null, isLoading: true, error: null }; }, [state]); }