Merge pull request #154 from splitio/hooks_polishing

Hooks evaluation optimization

Author: EmilianoSanchez

CHANGES.txt

@@ -1,5 +1,6 @@
 1.10.0 (September XX, 2023)
  - Added TypeScript types and interfaces to the library index exports, allowing them to be imported from the library index. For example, `import type { ISplitFactoryProps } from '@splitsoftware/splitio-react';` (Related to issue https://github.com/splitio/react-client/issues/162).
+ - Updated the `useTreatments` hook to optimize feature flag evaluation. It now uses the `useMemo` hook to memoize calls to the SDK's `getTreatmentsWithConfig` function. This avoids re-evaluating feature flags when the hook is called with the same parameters and the feature flag definitions have not changed.
  - Updated linter and other dependencies for vulnerability fixes.
  - Bugfixing - To adhere to the rules of hooks and prevent React warnings, conditional code within hooks was removed. Previously, this code checked for the availability of the hooks API (available in React version 16.8.0 or above) and logged an error message. Now, using hooks with React versions below 16.8.0 will throw an error.
  - Bugfixing - Updated `useClient` and `useTreatments` hooks to re-render and re-evaluate feature flags when they consume a different SDK client than the context and its status updates (i.e., when it emits SDK_READY or other event).

src/SplitTreatments.tsx

@@ -1,21 +1,8 @@
 import React from 'react';
-import memoizeOne from 'memoize-one';
-import shallowEqual from 'shallowequal';
 import { SplitContext } from './SplitContext';
 import { ISplitTreatmentsProps, ISplitContextValues } from './types';
 import { getControlTreatmentsWithConfig, WARN_ST_NO_CLIENT } from './constants';
-
-function argsAreEqual(newArgs: any[], lastArgs: any[]): boolean {
-  return newArgs[0] === lastArgs[0] && // client
-    newArgs[1] === lastArgs[1] && // lastUpdate
-    shallowEqual(newArgs[2], lastArgs[2]) && // names
-    shallowEqual(newArgs[3], lastArgs[3]) && // attributes
-    shallowEqual(newArgs[4], lastArgs[4]); // client attributes
-}
-
-function evaluateFeatureFlags(client: SplitIO.IBrowserClient, lastUpdate: number, names: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes) {
-  return client.getTreatmentsWithConfig(names, attributes);
-}
+import { memoizeGetTreatmentsWithConfig } from './utils';
 
 /**
  * SplitTreatments accepts a list of feature flag names and optional attributes. It access the client at SplitContext to
@@ -27,9 +14,8 @@ export class SplitTreatments extends React.Component<ISplitTreatmentsProps> {
 
   private logWarning?: boolean;
 
-  // Attaching a memoized `client.getTreatmentsWithConfig` function to the component instance, to avoid duplicated impressions because
-  // the function result is the same given the same `client` instance, `lastUpdate` timestamp, and list of feature flag `names` and `attributes`.
-  private evaluateFeatureFlags = memoizeOne(evaluateFeatureFlags, argsAreEqual);
+  // Using a memoized `client.getTreatmentsWithConfig` function to avoid duplicated impressions
+  private evaluateFeatureFlags = memoizeGetTreatmentsWithConfig();
 
   render() {
     const { names, children, attributes } = this.props;

src/__tests__/SplitTreatments.test.tsx

@@ -25,6 +25,7 @@ jest.mock('../constants', () => {
 import { getControlTreatmentsWithConfig, WARN_ST_NO_CLIENT } from '../constants';
 import { getStatus } from '../utils';
 import { newSplitFactoryLocalhostInstance } from './testUtils/utils';
+import { useSplitTreatments } from '../useSplitTreatments';
 
 describe('SplitTreatments', () => {
 
@@ -143,13 +144,26 @@ describe('SplitTreatments', () => {
 
 });
 
+let renderTimes = 0;
+
 /**
- * Tests for asserting that client.getTreatmentsWithConfig is not called unnecessarely
+ * Tests for asserting that client.getTreatmentsWithConfig is not called unnecessarily when using SplitTreatments and useSplitTreatments.
  */
-describe('SplitTreatments optimization', () => {
-
-  let renderTimes = 0;
-
+describe.each([
+  ({ names, attributes }) => (
+    <SplitTreatments names={names} attributes={attributes} >
+      {() => {
+        renderTimes++;
+        return null;
+      }}
+    </SplitTreatments>
+  ),
+  ({ names, attributes }) => {
+    useSplitTreatments(names, attributes);
+    renderTimes++;
+    return null;
+  }
+])('SplitTreatments & useSplitTreatments optimization', (InnerComponent) => {
   let outerFactory = SplitSdk(sdkBrowser);
   (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
 
@@ -162,12 +176,7 @@ describe('SplitTreatments optimization', () => {
     return (
       <SplitFactory factory={outerFactory} >
         <SplitClient splitKey={splitKey} updateOnSdkUpdate={true} attributes={clientAttributes} >
-          <SplitTreatments names={names} attributes={attributes} >
-            {() => {
-              renderTimes++;
-              return null;
-            }}
-          </SplitTreatments>
+          <InnerComponent names={names} attributes={attributes} />
         </SplitClient>
       </SplitFactory>
     );
@@ -243,17 +252,17 @@ describe('SplitTreatments optimization', () => {
     (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
   });
 
-  it('rerenders and re-evaluates feature flags if client changes.', () => {
+  it('rerenders and re-evaluates feature flags if client changes.', async () => {
     wrapper.rerender(<Component names={names} attributes={attributes} splitKey={'otherKey'} />);
-    act(() => (outerFactory as any).client('otherKey').__emitter__.emit(Event.SDK_READY));
+    await act(() => (outerFactory as any).client('otherKey').__emitter__.emit(Event.SDK_READY));
 
     // Initial render + 2 renders (in 3 updates) -> automatic batching https://reactjs.org/blog/2022/03/29/react-v18.html#new-feature-automatic-batching
     expect(renderTimes).toBe(3);
     expect(outerFactory.client().getTreatmentsWithConfig).toBeCalledTimes(1);
     expect(outerFactory.client('otherKey').getTreatmentsWithConfig).toBeCalledTimes(1);
   });
 
-  it('rerenders and re-evaluate splfeature flagsits when Split context changes (in both SplitFactory and SplitClient components).', async () => {
+  it('rerenders and re-evaluate feature flags when Split context changes (in both SplitFactory and SplitClient components).', async () => {
     // changes in SplitContext implies that either the factory, the client (user key), or its status changed, what might imply a change in treatments
     const outerFactory = SplitSdk(sdkBrowser);
     const names = ['split1', 'split2'];

src/__tests__/useSplitClient.test.tsx

@@ -74,9 +74,10 @@ test('useSplitClient must update on SDK events', () => {
         <SplitClient splitKey={'user_2'} updateOnSdkUpdate={true}>
           {React.createElement(() => {
             const status = useSplitClient('user_2', undefined, undefined, { updateOnSdkUpdate: true });
-            countNestedComponent++;
-
             expect(status.client).toBe(user2Client);
+
+            // useSplitClient doesn't re-render twice if it is in the context of a SplitClient with same user key and there is a SDK event
+            countNestedComponent++;
             switch (countNestedComponent) {
               case 1:
                 expect(status.isReady).toBe(false);

src/__tests__/useSplitTreatments.test.tsx

@@ -34,7 +34,7 @@ function validateTreatments({ treatments, isReady, isReadyFromCache }: ISplitTre
   }
 }
 
-test('useSplitTreatments', async () => {
+test('useSplitTreatments must update on SDK events', async () => {
   const outerFactory = SplitSdk(sdkBrowser);
   const mainClient = outerFactory.client() as any;
   const user2Client = outerFactory.client('user_2') as any;

src/__tests__/useTrack.test.tsx

@@ -21,16 +21,16 @@ describe('useTrack', () => {
   const value = 10;
   const properties = { prop1: 'prop1' };
 
-  test('returns the track method binded to the client at Split context updated by SplitFactory.', () => {
+  test('returns the track method bound to the client at Split context updated by SplitFactory.', () => {
     const outerFactory = SplitSdk(sdkBrowser);
-    let bindedTrack;
+    let boundTrack;
     let trackResult;
 
     render(
       <SplitFactory factory={outerFactory} >
         {React.createElement(() => {
-          bindedTrack = useTrack();
-          trackResult = bindedTrack(tt, eventType, value, properties);
+          boundTrack = useTrack();
+          trackResult = boundTrack(tt, eventType, value, properties);
           return null;
         })}
       </SplitFactory>,
@@ -40,17 +40,17 @@ describe('useTrack', () => {
     expect(track).toHaveReturnedWith(trackResult);
   });
 
-  test('returns the track method binded to the client at Split context updated by SplitClient.', () => {
+  test('returns the track method bound to the client at Split context updated by SplitClient.', () => {
     const outerFactory = SplitSdk(sdkBrowser);
-    let bindedTrack;
+    let boundTrack;
     let trackResult;
 
     render(
       <SplitFactory factory={outerFactory} >
         <SplitClient splitKey='user2' >
           {React.createElement(() => {
-            bindedTrack = useTrack();
-            trackResult = bindedTrack(tt, eventType, value, properties);
+            boundTrack = useTrack();
+            trackResult = boundTrack(tt, eventType, value, properties);
             return null;
           })}
         </SplitClient>
@@ -61,16 +61,16 @@ describe('useTrack', () => {
     expect(track).toHaveReturnedWith(trackResult);
   });
 
-  test('returns the track method binded to a new client given a splitKey and optional trafficType.', () => {
+  test('returns the track method bound to a new client given a splitKey and optional trafficType.', () => {
     const outerFactory = SplitSdk(sdkBrowser);
-    let bindedTrack;
+    let boundTrack;
     let trackResult;
 
     render(
       <SplitFactory factory={outerFactory} >
         {React.createElement(() => {
-          bindedTrack = useTrack('user2', tt);
-          trackResult = bindedTrack(eventType, value, properties);
+          boundTrack = useTrack('user2', tt);
+          trackResult = boundTrack(eventType, value, properties);
           return null;
         })}
       </SplitFactory>,

src/useSplitTreatments.ts

@@ -1,5 +1,6 @@
+import React from 'react';
 import { getControlTreatmentsWithConfig } from './constants';
-import { IClientWithContext } from './utils';
+import { IClientWithContext, memoizeGetTreatmentsWithConfig } from './utils';
 import { ISplitTreatmentsChildProps, IUpdateProps } from './types';
 import { useSplitClient } from './useSplitClient';
 
@@ -13,8 +14,11 @@ import { useSplitClient } from './useSplitClient';
 export function useSplitTreatments(splitNames: string[], attributes?: SplitIO.Attributes, key?: SplitIO.SplitKey, options?: IUpdateProps): ISplitTreatmentsChildProps {
   const context = useSplitClient(key, undefined, undefined, options);
   const client = context.client;
+
+  const getTreatmentsWithConfig = React.useMemo(memoizeGetTreatmentsWithConfig, []);
+
   const treatments = client && (client as IClientWithContext).__getStatus().isOperational ?
-    client.getTreatmentsWithConfig(splitNames, attributes) :
+    getTreatmentsWithConfig(client, context.lastUpdate, splitNames, attributes, { ...client.getAttributes() }) :
     getControlTreatmentsWithConfig(splitNames);
 
   return {

src/useTrack.ts

@@ -7,7 +7,7 @@ const noOpFalse = () => false;
  * 'useTrack' is a hook that returns the track method from a Split client.
  * It uses the 'useContext' hook to access the client from the Split context.
  *
- * @return A track function binded to a Split client. If the client is not available, the result is a no-op function that returns false.
+ * @return A track function bound to a Split client. If the client is not available, the result is a no-op function that returns false.
  * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#track}
  */
 export function useTrack(key?: SplitIO.SplitKey, trafficType?: string): SplitIO.IBrowserClient['track'] {

src/utils.ts

@@ -1,3 +1,5 @@
+import memoizeOne from 'memoize-one';
+import shallowEqual from 'shallowequal';
 import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client';
 import { VERSION } from './constants';
 import { ISplitStatus } from './types';
@@ -160,3 +162,23 @@ function uniq(arr: string[]): string[] {
 function isString(val: unknown): val is string {
   return typeof val === 'string' || val instanceof String;
 }
+
+/**
+ * Gets a memoized version of the `client.getTreatmentsWithConfig` method.
+ * It is used to avoid duplicated impressions, because the result treatments are the same given the same `client` instance, `lastUpdate` timestamp, and list of feature flag `names` and `attributes`.
+ */
+export function memoizeGetTreatmentsWithConfig() {
+  return memoizeOne(evaluateFeatureFlags, argsAreEqual);
+}
+
+function argsAreEqual(newArgs: any[], lastArgs: any[]): boolean {
+  return newArgs[0] === lastArgs[0] && // client
+    newArgs[1] === lastArgs[1] && // lastUpdate
+    shallowEqual(newArgs[2], lastArgs[2]) && // names
+    shallowEqual(newArgs[3], lastArgs[3]) && // attributes
+    shallowEqual(newArgs[4], lastArgs[4]); // client attributes
+}
+
+function evaluateFeatureFlags(client: SplitIO.IBrowserClient, lastUpdate: number, names: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes) {
+  return client.getTreatmentsWithConfig(names, attributes);
+}

types/useTrack.d.ts

@@ -2,7 +2,7 @@
  * 'useTrack' is a hook that returns the track method from a Split client.
  * It uses the 'useContext' hook to access the client from the Split context.
  *
- * @return A track function binded to a Split client. If the client is not available, the result is a no-op function that returns false.
+ * @return A track function bound to a Split client. If the client is not available, the result is a no-op function that returns false.
  * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#track}
  */
 export declare function useTrack(key?: SplitIO.SplitKey, trafficType?: string): SplitIO.IBrowserClient['track'];