From e8e0d2aabdcd0761ff5ddec53e6917dbc5e999ae Mon Sep 17 00:00:00 2001
From: Emiliano Sanchez <emiliano.sanchez@split.io>
Date: Mon, 21 Oct 2024 14:17:56 -0300
Subject: [PATCH] Refactor context value to use undefined rather than null for
 factory and client properties

---
 src/SplitFactoryProvider.tsx                |  6 +--
 src/__tests__/SplitClient.test.tsx          | 43 ++++++++++-----------
 src/__tests__/SplitFactoryProvider.test.tsx | 38 ++++++++----------
 src/__tests__/testUtils/utils.tsx           |  4 +-
 src/types.ts                                |  4 +-
 src/useSplitManager.ts                      |  6 +--
 src/utils.ts                                |  6 +--
 7 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx
index 9e370d1..1046d48 100644
--- a/src/SplitFactoryProvider.tsx
+++ b/src/SplitFactoryProvider.tsx
@@ -28,11 +28,11 @@ export function SplitFactoryProvider(props: ISplitFactoryProviderProps) {
     config = undefined;
   }
 
-  const [configFactory, setConfigFactory] = React.useState<IFactoryWithClients | null>(null);
+  const [configFactory, setConfigFactory] = React.useState<IFactoryWithClients>();
   const factory = React.useMemo(() => {
-    return propFactory || (configFactory && config === configFactory.config ? configFactory : null);
+    return propFactory || (configFactory && config === configFactory.config ? configFactory : undefined);
   }, [config, propFactory, configFactory]);
