Merge pull request #172 from splitio/hooks_improvements

Hooks improvements

CHANGES.txt

@@ -1,3 +1,6 @@
+1.10.1 (November 21, 2023)
+ - Bugfixing - Resolved an issue with `useSplitClient` hook, which was not re-rendering when an SDK client event was emitted between the hook's call (render phase) and the execution of its internal effect (commit phase).
+
 1.10.0 (November 16, 2023)
  - Added support for Flag Sets on the SDK, which enables grouping feature flags and interacting with the group rather than individually (more details in our documentation):
       - Added a new `flagSets` prop to the `SplitTreatments` component and `flagSets` option to the `useSplitTreatments` hook options object, to support evaluating flags in given flag set/s. Either `names` or `flagSets` must be provided to the component and hook. If both are provided, `names` will be used.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-react",
-  "version": "1.10.0",
+  "version": "1.10.1",
   "description": "A React library to easily integrate and use Split JS SDK",
   "main": "lib/index.js",
   "module": "es/index.js",

src/SplitClient.tsx

@@ -63,37 +63,36 @@ export class SplitComponent extends React.Component<IUpdateProps & { factory: Sp
   // The listeners take into account the value of `updateOnSdk***` props.
   subscribeToEvents(client: SplitIO.IBrowserClient | null) {
     if (client) {
-      const status = getStatus(client);
-      if (!status.isReady) client.once(client.Event.SDK_READY, this.setReady);
-      if (!status.isReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, this.setReadyFromCache);
-      if (!status.hasTimedout && !status.isReady) client.once(client.Event.SDK_READY_TIMED_OUT, this.setTimedout);
-      client.on(client.Event.SDK_UPDATE, this.setUpdate);
+      const statusOnEffect = getStatus(client);
+      const status = this.state;
+
+      if (this.props.updateOnSdkReady) {
+        if (!statusOnEffect.isReady) client.once(client.Event.SDK_READY, this.update);
+        else if (!status.isReady) this.update();
+      }
+      if (this.props.updateOnSdkReadyFromCache) {
+        if (!statusOnEffect.isReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, this.update);
+        else if (!status.isReadyFromCache) this.update();
+      }
+      if (this.props.updateOnSdkTimedout) {
+        if (!statusOnEffect.hasTimedout) client.once(client.Event.SDK_READY_TIMED_OUT, this.update);
+        else if (!status.hasTimedout) this.update();
+      }
+      if (this.props.updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, this.update);
     }
   }
 
   unsubscribeFromEvents(client: SplitIO.IBrowserClient | null) {
     if (client) {
-      client.removeListener(client.Event.SDK_READY, this.setReady);
-      client.removeListener(client.Event.SDK_READY_FROM_CACHE, this.setReadyFromCache);
-      client.removeListener(client.Event.SDK_READY_TIMED_OUT, this.setTimedout);
-      client.removeListener(client.Event.SDK_UPDATE, this.setUpdate);
+      client.off(client.Event.SDK_READY, this.update);
+      client.off(client.Event.SDK_READY_FROM_CACHE, this.update);
+      client.off(client.Event.SDK_READY_TIMED_OUT, this.update);
+      client.off(client.Event.SDK_UPDATE, this.update);
     }
   }
 
-  setReady = () => {
-    if (this.props.updateOnSdkReady) this.setState({ lastUpdate: (this.state.client as IClientWithContext).lastUpdate });
-  }
-
-  setReadyFromCache = () => {
-    if (this.props.updateOnSdkReadyFromCache) this.setState({ lastUpdate: (this.state.client as IClientWithContext).lastUpdate });
-  }
-
-  setTimedout = () => {
-    if (this.props.updateOnSdkTimedout) this.setState({ lastUpdate: (this.state.client as IClientWithContext).lastUpdate });
-  }
-
-  setUpdate = () => {
-    if (this.props.updateOnSdkUpdate) this.setState({ lastUpdate: (this.state.client as IClientWithContext).lastUpdate });
+  update = () => {
+    this.setState({ lastUpdate: (this.state.client as IClientWithContext).lastUpdate });
   }
 
   componentDidMount() {

src/__tests__/useSplitClient.test.tsx

@@ -82,7 +82,7 @@ describe('useSplitClient', () => {
 
     // eslint-disable-next-line react/prop-types
     const InnerComponent = ({ splitKey, attributesClient, testSwitch }) => {
-      useSplitClient({ splitKey, trafficType: 'user', attributes: attributesClient});
+      useSplitClient({ splitKey, trafficType: 'user', attributes: attributesClient });
       testSwitch(done, splitKey);
       return null;
     };
@@ -98,13 +98,13 @@ describe('useSplitClient', () => {
     testAttributesBinding(Component);
   });
 
-  test('useSplitClient must update on SDK events', () => {
+  test('must update on SDK events', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     const mainClient = outerFactory.client() as any;
     const user2Client = outerFactory.client('user_2') as any;
 
     let countSplitContext = 0, countSplitClient = 0, countSplitClientUser2 = 0, countUseSplitClient = 0, countUseSplitClientUser2 = 0;
-    let countSplitClientWithUpdate = 0, countUseSplitClientWithUpdate = 0, countSplitClientUser2WithUpdate = 0, countUseSplitClientUser2WithUpdate = 0;
+    let countSplitClientWithUpdate = 0, countUseSplitClientWithUpdate = 0, countSplitClientUser2WithUpdate = 0, countUseSplitClientUser2WithTimeout = 0;
     let countNestedComponent = 0;
 
     render(
@@ -150,8 +150,8 @@ describe('useSplitClient', () => {
             {() => { countSplitClientUser2WithUpdate++; return null }}
           </SplitClient>
           {React.createElement(() => {
-            useSplitClient({ splitKey: 'user_2', updateOnSdkUpdate: true });
-            countUseSplitClientUser2WithUpdate++;
+            useSplitClient({ splitKey: 'user_2', updateOnSdkTimedout: true });
+            countUseSplitClientUser2WithTimeout++;
             return null;
           })}
           <SplitClient splitKey={'user_2'} updateOnSdkUpdate={true}>
@@ -190,6 +190,7 @@ describe('useSplitClient', () => {
     act(() => mainClient.__emitter__.emit(Event.SDK_READY));
     act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE));
     act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE));
+    act(() => user2Client.__emitter__.emit(Event.SDK_READY_TIMED_OUT));
     act(() => user2Client.__emitter__.emit(Event.SDK_READY));
     act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE));
 
@@ -214,12 +215,35 @@ describe('useSplitClient', () => {
     // If SplitClient and useSplitClient retrieve a different client than the context and have updateOnSdkUpdate = true,
     // they render when the context renders and when the new client is ready, ready from cache and updates.
     expect(countSplitClientUser2WithUpdate).toEqual(countSplitContext + 3);
-    expect(countUseSplitClientUser2WithUpdate).toEqual(countSplitContext + 3);
+    expect(countUseSplitClientUser2WithTimeout).toEqual(countSplitContext + 3);
 
     expect(countNestedComponent).toEqual(4);
   });
 
-  test('useSplitClient must support changes in update props', () => {
+  // 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)', () =>{
+    const outerFactory = SplitSdk(sdkBrowser);
+    let count = 0;
+
+    render(
+      <SplitFactory factory={outerFactory} >
+        {React.createElement(() => {
+          useSplitClient({ splitKey: 'some_user' });
+          count++;
+
+          // side effect in the render phase
+          const client = outerFactory.client('some_user') as any;
+          if (!client.__getStatus().isReady) client.__emitter__.emit(Event.SDK_READY);
+
+          return null;
+        })}
+      </SplitFactory>
+    )
+
+    expect(count).toEqual(2);
+  });
+
+  test('must support changes in update props', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     const mainClient = outerFactory.client() as any;
 
@@ -240,19 +264,24 @@ describe('useSplitClient', () => {
     }
 
     const wrapper = render(<Component />);
+    expect(rendersCount).toBe(1);
 
     act(() => mainClient.__emitter__.emit(Event.SDK_READY)); // trigger re-render
+    expect(rendersCount).toBe(2);
+
     act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false by default
     expect(rendersCount).toBe(2);
 
     wrapper.rerender(<Component updateOnSdkUpdate={true} />); // trigger re-render
-    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // trigger re-render because updateOnSdkUpdate is true now
+    expect(rendersCount).toBe(3);
 
+    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // trigger re-render because updateOnSdkUpdate is true now
     expect(rendersCount).toBe(4);
 
     wrapper.rerender(<Component updateOnSdkUpdate={false} />); // trigger re-render
-    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false now
+    expect(rendersCount).toBe(5);
 
+    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false now
     expect(rendersCount).toBe(5);
   });
 

src/useSplitClient.ts

@@ -33,23 +33,36 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV
 
   let client = contextClient as IClientWithContext;
   if (splitKey && factory) {
+    // @TODO `getSplitClient` starts client sync. Move side effects to useEffect
     client = getSplitClient(factory, splitKey, trafficType);
   }
   initAttributes(client, attributes);
 
-  const [, setLastUpdate] = React.useState(client ? client.lastUpdate : 0);
+  const status = getStatus(client);
+  const [, setLastUpdate] = React.useState(status.lastUpdate);
 
   // Handle client events
   React.useEffect(() => {
     if (!client) return;
 
     const update = () => setLastUpdate(client.lastUpdate);
 
+    // Clients are created on the hook's call, so the status may have changed
+    const statusOnEffect = getStatus(client);
+
     // Subscribe to SDK events
-    const status = getStatus(client);
-    if (!status.isReady && updateOnSdkReady) client.once(client.Event.SDK_READY, update);
-    if (!status.isReadyFromCache && updateOnSdkReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, update);
-    if (!status.hasTimedout && !status.isReady && updateOnSdkTimedout) client.once(client.Event.SDK_READY_TIMED_OUT, update);
+    if (updateOnSdkReady) {
+      if (!statusOnEffect.isReady) client.once(client.Event.SDK_READY, update);
+      else if (!status.isReady) update();
+    }
+    if (updateOnSdkReadyFromCache) {
+      if (!statusOnEffect.isReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, update);
+      else if (!status.isReadyFromCache) update();
+    }
+    if (updateOnSdkTimedout) {
+      if (!statusOnEffect.hasTimedout) client.once(client.Event.SDK_READY_TIMED_OUT, update);
+      else if (!status.hasTimedout) update();
+    }
     if (updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, update);
 
     return () => {
@@ -59,9 +72,9 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV
       client.off(client.Event.SDK_READY_TIMED_OUT, update);
       client.off(client.Event.SDK_UPDATE, update);
     }
-  }, [client, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]);
+  }, [client, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate, status]);
 
   return {
-    factory, client, ...getStatus(client)
+    factory, client, ...status
   };
 }