[FSSDK-12293] feedback addressed

Author: junaed-optimizely

src/hooks/index.ts

@@ -16,5 +16,6 @@
 
 export { useOptimizelyClient } from './useOptimizelyClient';
 export { useOptimizelyUserContext } from './useOptimizelyUserContext';
+export type { UseOptimizelyUserContextResult } from './useOptimizelyUserContext';
 export { useDecide } from './useDecide';
 export type { UseDecideConfig, UseDecideResult } from './useDecide';

src/hooks/useDecide.spec.tsx

@@ -102,9 +102,7 @@ describe('useDecide', () => {
 
     expect(result.current.isLoading).toBe(true);
     expect(result.current.error).toBeNull();
-    expect(result.current.decision.enabled).toBe(false);
-    expect(result.current.decision.variationKey).toBeNull();
-    expect(result.current.decision.flagKey).toBe('flag_1');
+    expect(result.current.decision).toBeNull();
   });
 
   it('should return isLoading: true when config is available but no user context', () => {
@@ -127,17 +125,13 @@ describe('useDecide', () => {
     expect(result.current.error).toBeNull();
   });
 
-  it('should return default decision while loading', () => {
+  it('should return null decision while loading', () => {
     const wrapper = createWrapper(store, mockClient);
     const { result } = renderHook(() => useDecide('my_flag'), { wrapper });
 
-    const { decision } = result.current;
-    expect(decision.enabled).toBe(false);
-    expect(decision.variationKey).toBeNull();
-    expect(decision.ruleKey).toBeNull();
-    expect(decision.variables).toEqual({});
-    expect(decision.flagKey).toBe('my_flag');
-    expect(decision.reasons).toContain('Optimizely SDK not configured properly yet.');
+    expect(result.current.decision).toBeNull();
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.error).toBeNull();
   });
 
   it('should return actual decision when config and user context are available', () => {
@@ -215,8 +209,7 @@ describe('useDecide', () => {
 
     expect(result.current.isLoading).toBe(false);
     expect(result.current.error).toBe(testError);
-    expect(result.current.decision.enabled).toBe(false);
-    expect(result.current.decision.variationKey).toBeNull();
+    expect(result.current.decision).toBeNull();
   });
 
   it('should re-evaluate when flagKey changes', () => {
@@ -312,18 +305,18 @@ describe('useDecide', () => {
     expect(mockUserContext.decide).not.toHaveBeenCalled();
   });
 
-  it('should update default decision flagKey when flagKey changes', () => {
+  it('should return null decision for both flagKeys when loading', () => {
     const wrapper = createWrapper(store, mockClient);
     const { result, rerender } = renderHook(({ flagKey }) => useDecide(flagKey), {
       wrapper,
       initialProps: { flagKey: 'flag_a' },
     });
 
-    expect(result.current.decision.flagKey).toBe('flag_a');
+    expect(result.current.decision).toBeNull();
 
     rerender({ flagKey: 'flag_b' });
 
-    expect(result.current.decision.flagKey).toBe('flag_b');
+    expect(result.current.decision).toBeNull();
   });
 
   it('should re-call decide() when setClientReady fires after sync decision was already served', async () => {

src/hooks/useDecide.ts

@@ -20,17 +20,15 @@ import type { OptimizelyDecideOption, OptimizelyDecision } from '@optimizely/opt
 
 import { useOptimizelyContext } from './useOptimizelyContext';
 import { useStableArray } from './useStableArray';
-import { createDefaultDecision } from '../utils/helpers';
 
 export interface UseDecideConfig {
   decideOptions?: OptimizelyDecideOption[];
 }
 
-export interface UseDecideResult {
-  decision: OptimizelyDecision;
-  isLoading: boolean;
-  error: Error | null;
-}
+export type UseDecideResult =
+  | { isLoading: true; error: null; decision: null }
+  | { isLoading: false; error: Error; decision: null }
+  | { isLoading: false; error: null; decision: OptimizelyDecision };
 
 /**
  * Returns a feature flag decision for the given flag key.
@@ -70,11 +68,11 @@ export function useDecide(flagKey: string, config?: UseDecideConfig): UseDecideR
     const hasConfig = client.getOptimizelyConfig() !== null;
 
     if (error) {
-      return { decision: createDefaultDecision(flagKey), isLoading: false, error };
+      return { decision: null, isLoading: false, error };
     }
 
     if (!hasConfig || userContext === null) {
-      return { decision: createDefaultDecision(flagKey), isLoading: true, error: null };
+      return { decision: null, isLoading: true, error: null };
     }
 
     const decision = userContext.decide(flagKey, decideOptions);

src/hooks/useOptimizelyUserContext.spec.tsx

@@ -79,35 +79,41 @@ describe('useOptimizelyUserContext', () => {
     consoleSpy.mockRestore();
   });
 
-  it('should return null when no user context is set', () => {
+  it('should return isLoading: true with null userContext when no user context is set', () => {
     const wrapper = createWrapper(store);
     const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
 
-    expect(result.current).toBeNull();
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.error).toBeNull();
+    expect(result.current.userContext).toBeNull();
   });
 
-  it('should return the current user context from the store', () => {
+  it('should return the current user context with isLoading: false', () => {
     const mockUserContext = createMockUserContext();
     store.setUserContext(mockUserContext);
 
     const wrapper = createWrapper(store);
     const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
 
-    expect(result.current).toBe(mockUserContext);
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.error).toBeNull();
+    expect(result.current.userContext).toBe(mockUserContext);
   });
 
   it('should update when user context changes', async () => {
     const wrapper = createWrapper(store);
     const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
 
-    expect(result.current).toBeNull();
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.userContext).toBeNull();
 
     const mockUserContext = createMockUserContext('user-1');
     await act(async () => {
       store.setUserContext(mockUserContext);
     });
 
-    expect(result.current).toBe(mockUserContext);
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.userContext).toBe(mockUserContext);
   });
 
   it('should update when user context changes to a different user', async () => {
@@ -117,30 +123,47 @@ describe('useOptimizelyUserContext', () => {
     const wrapper = createWrapper(store);
     const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
 
-    expect(result.current).toBe(userContext1);
+    expect(result.current.userContext).toBe(userContext1);
 
     const userContext2 = createMockUserContext('user-2');
     await act(async () => {
       store.setUserContext(userContext2);
     });
 
-    expect(result.current).toBe(userContext2);
+    expect(result.current.userContext).toBe(userContext2);
   });
 
-  it('should update to null when store is reset', async () => {
+  it('should return isLoading: true when store is reset', async () => {
     const mockUserContext = createMockUserContext();
     store.setUserContext(mockUserContext);
 
     const wrapper = createWrapper(store);
     const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
 
-    expect(result.current).toBe(mockUserContext);
+    expect(result.current.userContext).toBe(mockUserContext);
 
     await act(async () => {
       store.reset();
     });
 
-    expect(result.current).toBeNull();
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.userContext).toBeNull();
+  });
+
+  it('should return error with isLoading: false when store has error', async () => {
+    const wrapper = createWrapper(store);
+    const { result } = renderHook(() => useOptimizelyUserContext(), { wrapper });
+
+    expect(result.current.isLoading).toBe(true);
+
+    const testError = new Error('SDK initialization failed');
+    await act(async () => {
+      store.setError(testError);
+    });
+
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.error).toBe(testError);
+    expect(result.current.userContext).toBeNull();
   });
 
   it('should unsubscribe from store on unmount', () => {
@@ -164,10 +187,10 @@ describe('useOptimizelyUserContext', () => {
 
     let capturedRenderCount = 0;
     function TestComponent() {
-      const ctx = useOptimizelyUserContext();
+      const { userContext } = useOptimizelyUserContext();
       const renderCount = useRenderCount();
       capturedRenderCount = renderCount;
-      return <div data-testid="user-id">{ctx?.getUserId()}</div>;
+      return <div data-testid="user-id">{userContext?.getUserId()}</div>;
     }
 
     const contextValue: OptimizelyContextValue = {
@@ -184,8 +207,8 @@ describe('useOptimizelyUserContext', () => {
     const initialRenderCount = capturedRenderCount;
 
     // Changing isClientReady triggers a store notification,
-    // but since userContext reference didn't change, React's useState
-    // bails out and skips the re-render
+    // but since the derived result hasn't changed, useMemo returns
+    // the same reference and React bails out
     act(() => {
       store.setClientReady(true);
     });

src/hooks/useOptimizelyUserContext.ts

@@ -14,27 +14,42 @@
  * limitations under the License.
  */
 
-import { useCallback } from 'react';
+import { useCallback, useMemo } from 'react';
 import { useSyncExternalStore } from 'use-sync-external-store/shim';
 import type { OptimizelyUserContext } from '@optimizely/optimizely-sdk';
 
 import { useOptimizelyContext } from './useOptimizelyContext';
 
+export type UseOptimizelyUserContextResult =
+  | { isLoading: true; error: null; userContext: null }
+  | { isLoading: false; error: Error; userContext: null }
+  | { isLoading: false; error: null; userContext: OptimizelyUserContext };
+
 /**
  * Returns the current {@link OptimizelyUserContext} for the nearest `<OptimizelyProvider>`.
  *
  * The user context gives access to the user's identity (user ID and attributes)
  * and methods for working with forced decisions (`setForcedDecision`,
  * `removeForcedDecision`, `removeAllForcedDecisions`).
- *
- * Returns `null` while the SDK is initializing or if no user has been set yet.
  */
-export function useOptimizelyUserContext(): OptimizelyUserContext | null {
+export function useOptimizelyUserContext(): UseOptimizelyUserContextResult {
   const { store } = useOptimizelyContext();
 
   const subscribe = useCallback((onStoreChange: () => void) => store.subscribe(onStoreChange), [store]);
+  const getSnapshot = useCallback(() => store.getState(), [store]);
+  const state = useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
+
+  return useMemo(() => {
+    const { userContext, error } = state;
+
+    if (error) {
+      return { userContext: null, isLoading: false, error };
+    }
 
-  const getSnapshot = useCallback(() => store.getState().userContext, [store]);
+    if (userContext === null) {
+      return { userContext: null, isLoading: true, error: null };
+    }
 
-  return useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
+    return { userContext, isLoading: false, error: null };
+  }, [state]);
 }

src/hooks/useStableArray.ts

@@ -36,8 +36,11 @@ function shallowEqualArrays<T>(a: T[] | undefined, b: T[] | undefined): boolean
   if (a === undefined || b === undefined) return false;
   if (a.length !== b.length) return false;
 
-  for (let i = 0; i < a.length; i++) {
-    if (a[i] !== b[i]) return false;
+  const sortedA = [...a].sort();
+  const sortedB = [...b].sort();
+
+  for (let i = 0; i < sortedA.length; i++) {
+    if (sortedA[i] !== sortedB[i]) return false;
   }
 
   return true;

src/provider/types.ts

@@ -58,7 +58,7 @@ export interface OptimizelyProviderProps {
    * and a background fetch verifies them (unless skipSegments is true).
    * `undefined` = normal flow, `[]` = explicit "zero segments".
    */
-  qualifiedSegments?: string[] | null;
+  qualifiedSegments?: string[];
 
   /**
    * React children to render.

src/utils/UserContextManager.ts

@@ -47,7 +47,7 @@ export class UserContextManager {
   private initialized = false;
   private skipSegments = false;
   private prevUser?: UserInfo;
-  private prevSegments?: string[] | null;
+  private prevSegments?: string[];
 
   constructor(config: UserContextManagerConfig) {
     this.client = config.client;
@@ -66,7 +66,7 @@ export class UserContextManager {
    * @param qualifiedSegments - Optional pre-fetched segments. When provided,
    * @param skipSegments - Whether to skip ODP segment fetching (default: false)
    */
-  resolveUserContext(user?: UserInfo, qualifiedSegments?: string[] | null, skipSegments = false): void {
+  resolveUserContext(user?: UserInfo, qualifiedSegments?: string[], skipSegments = false): void {
     if (
       this.initialized &&
       this.skipSegments === skipSegments &&
@@ -96,11 +96,7 @@ export class UserContextManager {
     this.disposed = true;
   }
 
-  private async createUserContext(
-    requestId: number,
-    user?: UserInfo,
-    qualifiedSegments?: string[] | null
-  ): Promise<void> {
+  private async createUserContext(requestId: number, user?: UserInfo, qualifiedSegments?: string[]): Promise<void> {
     if (!user?.id && this.meta.hasVuidManager) {
       await this.client.onReady();
       if (this.isStale(requestId)) return;
@@ -122,7 +118,7 @@ export class UserContextManager {
         if (this.isStale(requestId)) return;
 
         if (this.client.isOdpIntegrated()) {
-          const snapshot = qualifiedSegments ? [...qualifiedSegments] : null;
+          const snapshot = [...qualifiedSegments];
 
           await ctx.fetchQualifiedSegments();
 

src/utils/helpers.ts

@@ -15,29 +15,13 @@
  */
 
 import type { UserInfo } from '../provider/types';
-import type { OptimizelyDecision, OptimizelyUserContext } from '@optimizely/optimizely-sdk';
-
-/**
- * Creates a default decision to return while loading or when an error occurs.
- */
-export function createDefaultDecision(flagKey: string): OptimizelyDecision {
-  return {
-    variationKey: null,
-    enabled: false,
-    variables: {},
-    ruleKey: null,
-    flagKey,
-    userContext: { id: null, attributes: {} } as unknown as OptimizelyUserContext,
-    reasons: ['Optimizely SDK not configured properly yet.'],
-  };
-}
 
 /**
  * Compares two string arrays for value equality (order-insensitive).
  * Used to prevent redundant user context creation when the segments prop
  * is referentially different but value-equal.
  */
-export function areSegmentsEqual(a?: string[] | null, b?: string[] | null): boolean {
+export function areSegmentsEqual(a?: string[], b?: string[]): boolean {
   if (a === b) return true;
   if (!a || !b) return false;
   if (a.length !== b.length) return false;