-  const client = factory ? getSplitClient(factory) : null;
+  const client = factory ? getSplitClient(factory) : undefined;
 
   // Effect to initialize and destroy the factory
   React.useEffect(() => {
diff --git a/src/__tests__/SplitClient.test.tsx b/src/__tests__/SplitClient.test.tsx
index 94b95cc..9675710 100644
--- a/src/__tests__/SplitClient.test.tsx
+++ b/src/__tests__/SplitClient.test.tsx
@@ -14,7 +14,7 @@ import { ISplitClientChildProps } from '../types';
 import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { SplitClient } from '../SplitClient';
 import { SplitContext } from '../SplitContext';
-import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
+import { INITIAL_CONTEXT, testAttributesBinding, TestComponentProps } from './testUtils/utils';
 import { IClientWithContext } from '../utils';
 import { EXCEPTION_NO_SFP } from '../constants';
 
@@ -24,13 +24,8 @@ describe('SplitClient', () => {
     render(
       <SplitFactoryProvider config={sdkBrowser} >
         <SplitClient splitKey='user1' >
-          {({ isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => {
-            expect(isReady).toBe(false);
-            expect(isReadyFromCache).toBe(false);
-            expect(hasTimedout).toBe(false);
-            expect(isTimedout).toBe(false);
-            expect(isDestroyed).toBe(false);
-            expect(lastUpdate).toBe(0);
+          {(childProps: ISplitClientChildProps) => {
+            expect(childProps).toEqual(INITIAL_CONTEXT);
 
             return null;
           }}
@@ -50,14 +45,15 @@ describe('SplitClient', () => {
       <SplitFactoryProvider factory={outerFactory} >
         {/* Equivalent to <SplitClient splitKey={undefined} > */}
         <SplitClient splitKey={sdkBrowser.core.key} >
-          {({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => {
-            expect(client).toBe(outerFactory.client());
-            expect(isReady).toBe(true);
-            expect(isReadyFromCache).toBe(true);
-            expect(hasTimedout).toBe(false);
-            expect(isTimedout).toBe(false);
-            expect(isDestroyed).toBe(false);
-            expect(lastUpdate).toBe((outerFactory.client() as IClientWithContext).__getStatus().lastUpdate);
+          {(childProps: ISplitClientChildProps) => {
+            expect(childProps).toEqual({
+              ...INITIAL_CONTEXT,
+              factory : outerFactory,
+              client: outerFactory.client(),
+              isReady: true,
+              isReadyFromCache: true,
+              lastUpdate: (outerFactory.client() as IClientWithContext).__getStatus().lastUpdate
+            });
 
             return null;
           }}
@@ -162,7 +158,7 @@ describe('SplitClient', () => {
     expect(renderTimes).toBe(3);
   });
 
-  test('rerender child only on SDK_READY event, as default behaviour.', async () => {
+  test('rerender child only on SDK_READY event, as default behavior.', async () => {
     const outerFactory = SplitFactory(sdkBrowser);
     (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
     (outerFactory.manager().names as jest.Mock).mockReturnValue(['split1']);
@@ -228,18 +224,19 @@ describe('SplitClient', () => {
     expect(count).toEqual(2);
   });
 
-  test('renders a passed JSX.Element with a new SplitContext value.', (done) => {
+  test('renders a passed JSX.Element with a new SplitContext value.', () => {
     const outerFactory = SplitFactory(sdkBrowser);
 
     const Component = () => {
       return (
         <SplitContext.Consumer>
           {(value) => {
-            expect(value.client).toBe(outerFactory.client('user2'));
-            expect(value.isReady).toBe(false);
-            expect(value.isTimedout).toBe(false);
-            expect(value.lastUpdate).toBe(0);
-            done();
+            expect(value).toEqual({
+              ...INITIAL_CONTEXT,
+              factory: outerFactory,
+              client: outerFactory.client('user2'),
+            });
+
             return null;
           }}
         </SplitContext.Consumer>
diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx
index 5efddd4..f2c5039 100644
--- a/src/__tests__/SplitFactoryProvider.test.tsx
+++ b/src/__tests__/SplitFactoryProvider.test.tsx
@@ -24,15 +24,8 @@ describe('SplitFactoryProvider', () => {
   test('passes no-ready props to the child if initialized with a config.', () => {
     render(
       <SplitFactoryProvider config={sdkBrowser} >
-        {({ factory, client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryProviderChildProps) => {
-          expect(factory).toBe(null);
-          expect(client).toBe(null);
-          expect(isReady).toBe(false);
-          expect(isReadyFromCache).toBe(false);
-          expect(hasTimedout).toBe(false);
-          expect(isTimedout).toBe(false);
-          expect(isDestroyed).toBe(false);
-          expect(lastUpdate).toBe(0);
+        {(childProps: ISplitFactoryProviderChildProps) => {
+          expect(childProps).toEqual(INITIAL_CONTEXT);
           return null;
         }}
       </SplitFactoryProvider>
@@ -48,15 +41,16 @@ describe('SplitFactoryProvider', () => {
 
     render(
       <SplitFactoryProvider factory={outerFactory} >
-        {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryProviderChildProps) => {
-          expect(factory).toBe(outerFactory);
-          expect(isReady).toBe(true);
-          expect(isReadyFromCache).toBe(true);
-          expect(hasTimedout).toBe(false);
-          expect(isTimedout).toBe(false);
-          expect(isDestroyed).toBe(false);
-          expect(lastUpdate).toBe((outerFactory.client() as IClientWithContext).__getStatus().lastUpdate);
-          expect((factory as SplitIO.IBrowserSDK).settings.version).toBe(outerFactory.settings.version);
+        {(childProps: ISplitFactoryProviderChildProps) => {
+          expect(childProps).toEqual({
+            ...INITIAL_CONTEXT,
+            factory: outerFactory,
+            client: outerFactory.client(),
+            isReady: true,
+            isReadyFromCache: true,
+            lastUpdate: (outerFactory.client() as IClientWithContext).__getStatus().lastUpdate
+          });
+          expect((childProps.factory as SplitIO.IBrowserSDK).settings.version).toBe(outerFactory.settings.version);
           return null;
         }}
       </SplitFactoryProvider>
@@ -229,7 +223,7 @@ describe('SplitFactoryProvider', () => {
     expect(renderTimes).toBe(3);
   });
 
-  test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (config prop)', async () => {
+  test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behavior (config prop)', async () => {
     let renderTimes = 0;
     let previousLastUpdate = -1;
 
@@ -263,7 +257,7 @@ describe('SplitFactoryProvider', () => {
     expect(renderTimes).toBe(2);
   });
 
-  test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (factory prop)', async () => {
+  test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behavior (factory prop)', async () => {
     const outerFactory = SplitFactory(sdkBrowser);
     let renderTimes = 0;
     let previousLastUpdate = -1;
@@ -350,14 +344,14 @@ describe('SplitFactoryProvider', () => {
         case 5:
           expect(isReady).toBe(false);
           expect(hasTimedout).toBe(false);
-          expect(factory).toBe(null);
+          expect(factory).toBe(undefined);
           return null;
         case 3:
         case 4:
         case 6:
           expect(isReady).toBe(true);
           expect(hasTimedout).toBe(true);
-          expect(factory).not.toBe(null);
+          expect(factory).not.toBe(undefined);
           createdFactories.add(factory!);
           clientDestroySpies.push(jest.spyOn(factory!.client(), 'destroy'));
           return (
diff --git a/src/__tests__/testUtils/utils.tsx b/src/__tests__/testUtils/utils.tsx
index d19143a..6324bde 100644
--- a/src/__tests__/testUtils/utils.tsx
+++ b/src/__tests__/testUtils/utils.tsx
@@ -117,8 +117,8 @@ export function testAttributesBinding(Component: React.FunctionComponent<TestCom
 }
 
 export const INITIAL_CONTEXT: ISplitContextValues = {
-  client: null,
-  factory: null,
+  client: undefined,
+  factory: undefined,
   isReady: false,
   isReadyFromCache: false,
   isTimedout: false,
diff --git a/src/types.ts b/src/types.ts
index 88d7006..42db265 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -50,7 +50,7 @@ export interface ISplitContextValues extends ISplitStatus {
    *
    * NOTE: This property is not recommended for direct use, as better alternatives are available.
    */
-  factory: SplitIO.IBrowserSDK | null;
+  factory?: SplitIO.IBrowserSDK;
 
   /**
    * Split client instance.
@@ -59,7 +59,7 @@ export interface ISplitContextValues extends ISplitStatus {
    *
    * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#2-instantiate-the-sdk-and-create-a-new-split-client}
    */
-  client: SplitIO.IBrowserClient | null;
+  client?: SplitIO.IBrowserClient;
 }
 
 /**
diff --git a/src/useSplitManager.ts b/src/useSplitManager.ts
index d2d7270..36c73eb 100644
--- a/src/useSplitManager.ts
+++ b/src/useSplitManager.ts
@@ -5,7 +5,7 @@ import { ISplitContextValues } from './types';
  * 'useSplitManager' is a hook that returns an Split Context object with the Manager instance from the Split factory.
  * It uses the 'useContext' hook to access the factory at Split context, which is updated by the SplitFactoryProvider component.
  *
- * @returns An object containing the Split context and the Split Manager instance, which is null if used outside the scope of SplitFactoryProvider or factory is not ready.
+ * @returns An object containing the Split context and the Split Manager instance, which is undefined if factory is not ready.
  *
  * @example
  * ```js
@@ -14,11 +14,11 @@ import { ISplitContextValues } from './types';
  *
  * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#manager}
  */
-export function useSplitManager(): ISplitContextValues & { manager: SplitIO.IManager | null } {
+export function useSplitManager(): ISplitContextValues & { manager?: SplitIO.IManager } {
   // Update options are not supported, because updates can be controlled at the SplitFactoryProvider component.
   const context = useSplitContext();
   return {
     ...context,
-    manager: context.factory ? context.factory.manager() : null
+    manager: context.factory ? context.factory.manager() : undefined
   };
 }
diff --git a/src/utils.ts b/src/utils.ts
index 9ae5a3c..96e9ab7 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -64,7 +64,7 @@ export function destroySplitFactory(factory: IFactoryWithClients): Promise<void>
 
 // Util used to get client status.
 // It might be removed in the future, if the JS SDK extends its public API with a `getStatus` method
-export function getStatus(client: SplitIO.IBrowserClient | null): ISplitStatus {
+export function getStatus(client?: SplitIO.IBrowserClient): ISplitStatus {
   const status = client && (client as IClientWithContext).__getStatus();
 
   return {
@@ -80,7 +80,7 @@ export function getStatus(client: SplitIO.IBrowserClient | null): ISplitStatus {
 /**
  * Manage client attributes binding
  */
-export function initAttributes(client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes) {
+export function initAttributes(client?: SplitIO.IBrowserClient, attributes?: SplitIO.Attributes) {
   if (client && attributes) client.setAttributes(attributes);
 }
 
@@ -174,7 +174,7 @@ function argsAreEqual(newArgs: any[], lastArgs: any[]): boolean {
     shallowEqual(newArgs[5], lastArgs[5]); // flagSets
 }
 
-function evaluateFeatureFlags(client: SplitIO.IBrowserClient | null, _lastUpdate: number, names?: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes, flagSets?: string[]) {
+function evaluateFeatureFlags(client: SplitIO.IBrowserClient | undefined, _lastUpdate: number, names?: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes, flagSets?: string[]) {
   if (names && flagSets) console.log(WARN_NAMES_AND_FLAGSETS);
 
   return client && (client as IClientWithContext).__getStatus().isOperational && (names || flagSets) ?