chore (Hooks): Move common client ready logic to a separate function (#34)

## Summary

In preparation to introduce a `useExperiment` hook, this PR moves the logic for waiting for the client to become ready, setting initial state, and configuring autoupdate to a shared function, which is passed to `useEffect`

There are a lot of variables that need to be passed, but I think this is much preferable to duplicating the somewhat intricate & nuanced logic that exists there.

## Test Plan

Existing unit tests cover this functionality, and more will be added with the introduction of `useExperiment`

Author: James Fox

src/autoUpdate.ts

@@ -21,7 +21,7 @@ import { ReactSDKClient } from './client';
 interface AutoUpdate {
   (
     optimizely: ReactSDKClient,
-    type: 'feature' | 'experiment',
+    type: 'Feature' | 'Experiment',
     value: string,
     logger: LoggerFacade,
     callback: () => void
@@ -41,15 +41,15 @@ export const setupAutoUpdateListeners: AutoUpdate = (optimizely, type, value, lo
       callback();
     }
   );
-  const unregisterConfigUpdateListener = () =>
+  const unregisterConfigUpdateListener: () => void = () =>
     optimizely.notificationCenter.removeNotificationListener(optimizelyNotificationId);
 
   const unregisterUserListener = optimizely.onUserUpdate(() => {
     logger.info(`User update, ${loggerSuffix}`);
     callback();
   });
 
-  return () => {
+  return (): void => {
     unregisterConfigUpdateListener();
     unregisterUserListener();
   };

src/hooks.ts

@@ -13,36 +13,60 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { useCallback, useContext, useEffect, useState } from 'react';
+import { Dispatch, EffectCallback, SetStateAction, useCallback, useContext, useEffect, useState } from 'react';
+
 import { UserAttributes } from '@optimizely/optimizely-sdk';
-import { getLogger } from '@optimizely/js-sdk-logging';
+import { getLogger, LoggerFacade } from '@optimizely/js-sdk-logging';
 
 import { setupAutoUpdateListeners } from './autoUpdate';
-import { VariableValuesObject, OnReadyResult } from './client';
+import { ReactSDKClient, VariableValuesObject, OnReadyResult } from './client';
 import { OptimizelyContext } from './Context';
 
-const useFeatureLogger = getLogger('useFeature');
-
-type UseFeatureState = {
-  isEnabled: boolean;
-  variables: VariableValuesObject;
-};
-
-type ClientReady = boolean;
-type DidTimeout = boolean;
+enum HookType {
+  EXPERIMENT = 'Experiment',
+  FEATURE = 'Feature',
+}
 
-type UseFeatureOptions = {
+type HookOptions = {
   autoUpdate?: boolean;
   timeout?: number;
 };
 
-type UseFeatureOverrides = {
+type HookOverrides = {
   overrideUserId?: string;
   overrideAttributes?: UserAttributes;
 };
 
+type ClientReady = boolean;
+
+type DidTimeout = boolean;
+
+interface HookStateBase {
+  clientReady: ClientReady;
+  didTimeout: DidTimeout;
+}
+
+// TODO - Get these from the core SDK once it's typed
+interface ExperimentDecisionValues {
+  variation: string | null;
+}
+
+// TODO - Get these from the core SDK once it's typed
+interface FeatureDecisionValues {
+  isEnabled: boolean;
+  variables: VariableValuesObject;
+}
+
+interface UseExperimentState extends HookStateBase, ExperimentDecisionValues {}
+
+interface UseFeatureState extends HookStateBase, FeatureDecisionValues {}
+
+type HookState = UseExperimentState | UseFeatureState;
+
+type CurrentDecisionValues = ExperimentDecisionValues | FeatureDecisionValues;
+
 interface UseFeature {
-  (featureKey: string, options?: UseFeatureOptions, overrides?: UseFeatureOverrides): [
+  (featureKey: string, options?: HookOptions, overrides?: HookOverrides): [
     UseFeatureState['isEnabled'],
     UseFeatureState['variables'],
     ClientReady,
@@ -51,84 +75,104 @@ interface UseFeature {
 }
 
 /**
- * A React Hook that retrieves the status of a feature flag and its variables, optionally
- * auto updating those values based on underlying user or datafile changes.
- *
- * Note: The react client can become ready AFTER the timeout period.
- *       ClientReady and DidTimeout provide signals to handle this scenario.
+ * A function which waits for the optimizely client instance passed to become
+ * ready and then sets up initial state and (optionally) autoUpdate listeners
+ * for the hook type specified.
  */
-export const useFeature: UseFeature = (featureKey, options = {}, overrides = {}) => {
-  const { isServerSide, optimizely, timeout } = useContext(OptimizelyContext);
-  if (!optimizely) {
-    throw new Error('optimizely prop must be supplied via a parent <OptimizelyProvider>');
-  }
-  const finalReadyTimeout: number | undefined = options.timeout !== undefined ? options.timeout : timeout;
-
-  // Helper function to return the current values for isEnabled and variables.
-  const getCurrentValues = useCallback(
-    () => ({
-      isEnabled: optimizely.isFeatureEnabled(featureKey, overrides.overrideUserId, overrides.overrideAttributes),
-      variables: optimizely.getFeatureVariables(featureKey, overrides.overrideUserId, overrides.overrideAttributes),
-    }),
-    [featureKey, overrides]
-  );
-
-  // Set the initial state immediately serverSide
-  const [data, setData] = useState<UseFeatureState>(() => {
-    if (isServerSide) {
-      return getCurrentValues();
-    }
-    return { isEnabled: false, variables: {} };
-  });
-
-  const [clientReady, setClientReady] = useState(isServerSide ? true : false);
-  const [didTimeout, setDidTimeout] = useState(false);
-
-  useEffect(() => {
+const initializeWhenClientReadyFn = (
+  type: HookType,
+  name: string,
+  optimizely: ReactSDKClient,
+  options: HookOptions,
+  timeout: number | undefined,
+  setState: Dispatch<SetStateAction<HookState>>,
+  getCurrentDecisionValues: () => CurrentDecisionValues
+): EffectCallback => {
+  return (): (() => void) => {
     const cleanupFns: Array<() => void> = [];
+    const finalReadyTimeout: number | undefined = options.timeout !== undefined ? options.timeout : timeout;
+    const logger: LoggerFacade = getLogger(`use${type}`);
 
     optimizely
       .onReady({ timeout: finalReadyTimeout })
       .then((res: OnReadyResult) => {
         if (res.success) {
           // didTimeout=false
-          useFeatureLogger.info(`feature="${featureKey}" successfully set for user="${optimizely.user.id}"`);
+          logger.info(`${type}="${name}" successfully set for user="${optimizely.user.id}"`);
           return;
         }
-        setDidTimeout(true);
-        useFeatureLogger.info(
-          `feature="${featureKey}" could not be set before timeout of ${finalReadyTimeout}ms, reason="${res.reason ||
-            ''}"`
-        );
+        setState((state: HookState) => ({ ...state, didTimeout: true }));
+        logger.info(`${type}="${name}" could not be set before timeout of ${timeout}ms, reason="${res.reason || ''}"`);
         // Since we timed out, wait for the dataReadyPromise to resolve before setting up.
         return res.dataReadyPromise!.then(() => {
-          useFeatureLogger.info(`feature="${featureKey}" is now set, but after timeout.`);
+          logger.info(`${type}="${name}" is now set, but after timeout.`);
         });
       })
       .then(() => {
-        setClientReady(true);
-        setData(getCurrentValues());
+        setState((state: HookState) => ({ ...state, ...getCurrentDecisionValues(), clientReady: true }));
         if (options.autoUpdate) {
           cleanupFns.push(
-            setupAutoUpdateListeners(optimizely, 'feature', featureKey, useFeatureLogger, () => {
+            setupAutoUpdateListeners(optimizely, type, name, logger, () => {
               if (cleanupFns.length) {
-                setData(getCurrentValues());
+                setState((state: HookState) => ({ ...state, ...getCurrentDecisionValues() }));
               }
             })
           );
         }
       })
       .catch(() => {
         /* The user promise or core client promise rejected. */
-        useFeatureLogger.error(`Error initializing client. The core client or user promise(s) rejected.`);
+        logger.error(`Error initializing client. The core client or user promise(s) rejected.`);
       });
 
-    return () => {
+    return (): void => {
       while (cleanupFns.length) {
         cleanupFns.shift()!();
       }
     };
-  }, [optimizely]);
+  };
+};
+
+/**
+ * A React Hook that retrieves the status of a feature flag and its variables, optionally
+ * auto updating those values based on underlying user or datafile changes.
+ *
+ * Note: The react client can become ready AFTER the timeout period.
+ *       ClientReady and DidTimeout provide signals to handle this scenario.
+ */
+export const useFeature: UseFeature = (featureKey, options = {}, overrides = {}) => {
+  const { isServerSide, optimizely, timeout } = useContext(OptimizelyContext);
+  if (!optimizely) {
+    throw new Error('optimizely prop must be supplied via a parent <OptimizelyProvider>');
+  }
+
+  // Helper function to return the current values for isEnabled and variables.
+  const getCurrentValues = useCallback<() => FeatureDecisionValues>(
+    () => ({
+      isEnabled: optimizely.isFeatureEnabled(featureKey, overrides.overrideUserId, overrides.overrideAttributes),
+      variables: optimizely.getFeatureVariables(featureKey, overrides.overrideUserId, overrides.overrideAttributes),
+    }),
+    [featureKey, overrides]
+  );
+
+  // Set the initial state immediately serverSide
+  const [state, setState] = useState<UseFeatureState>(() => {
+    const initialState = {
+      isEnabled: false,
+      variables: {},
+      clientReady: isServerSide ? true : false,
+      didTimeout: false,
+    };
+    if (isServerSide) {
+      return { ...initialState, ...getCurrentValues() };
+    }
+    return initialState;
+  });
+
+  useEffect(
+    initializeWhenClientReadyFn(HookType.FEATURE, featureKey, optimizely, options, timeout, setState, getCurrentValues),
+    [optimizely]
+  );
 
-  return [data.isEnabled, data.variables, clientReady, didTimeout];
+  return [state.isEnabled, state.variables, state.clientReady, state.didTimeout];
 };