Merge pull request #206 from splitio/error_handling_updates

[Breaking change] Error handling updates

Author: EmilianoSanchez

CHANGES.txt

@@ -3,6 +3,7 @@
  - Updated @splitsoftware/splitio package to version 10.29.0 that includes minor updates, and updated some transitive dependencies for vulnerability fixes.
  - Renamed distribution folders from `/lib` to `/cjs` for CommonJS build, and `/es` to `/esm` for EcmaScript Modules build.
  - BREAKING CHANGES:
+      - Updated error handling: using the library modules without wrapping them in a `SplitFactoryProvider` component will now throw an error instead of logging it, as the modules requires the `SplitContext` to work properly.
       - Removed deprecated modules: `SplitFactory` component, `useClient`, `useTreatments` and `useManager` hooks, and `withSplitFactory`, `withSplitClient` and `withSplitTreatments` high-order components. Refer to ./MIGRATION-GUIDE.md for instructions on how to migrate to the new alternatives.
       - Renamed TypeScript interfaces `ISplitFactoryProps` to `ISplitFactoryProviderProps`, and `ISplitFactoryChildProps` to `ISplitFactoryProviderChildProps`.
       - Renamed `SplitSdk` to `SplitFactory` function, which is the underlying Split SDK factory, i.e., `import { SplitFactory } from '@splitsoftware/splitio'`.

src/SplitContext.ts

@@ -1,23 +1,24 @@
 import React from 'react';
 import { ISplitContextValues } from './types';
-import { EXCEPTION_NO_REACT_OR_CREATECONTEXT } from './constants';
-
-if (!React || !React.createContext) throw new Error(EXCEPTION_NO_REACT_OR_CREATECONTEXT);
-
-export const INITIAL_CONTEXT: ISplitContextValues = {
-  client: null,
-  factory: null,
-  isReady: false,
-  isReadyFromCache: false,
-  isTimedout: false,
-  hasTimedout: false,
-  lastUpdate: 0,
-  isDestroyed: false,
-}
+import { EXCEPTION_NO_SFP } from './constants';
 
 /**
  * Split Context is the React Context instance that represents our SplitIO global state.
  * It contains Split SDK objects, such as a factory instance, a client and its status (isReady, isTimedout, lastUpdate)
  * The context is created with default empty values, that SplitFactoryProvider and SplitClient access and update.
  */
-export const SplitContext = React.createContext<ISplitContextValues>(INITIAL_CONTEXT);
+export const SplitContext = React.createContext<ISplitContextValues | undefined>(undefined);
+
+/**
+ * Hook to access the value of `SplitContext`.
+ *
+ * @returns The Split Context object value
+ * @throws Throws an error if the Split Context is not set (i.e. the component is not wrapped in a SplitFactoryProvider)
+ */
+export function useSplitContext() {
+  const context = React.useContext(SplitContext);
+
+  if (!context) throw new Error(EXCEPTION_NO_SFP)
+
+  return context;
+}

src/__tests__/SplitClient.test.tsx

@@ -16,6 +16,7 @@ import { SplitClient } from '../SplitClient';
 import { SplitContext } from '../SplitContext';
 import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
 import { IClientWithContext } from '../utils';
