Skip to content

[FSSDK-12296] Async Hook implementation#324

Open
junaed-optimizely wants to merge 4 commits intomasterfrom
junaed/fssdk-12296-async-hook-impl
Open

[FSSDK-12296] Async Hook implementation#324
junaed-optimizely wants to merge 4 commits intomasterfrom
junaed/fssdk-12296-async-hook-impl

Conversation

@junaed-optimizely
Copy link
Copy Markdown
Collaborator

Summary

  • Add useDecideAsync, useDecideForKeysAsync, and useDecideAllAsync hooks — async counterparts to the existing sync decide hooks, required for CMAB, async UPS etc.

Test plan

  • Add tests for all 3 new async hooks covering store states, race conditions, re-evaluation triggers, forced decision reactivity, and render optimization

Issues

  • FSSDK-12296

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds async counterparts to the existing “decide” React hooks so consumers can evaluate flags via the Optimizely SDK’s async decide APIs (needed for CMAB / async decisioning) while handling loading/error states, cancellation, and forced-decision reactivity.

Changes:

  • Introduces useDecideAsync, useDecideForKeysAsync, and useDecideAllAsync and exports them from the public entrypoints.
  • Adds shared useAsyncDecision state machine to manage async execution, cancellation of stale promises, and store readiness transitions.
  • Adds comprehensive Vitest coverage for the 3 new hooks and extends hook test utilities with async decide method stubs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/index.ts Exports the new async hooks from the package entrypoint.
src/hooks/index.ts Exposes the new async hooks (and related types) from the hooks barrel.
src/hooks/useAsyncDecision.ts Adds shared async state machine used by all async decide hooks.
src/hooks/useDecideAsync.ts Implements async single-flag decision hook w/ forced decision subscription.
src/hooks/useDecideForKeysAsync.ts Implements async multi-key decision hook w/ per-key forced decision subscription.
src/hooks/useDecideAllAsync.ts Implements async all-flags decision hook w/ global forced decision subscription.
src/hooks/*.spec.tsx Adds tests covering readiness states, cancellation, re-evaluation triggers, and forced-decision reactivity.
src/hooks/testUtils.tsx Extends mock user context to support async decide APIs for testing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


// Store not ready — stay in loading
if (!hasConfig || userContext === null) {
setAsyncState({ result: emptyResult, error: null, isLoading: true });
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useAsyncDecision calls setAsyncState({ result: emptyResult, error: null, isLoading: true }) whenever the provider is not ready (!hasConfig || userContext === null). On initial mount (and on any store refresh while still not ready), this will schedule a state update even when the hook is already in the exact same loading state, causing an avoidable extra render. Consider using a functional setAsyncState with a guard (similar to the ready-path guard) so it returns prev when {isLoading:true,error:null,result===emptyResult} is already true.

Suggested change
setAsyncState({ result: emptyResult, error: null, isLoading: true });
setAsyncState((prev) => {
if (prev.isLoading && prev.error === null && prev.result === emptyResult) return prev;
return { result: emptyResult, error: null, isLoading: true };
});

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +53
interface AsyncState<TResult> {
result: TResult;
error: Error | null;
isLoading: boolean;
}

/**
* Shared async decision state machine used by useDecideAsync,
* useDecideForKeysAsync, and useDecideAllAsync.
*
* Handles: loading state, error propagation, cancellation of stale promises,
* and redundant re-render avoidance on first mount.
*
* @param state - Provider state from useProviderState
* @param client - Optimizely client instance
* @param fdVersion - Forced decision version counter (triggers re-evaluation)
* @param emptyResult - Default/empty result value (null for single, {} for multi)
* @param execute - Callback that performs the async SDK call
*/
export function useAsyncDecision<TResult>(
state: ProviderState,
client: Client,
fdVersion: number,
emptyResult: TResult,
execute: (userContext: OptimizelyUserContext) => Promise<TResult>
): AsyncState<TResult> {
const [asyncState, setAsyncState] = useState<AsyncState<TResult>>({
result: emptyResult,
error: null,
isLoading: true,
});
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The async hooks rely on as UseDecide*AsyncResult type assertions when returning { result, error, isLoading }, which can mask violations of the intended discriminated union (e.g., accidentally returning isLoading: true with a non-null error after future refactors). Consider making useAsyncDecision return a discriminated union (or a helper that maps its internal state into the union) so TypeScript enforces the invariants and the hooks can return without assertions.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants