Merge pull request #234 from splitio/fix_update_options_toggle

Fix: toggling `updateOnSdkUpdate` option

Author: EmilianoSanchez

CHANGES.txt

@@ -1,5 +1,5 @@
 2.1.1 (April 8, 2025)
- - Bugfix - Fixed `useSplitClient` and `useSplitTreatments` hooks to properly respect `updateOn<Event>` options. Previously, if the hooks were re-called due to a component re-render, they used the latest version of the SDK client status ignoring when `updateOn<Event>` options were set to `false` and resulting in unexpected changes in treatment values.
+ - Bugfixing - Fixed `useSplitClient` and `useSplitTreatments` hooks to properly respect `updateOn<Event>` options. Previously, if the hooks were re-called due to a component re-render, they used the latest version of the SDK client status ignoring when `updateOn<Event>` options were set to `false` and resulting in unexpected changes in treatment values.
 
 2.1.0 (March 28, 2025)
  - Added a new optional `properties` argument to the options object of the `useSplitTreatments` hook, allowing to pass a map of properties to append to the generated impressions sent to Split backend. Read more in our docs.

src/__tests__/useSplitClient.test.tsx

@@ -13,7 +13,7 @@ import { sdkBrowser } from './testUtils/sdkConfigs';
 import { useSplitClient } from '../useSplitClient';
 import { SplitFactoryProvider } from '../SplitFactoryProvider';
 import { SplitContext } from '../SplitContext';
-import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
+import { INITIAL_STATUS, testAttributesBinding, TestComponentProps } from './testUtils/utils';
 import { EXCEPTION_NO_SFP } from '../constants';
 
 describe('useSplitClient', () => {
@@ -222,9 +222,11 @@ describe('useSplitClient', () => {
     const mainClient = outerFactory.client() as any;
 
     let rendersCount = 0;
+    let currentStatus, previousStatus;
 
     function InnerComponent(updateOptions) {
-      useSplitClient(updateOptions);
+      previousStatus = currentStatus;
+      currentStatus = useSplitClient(updateOptions);
       rendersCount++;
       return null;
     }
@@ -237,26 +239,46 @@ describe('useSplitClient', () => {
       )
     }
 
-    const wrapper = render(<Component updateOnSdkUpdate={false} />);
+    const wrapper = render(<Component updateOnSdkUpdate={false} updateOnSdkTimedout={false} />);
     expect(rendersCount).toBe(1);
 
-    act(() => mainClient.__emitter__.emit(Event.SDK_READY)); // trigger re-render
-    expect(rendersCount).toBe(2);
+    act(() => mainClient.__emitter__.emit(Event.SDK_READY_TIMED_OUT)); // do not trigger re-render because updateOnSdkTimedout is false
+    expect(rendersCount).toBe(1);
+    expect(currentStatus).toMatchObject(INITIAL_STATUS);
 
-    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false
+    wrapper.rerender(<Component updateOnSdkUpdate={false} updateOnSdkTimedout={false} />);
     expect(rendersCount).toBe(2);
+    expect(currentStatus).toEqual(previousStatus);
 
-    wrapper.rerender(<Component updateOnSdkUpdate={null /** invalid type should default to `true` */} />); // trigger re-render
-    expect(rendersCount).toBe(3);
-
-    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // trigger re-render because updateOnSdkUpdate is true now
+    wrapper.rerender(<Component updateOnSdkUpdate={false} updateOnSdkTimedout={true} />); // trigger re-render because there was an SDK_READY_TIMED_OUT event
     expect(rendersCount).toBe(4);
+    expect(currentStatus).toMatchObject({ isReady: false, isReadyFromCache: false, hasTimedout: true });
 
-    wrapper.rerender(<Component updateOnSdkUpdate={false} />); // trigger re-render
+    act(() => mainClient.__emitter__.emit(Event.SDK_READY)); // trigger re-render
     expect(rendersCount).toBe(5);
+    expect(currentStatus).toMatchObject({ isReady: true, isReadyFromCache: false, hasTimedout: true });
 
-    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false now
+    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false
     expect(rendersCount).toBe(5);
+
+    wrapper.rerender(<Component updateOnSdkUpdate={false} updateOnSdkTimedout={false} />); // should not update the status (SDK_UPDATE event should be ignored)
+    expect(rendersCount).toBe(6);
+    expect(currentStatus).toEqual(previousStatus);
+
+    wrapper.rerender(<Component updateOnSdkUpdate={null /** invalid type should default to `true` */} />); // trigger re-render and update the status because updateOnSdkUpdate is true and there was an SDK_UPDATE event
+    expect(rendersCount).toBe(8);
+    expect(currentStatus.lastUpdate).toBeGreaterThan(previousStatus.lastUpdate);
+
+    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // trigger re-render because updateOnSdkUpdate is true
+    expect(rendersCount).toBe(9);
+    expect(currentStatus.lastUpdate).toBeGreaterThan(previousStatus.lastUpdate);
+
+    wrapper.rerender(<Component updateOnSdkUpdate={false} />);
+    expect(rendersCount).toBe(10);
+    expect(currentStatus).toEqual(previousStatus);
+
+    act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); // do not trigger re-render because updateOnSdkUpdate is false now
+    expect(rendersCount).toBe(10);
   });
 
 });

src/useSplitClient.ts

@@ -31,7 +31,7 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV
   const context = useSplitContext();
   const { client: contextClient, factory } = context;
 
-  // @TODO Move `getSplitClient` side effects
+  // @TODO Move `getSplitClient` side effects and reduce the function cognitive complexity
   // @TODO Once `SplitClient` is removed, which updates the context, simplify next line as `const client = factory ? getSplitClient(factory, splitKey) : undefined;`
   const client = factory && splitKey ? getSplitClient(factory, splitKey) : contextClient;
 
@@ -68,7 +68,10 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV
         if (!status.hasTimedout) update();
       }
     }
-    if (updateOnSdkUpdate !== false) client.on(client.Event.SDK_UPDATE, update);
+    if (updateOnSdkUpdate !== false) {
+      client.on(client.Event.SDK_UPDATE, update);
+      if (statusOnEffect.isReady && statusOnEffect.lastUpdate > status.lastUpdate) update();
+    }
 
     return () => {
       // Unsubscribe from events