+import { EXCEPTION_NO_SFP } from '../constants';
 
 describe('SplitClient', () => {
 
@@ -254,19 +255,15 @@ describe('SplitClient', () => {
     );
   });
 
-  // @TODO Update test in breaking change, following common practice in React libraries, like React-redux and React-query: use a falsy value as default context value, and throw an error – instead of logging it – if components are not wrapped in a SplitContext.Provider, i.e., if the context is falsy.
-  // test('logs error and passes null client if rendered outside an SplitProvider component.', () => {
-  //   const errorSpy = jest.spyOn(console, 'error');
-  //   render(
-  //     <SplitClient splitKey='user2' >
-  //       {({ client }) => {
-  //         expect(client).toBe(null);
-  //         return null;
-  //       }}
-  //     </SplitClient>
-  //   );
-  //   expect(errorSpy).toBeCalledWith(ERROR_SC_NO_FACTORY);
-  // });
+  test('throws error if invoked outside of SplitFactoryProvider.', () => {
+    expect(() => {
+      render(
+        <SplitClient splitKey='user2' >
+          {() => null}
+        </SplitClient>
+      );
+    }).toThrow(EXCEPTION_NO_SFP);
+  });
 
   test(`passes a new client if re-rendered with a different splitKey.
         Only updates the state if the new client triggers an event, but not the previous one.`, (done) => {

src/__tests__/SplitContext.test.tsx

@@ -1,6 +1,8 @@
 import React from 'react';
 import { render } from '@testing-library/react';
 import { SplitContext } from '../SplitContext';
+import { SplitFactoryProvider } from '../SplitFactoryProvider';
+import { INITIAL_CONTEXT } from './testUtils/utils';
 
 /**
  * Test default SplitContext value
@@ -9,16 +11,22 @@ test('SplitContext.Consumer shows default value', () => {
   render(
     <SplitContext.Consumer>
       {(value) => {
-        expect(value.factory).toBe(null);
-        expect(value.client).toBe(null);
-        expect(value.isReady).toBe(false);
-        expect(value.isReadyFromCache).toBe(false);
-        expect(value.hasTimedout).toBe(false);
-        expect(value.isTimedout).toBe(false);
-        expect(value.isDestroyed).toBe(false);
-        expect(value.lastUpdate).toBe(0);
+        expect(value).toBe(undefined);
         return null;
       }}
     </SplitContext.Consumer>
   );
 });
+
+test('SplitContext.Consumer shows value when wrapped in a SplitFactoryProvider', () => {
+  render(
+    <SplitFactoryProvider >
+      <SplitContext.Consumer>
+        {(value) => {
+          expect(value).toEqual(INITIAL_CONTEXT);
+          return null;
+        }}
+      </SplitContext.Consumer>
+    </SplitFactoryProvider>
+  );
+});

src/__tests__/SplitFactoryProvider.test.tsx

@@ -14,9 +14,10 @@ const logSpy = jest.spyOn(console, 'log');
 import { ISplitFactoryProviderChildProps } from '../types';
 import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { SplitClient } from '../SplitClient';
-import { INITIAL_CONTEXT, SplitContext } from '../SplitContext';
+import { SplitContext } from '../SplitContext';
 import { __factories, IClientWithContext } from '../utils';
 import { WARN_SF_CONFIG_AND_FACTORY } from '../constants';
+import { INITIAL_CONTEXT } from './testUtils/utils';
 
 describe('SplitFactoryProvider', () => {
 

src/__tests__/SplitTreatments.test.tsx

@@ -10,7 +10,7 @@ import { SplitFactory } from '@splitsoftware/splitio/client';
 import { sdkBrowser } from './testUtils/sdkConfigs';
 import { getStatus, IClientWithContext } from '../utils';
 import { newSplitFactoryLocalhostInstance } from './testUtils/utils';
-import { CONTROL_WITH_CONFIG } from '../constants';
+import { CONTROL_WITH_CONFIG, EXCEPTION_NO_SFP } from '../constants';
 
 /** Test target */
 import { ISplitTreatmentsChildProps, ISplitTreatmentsProps, ISplitClientProps } from '../types';
@@ -90,19 +90,15 @@ describe('SplitTreatments', () => {
     );
   });
 
-  // @TODO Update test in breaking change, following common practice in React libraries, like React-redux and React-query: use a falsy value as default context value, and throw an error – instead of logging it – if components are not wrapped in a SplitContext.Provider, i.e., if the context is falsy.
-  // it('logs error and passes control treatments if rendered outside an SplitProvider component.', () => {
-  //   render(
-  //     <SplitTreatments names={featureFlagNames} >
-  //       {({ treatments }: ISplitTreatmentsChildProps) => {
-  //         expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG });
-  //         return null;
-  //       }}
-  //     </SplitTreatments>
-  //   );
-
-  //   expect(logSpy).toBeCalledWith(WARN_ST_NO_CLIENT);
-  // });
+  test('throws error if invoked outside of SplitFactoryProvider.', () => {
+    expect(() => {
+      render(
+        <SplitTreatments names={featureFlagNames} >
+          {() => null}
+        </SplitTreatments>
+      );
+    }).toThrow(EXCEPTION_NO_SFP);
+  });
 
   /**
    * Input validation. Passing invalid feature flag names or attributes while the Sdk
@@ -149,13 +145,15 @@ describe('SplitTreatments', () => {
 
   test('ignores flagSets and logs a warning if both names and flagSets params are provided.', () => {
     render(
-      // @ts-expect-error flagSets and names are mutually exclusive
-      <SplitTreatments names={featureFlagNames} flagSets={flagSets} >
-        {({ treatments }) => {
-          expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG });
-          return null;
-        }}
-      </SplitTreatments>
+      <SplitFactoryProvider >
+        {/* @ts-expect-error flagSets and names are mutually exclusive */}
+        <SplitTreatments names={featureFlagNames} flagSets={flagSets} >
+          {({ treatments }) => {
+            expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG });
+            return null;
+          }}
+        </SplitTreatments>
+      </SplitFactoryProvider>
     );
 
     expect(logSpy).toBeCalledWith('[WARN]  Both names and flagSets properties were provided. flagSets will be ignored.');

