Merge pull request #181 from splitio/add_SplitFactoryProvider

Polishing: replace `SplitFactory` for `SplitFactoryProvider` in comments and tests

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,9 @@
+1.11.0 (January 15, 2023)
+ - Added new `SplitFactoryProvider` component as replacement for the now deprecated `SplitFactory` component.
+ This new component is a fixed version of the `SplitFactory` component, which is not handling the SDK initialization side-effects in the `componentDidMount` and `componentDidUpdate` methods (commit phase), causing some issues like the SDK not being reinitialized when component props change (Related to issue #11 and #148).
+ The new component also supports server-side rendering. See our documentation for more details: https://help.split.io/hc/en-us/articles/360038825091-React-SDK#server-side-rendering (Related to issue #11 and #109).
+ - Updated internal code to remove a circular dependency and avoid warning messages with tools like PNPM (Related to issue #176).
+
 1.10.2 (December 12, 2023)
  - Updated @splitsoftware/splitio package to version 10.24.1 that updates localStorage usage to clear cached feature flag definitions before initiating the synchronization process, if the cache was previously synchronized with a different SDK key (i.e., a different environment) or different Split Filter criteria, to avoid using invalid cached data when the SDK is ready from cache.
 

src/SplitContext.ts

@@ -18,6 +18,6 @@ export const INITIAL_CONTEXT: ISplitContextValues = {
 /**
  * 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 eventually SplitFactory and SplitClient access and update.
+ * The context is created with default empty values, that SplitFactoryProvider and SplitClient access and update.
  */
 export const SplitContext = React.createContext<ISplitContextValues>(INITIAL_CONTEXT);

src/SplitFactoryProvider.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 import { SplitComponent } from './SplitClient';
 import { ISplitFactoryProps } from './types';
 import { WARN_SF_CONFIG_AND_FACTORY } from './constants';
-import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient, getStatus } from './utils';
+import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient, getStatus, __factories } from './utils';
 import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient';
 
 /**
@@ -27,24 +27,39 @@ export function SplitFactoryProvider(props: ISplitFactoryProps) {
     config = undefined;
   }
 
-  const [stateFactory, setStateFactory] = React.useState(propFactory || null);
-  const factory = propFactory || stateFactory;
+  const [configFactory, setConfigFactory] = React.useState<IFactoryWithClients | null>(null);
+  const factory = propFactory || (configFactory && config === configFactory.config ? configFactory : null);
   const client = factory ? getSplitClient(factory) : null;
 
+  // Effect to initialize and destroy the factory
   React.useEffect(() => {
     if (config) {
       const factory = getSplitFactory(config);
+
+      return () => {
+        destroySplitFactory(factory);
+      }
+    }
+  }, [config]);
+
+  // Effect to subscribe/unsubscribe to events
+  React.useEffect(() => {
+    const factory = config && __factories.get(config);
+    if (factory) {
       const client = getSplitClient(factory);
       const status = getStatus(client);
 
-      // Update state and unsubscribe from events when first event is emitted
-      const update = () => {
+      // Unsubscribe from events and update state when first event is emitted
+      const update = () => { // eslint-disable-next-line no-use-before-define
+        unsubscribe();
+        setConfigFactory(factory);
+      }
+
+      const unsubscribe = () => {
         client.off(client.Event.SDK_READY, update);
         client.off(client.Event.SDK_READY_FROM_CACHE, update);
         client.off(client.Event.SDK_READY_TIMED_OUT, update);
         client.off(client.Event.SDK_UPDATE, update);
-
-        setStateFactory(factory);
       }
 
       if (updateOnSdkReady) {
@@ -61,10 +76,7 @@ export function SplitFactoryProvider(props: ISplitFactoryProps) {
       }
       if (updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, update);
 
-      return () => {
-        // Factory destroy unsubscribes from events
-        destroySplitFactory(factory as IFactoryWithClients);
-      }
+      return unsubscribe;
     }
   }, [config, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]);
 

src/__tests__/SplitFactoryProvider.test.tsx

@@ -336,40 +336,95 @@ describe('SplitFactoryProvider', () => {
     logSpy.mockRestore();
   });
 
-  test('cleans up on unmount.', () => {
-    let destroyMainClientSpy;
-    let destroySharedClientSpy;
-    const wrapper = render(
-      <SplitFactoryProvider config={sdkBrowser} >
-        {({ factory }) => {
-          if (!factory) return null; // 1st render
+  test('cleans up on update and unmount if config prop is provided.', () => {
+    let renderTimes = 0;
+    const createdFactories = new Set<SplitIO.ISDK>();
+    const clientDestroySpies: jest.SpyInstance[] = [];
+    const outerFactory = SplitSdk(sdkBrowser);
+
+    const Component = ({ factory, isReady, hasTimedout }: ISplitFactoryChildProps) => {
+      renderTimes++;
 
-          // 2nd render (SDK ready)
-          expect(__factories.size).toBe(1);
-          destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy');
+      switch (renderTimes) {
+        case 1:
+          expect(factory).toBe(outerFactory);
+          return null;
+        case 2:
+        case 5:
+          expect(isReady).toBe(false);
+          expect(hasTimedout).toBe(false);
+          expect(factory).toBe(null);
+          return null;
+        case 3:
+        case 4:
+        case 6:
+          expect(isReady).toBe(true);
+          expect(hasTimedout).toBe(true);
+          expect(factory).not.toBe(null);
+          createdFactories.add(factory!);
+          clientDestroySpies.push(jest.spyOn(factory!.client(), 'destroy'));
           return (
             <SplitClient splitKey='other_key' >
               {({ client }) => {
-                destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy');
+                clientDestroySpies.push(jest.spyOn(client!, 'destroy'));
                 return null;
               }}
             </SplitClient>
           );
-        }}
-      </SplitFactoryProvider>
-    );
+        case 7:
+          throw new Error('Must not rerender');
+      }
+    };
 
-    // SDK ready to re-render
-    act(() => {
+    const emitSdkEvents = () => {
       const factory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value;
+      factory.client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)
       factory.client().__emitter__.emit(Event.SDK_READY)
-    });
+    };
+
+    // 1st render: factory provided
+    const wrapper = render(
+      <SplitFactoryProvider factory={outerFactory} >
+        {Component}
+      </SplitFactoryProvider>
+    );
+
+    // 2nd render: factory created, not ready (null)
+    wrapper.rerender(
+      <SplitFactoryProvider config={sdkBrowser} >
+        {Component}
+      </SplitFactoryProvider>
+    );
+
+    // 3rd render: SDK ready (timeout is ignored due to updateOnSdkTimedout=false)
+    act(emitSdkEvents);
+
+    // 4th render: same config prop -> factory is not recreated
+    wrapper.rerender(
+      <SplitFactoryProvider config={sdkBrowser} updateOnSdkReady={false} updateOnSdkTimedout={true} >
+        {Component}
+      </SplitFactoryProvider>
+    );
+
+    act(emitSdkEvents); // Emitting events again has no effect
+    expect(createdFactories.size).toBe(1);
+
+    // 5th render: Update config prop -> factory is recreated, not ready yet (null)
+    wrapper.rerender(
+      <SplitFactoryProvider config={{ ...sdkBrowser }} updateOnSdkReady={false} updateOnSdkTimedout={true} >
+        {Component}
+      </SplitFactoryProvider>
+    );
+
+    // 6th render: SDK timeout (ready is ignored due to updateOnSdkReady=false)
+    act(emitSdkEvents);
 
     wrapper.unmount();
-    // the factory created by the component is removed from `factories` cache and its clients are destroyed
+
+    // Created factories are removed from `factories` cache and their clients are destroyed
+    expect(createdFactories.size).toBe(2);
     expect(__factories.size).toBe(0);
-    expect(destroyMainClientSpy).toBeCalledTimes(1);
-    expect(destroySharedClientSpy).toBeCalledTimes(1);
+    clientDestroySpies.forEach(spy => expect(spy).toBeCalledTimes(1));
   });
 
   test('doesn\'t clean up on unmount if the factory is provided as a prop.', () => {
@@ -381,11 +436,11 @@ describe('SplitFactoryProvider', () => {
         {({ factory }) => {
           // if factory is provided as a prop, `factories` cache is not modified
           expect(__factories.size).toBe(0);
-          destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy');
+          destroyMainClientSpy = jest.spyOn(factory!.client(), 'destroy');
           return (
             <SplitClient splitKey='other_key' >
               {({ client }) => {
-                destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy');
+                destroySharedClientSpy = jest.spyOn(client!, 'destroy');
                 return null;
               }}
             </SplitClient>

src/__tests__/useSplitClient.test.tsx

@@ -11,23 +11,23 @@ import { sdkBrowser } from './testUtils/sdkConfigs';
 
 /** Test target */
 import { useSplitClient } from '../useSplitClient';
-import { SplitFactory } from '../SplitFactory';
+import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { SplitClient } from '../SplitClient';
 import { SplitContext } from '../SplitContext';
 import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
 
 describe('useSplitClient', () => {
 
-  test('returns the main client from the context updated by SplitFactory.', () => {
+  test('returns the main client from the context updated by SplitFactoryProvider.', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     let client;
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         {React.createElement(() => {
           client = useSplitClient().client;
           return null;
         })}
-      </SplitFactory>
+      </SplitFactoryProvider>
     );
     expect(client).toBe(outerFactory.client());
   });
@@ -36,14 +36,14 @@ describe('useSplitClient', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     let client;
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         <SplitClient splitKey='user2' >
           {React.createElement(() => {
             client = useSplitClient().client;
             return null;
           })}
         </SplitClient>
-      </SplitFactory>
+      </SplitFactoryProvider>
     );
     expect(client).toBe(outerFactory.client('user2'));
   });
@@ -52,13 +52,13 @@ describe('useSplitClient', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     let client;
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         {React.createElement(() => {
           (outerFactory.client as jest.Mock).mockClear();
           client = useSplitClient({ splitKey: 'user2', trafficType: 'user' }).client;
           return null;
         })}
-      </SplitFactory>
+      </SplitFactoryProvider>
     );
     expect(outerFactory.client as jest.Mock).toBeCalledWith('user2', 'user');
     expect(outerFactory.client as jest.Mock).toHaveReturnedWith(client);
@@ -89,9 +89,9 @@ describe('useSplitClient', () => {
 
     function Component({ attributesFactory, attributesClient, splitKey, testSwitch, factory }: TestComponentProps) {
       return (
-        <SplitFactory factory={factory} attributes={attributesFactory} >
+        <SplitFactoryProvider factory={factory} attributes={attributesFactory} >
           <InnerComponent splitKey={splitKey} attributesClient={attributesClient} testSwitch={testSwitch} />
-        </SplitFactory>
+        </SplitFactoryProvider>
       );
     }
 
@@ -108,21 +108,21 @@ describe('useSplitClient', () => {
     let countNestedComponent = 0;
 
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         <>
           <SplitContext.Consumer>
             {() => countSplitContext++}
           </SplitContext.Consumer>
           <SplitClient splitKey={sdkBrowser.core.key} trafficType={sdkBrowser.core.trafficType}
-            /* Disabling update props is ineffective because the wrapping SplitFactory has them enabled: */
+            /* Disabling update props is ineffective because the wrapping SplitFactoryProvider has them enabled: */
             updateOnSdkReady={false} updateOnSdkReadyFromCache={false}
           >
             {() => { countSplitClient++; return null }}
           </SplitClient>
           {React.createElement(() => {
             // Equivalent to
             // - Using config key and traffic type: `const { client } = useSplitClient(sdkBrowser.core.key, sdkBrowser.core.trafficType, { att1: 'att1' });`
-            // - Disabling update props, since the wrapping SplitFactory has them enabled: `const { client } = useSplitClient(undefined, undefined, { att1: 'att1' }, { updateOnSdkReady: false, updateOnSdkReadyFromCache: false });`
+            // - Disabling update props, since the wrapping SplitFactoryProvider has them enabled: `const { client } = useSplitClient(undefined, undefined, { att1: 'att1' }, { updateOnSdkReady: false, updateOnSdkReadyFromCache: false });`
             const { client } = useSplitClient({ attributes: { att1: 'att1' } });
             expect(client).toBe(mainClient); // Assert that the main client was retrieved.
             expect(client!.getAttributes()).toEqual({ att1: 'att1' }); // Assert that the client was retrieved with the provided attributes.
@@ -183,7 +183,7 @@ describe('useSplitClient', () => {
             })}
           </SplitClient>
         </>
-      </SplitFactory>
+      </SplitFactoryProvider>
     );
 
     act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE));
@@ -226,7 +226,7 @@ describe('useSplitClient', () => {
     let count = 0;
 
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         {React.createElement(() => {
           useSplitClient({ splitKey: 'some_user' });
           count++;
@@ -237,7 +237,7 @@ describe('useSplitClient', () => {
 
           return null;
         })}
-      </SplitFactory>
+      </SplitFactoryProvider>
     )
 
     expect(count).toEqual(2);
@@ -257,9 +257,9 @@ describe('useSplitClient', () => {
 
     function Component(updateOptions) {
       return (
-        <SplitFactory factory={outerFactory} >
+        <SplitFactoryProvider factory={outerFactory} >
           <InnerComponent {...updateOptions} />
-        </SplitFactory>
+        </SplitFactoryProvider>
       )
     }
 

src/__tests__/useSplitManager.test.tsx

@@ -11,7 +11,7 @@ import { sdkBrowser } from './testUtils/sdkConfigs';
 import { getStatus } from '../utils';
 
 /** Test target */
-import { SplitFactory } from '../SplitFactory';
+import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { useSplitManager } from '../useSplitManager';
 
 describe('useSplitManager', () => {
@@ -20,12 +20,12 @@ describe('useSplitManager', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     let hookResult;
     render(
-      <SplitFactory factory={outerFactory} >
+      <SplitFactoryProvider factory={outerFactory} >
         {React.createElement(() => {
           hookResult = useSplitManager();
           return null;
         })}
-      </SplitFactory>
+      </SplitFactoryProvider>
     );
 
     expect(hookResult).toStrictEqual({