src/__tests__/testUtils/utils.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import { render } from '@testing-library/react';
+import { ISplitContextValues } from '../../types';
 const { SplitFactory: originalSplitFactory } = jest.requireActual('@splitsoftware/splitio/client');
 
 export interface TestComponentProps {
@@ -114,3 +115,14 @@ export function testAttributesBinding(Component: React.FunctionComponent<TestCom
   wrapper.rerender(<Component splitKey={undefined} attributesFactory={{ at4: 'at4' }} attributesClient={undefined} testSwitch={attributesBindingSwitch} factory={factory} />);
   wrapper.rerender(<Component splitKey={undefined} attributesFactory={undefined} attributesClient={undefined} testSwitch={attributesBindingSwitch} factory={factory} />);
 }
+
+export const INITIAL_CONTEXT: ISplitContextValues = {
+  client: null,
+  factory: null,
+  isReady: false,
+  isReadyFromCache: false,
+  isTimedout: false,
+  hasTimedout: false,
+  lastUpdate: 0,
+  isDestroyed: false,
+}

src/__tests__/useSplitClient.test.tsx

@@ -15,6 +15,7 @@ import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { SplitClient } from '../SplitClient';
 import { SplitContext } from '../SplitContext';
 import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
+import { EXCEPTION_NO_SFP } from '../constants';
 
 describe('useSplitClient', () => {
 
@@ -64,18 +65,16 @@ describe('useSplitClient', () => {
     expect(outerFactory.client as jest.Mock).toHaveReturnedWith(client);
   });
 
-  test('returns null if invoked outside Split context.', () => {
-    let client;
-    let sharedClient;
-    render(
-      React.createElement(() => {
-        client = useSplitClient().client;
-        sharedClient = useSplitClient({ splitKey: 'user2', trafficType: 'user' }).client;
-        return null;
-      })
-    );
-    expect(client).toBe(null);
-    expect(sharedClient).toBe(null);
+  test('throws error if invoked outside of SplitFactoryProvider.', () => {
+    expect(() => {
+      render(
+        React.createElement(() => {
+          useSplitClient();
+          useSplitClient({ splitKey: 'user2', trafficType: 'user' });
+          return null;
+        })
+      );
+    }).toThrow(EXCEPTION_NO_SFP);
   });
 
   test('attributes binding test with utility', (done) => {
@@ -223,7 +222,7 @@ describe('useSplitClient', () => {
   });
 
   // Remove this test once side effects are moved to the useSplitClient effect.
-  test('must update on SDK events between the render phase (hook call) and commit phase (effect call)', () =>{
+  test('must update on SDK events between the render phase (hook call) and commit phase (effect call)', () => {
     const outerFactory = SplitFactory(sdkBrowser);
     let count = 0;
 

src/__tests__/useSplitManager.test.tsx

@@ -13,6 +13,7 @@ import { getStatus } from '../utils';
 /** Test target */
 import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { useSplitManager } from '../useSplitManager';
+import { EXCEPTION_NO_SFP } from '../constants';
 
 describe('useSplitManager', () => {
 
@@ -55,26 +56,15 @@ describe('useSplitManager', () => {
     });
   });
 
-  test('returns null if invoked outside Split context.', () => {
-    let hookResult;
-    render(
-      React.createElement(() => {
-        hookResult = useSplitManager();
-        return null;
-      })
-    );
-
-    expect(hookResult).toStrictEqual({
-      manager: null,
-      client: null,
-      factory: null,
-      hasTimedout: false,
-      isDestroyed: false,
-      isReady: false,
-      isReadyFromCache: false,
-      isTimedout: false,
-      lastUpdate: 0,
-    });
+  test('throws error if invoked outside of SplitFactoryProvider.', () => {
+    expect(() => {
+      render(
+        React.createElement(() => {
+          useSplitManager();
+          return null;
+        })
+      );
+    }).toThrow(EXCEPTION_NO_SFP);
   });
 
 });