From 44f0d13c4123701842b2de29ea4a8ac67f5dd326 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 17 Jul 2023 17:42:23 -0300 Subject: [PATCH 1/8] optimize evaluation on useSplitTreatments using a memoized version of client.getTreatmentsWithConfig --- src/SplitTreatments.tsx | 20 +++----------------- src/useSplitTreatments.ts | 8 ++++++-- src/utils.ts | 25 ++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/SplitTreatments.tsx b/src/SplitTreatments.tsx index 8870e16..563b2cf 100644 --- a/src/SplitTreatments.tsx +++ b/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 { 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; diff --git a/src/useSplitTreatments.ts b/src/useSplitTreatments.ts index a090f6c..1a8485a 100644 --- a/src/useSplitTreatments.ts +++ b/src/useSplitTreatments.ts @@ -1,5 +1,6 @@ +import React from 'react'; import { getControlTreatmentsWithConfig, ERROR_UT_NO_USECONTEXT } from './constants'; -import { checkHooks, IClientWithContext } from './utils'; +import { checkHooks, IClientWithContext, memoizeGetTreatmentsWithConfig } from './utils'; import { ISplitTreatmentsChildProps, IUpdateProps } from './types'; import { INITIAL_CONTEXT } from './SplitContext'; import { useSplitClient } from './useSplitClient'; @@ -14,8 +15,11 @@ import { useSplitClient } from './useSplitClient'; export function useSplitTreatments(splitNames: string[], attributes?: SplitIO.Attributes, key?: SplitIO.SplitKey, options?: IUpdateProps): ISplitTreatmentsChildProps { const context = checkHooks(ERROR_UT_NO_USECONTEXT) ? useSplitClient(key, undefined, undefined, options) : INITIAL_CONTEXT; 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 { diff --git a/src/utils.ts b/src/utils.ts index 50f93f6..66b2f6e 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,4 +1,6 @@ import React from 'react'; +import memoizeOne from 'memoize-one'; +import shallowEqual from 'shallowequal'; import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client'; import { VERSION } from './constants'; @@ -91,7 +93,8 @@ export function getStatus(client: SplitIO.IBrowserClient | null): IClientStatus // Other utils /** - * Checks if React.useContext is available, and logs given message if not + * Checks if React.useContext is available, and logs given message if not. + * Checking for React.useContext is a way to detect if the current React version is >= 16.8.0 and other hooks used by the SDK are available too. * * @param message * @returns boolean indicating if React.useContext is available @@ -170,3 +173,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); +} From 7f64178885812914064846ad5268e71e967c87f5 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 17 Jul 2023 18:02:43 -0300 Subject: [PATCH 2/8] adding a delay on test, to reduce flakiness --- src/__tests__/useSplitTreatments.test.tsx | 16 +++++++++------- types/utils.d.ts | 10 +++++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/__tests__/useSplitTreatments.test.tsx b/src/__tests__/useSplitTreatments.test.tsx index 5cf48ea..a69394d 100644 --- a/src/__tests__/useSplitTreatments.test.tsx +++ b/src/__tests__/useSplitTreatments.test.tsx @@ -75,13 +75,15 @@ test('useSplitTreatments', async () => { ); - // Awaiting to make sure each event is processed with a different lastUpdate timestamp. - await act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - await act(() => mainClient.__emitter__.emit(Event.SDK_READY)); - await act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); - await act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - await act(() => user2Client.__emitter__.emit(Event.SDK_READY)); - await act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); + // Adding a delay between events to make sure they are processed with a different lastUpdate timestamp. + act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + await new Promise(resolve => setTimeout(resolve, 10)); + act(() => mainClient.__emitter__.emit(Event.SDK_READY)); + act(() => user2Client.__emitter__.emit(Event.SDK_READY)); + await new Promise(resolve => setTimeout(resolve, 10)); + act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); + act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); // SplitContext renders 3 times: initially, when ready from cache, and when ready. expect(countSplitContext).toEqual(3); diff --git a/types/utils.d.ts b/types/utils.d.ts index 71fc810..4a7a503 100644 --- a/types/utils.d.ts +++ b/types/utils.d.ts @@ -30,7 +30,8 @@ export interface IClientStatus { } export declare function getStatus(client: SplitIO.IBrowserClient | null): IClientStatus; /** - * Checks if React.useContext is available, and logs given message if not + * Checks if React.useContext is available, and logs given message if not. + * Checking for React.useContext is a way to detect if the current React version is >= 16.8.0 and other hooks used by the SDK are available too. * * @param message * @returns boolean indicating if React.useContext is available @@ -41,3 +42,10 @@ export declare function validateFeatureFlags(maybeFeatureFlags: unknown, listNam * Manage client attributes binding */ export declare function initAttributes(client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes): void; +/** + * 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 declare function memoizeGetTreatmentsWithConfig(): typeof evaluateFeatureFlags; +declare function evaluateFeatureFlags(client: SplitIO.IBrowserClient, lastUpdate: number, names: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes): import("@splitsoftware/splitio/types/splitio").TreatmentsWithConfig; +export {}; From 1239b3e40f26aea346fc0428c9c3869627f14897 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 17 Jul 2023 23:43:51 -0300 Subject: [PATCH 3/8] Rename getSplitSharedClient to getSplitClient, and reuse in SplitFactory to get the main client --- src/SplitClient.tsx | 6 +++--- src/SplitFactory.tsx | 4 ++-- src/__tests__/useSplitClient.test.tsx | 8 +++++--- src/useSplitClient.ts | 4 ++-- src/utils.ts | 19 +++++++++---------- types/utils.d.ts | 4 ++-- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index b05ae03..8c1bc72 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { SplitContext } from './SplitContext'; import { ISplitClientProps, ISplitContextValues, IUpdateProps } from './types'; import { ERROR_SC_NO_FACTORY } from './constants'; -import { getStatus, getSplitSharedClient, initAttributes } from './utils'; +import { getStatus, getSplitClient, initAttributes } from './utils'; /** * Common component used to handle the status and events of a Split client passed as prop. @@ -145,8 +145,8 @@ export function SplitClient(props: ISplitClientProps) { {(splitContext: ISplitContextValues) => { const { factory } = splitContext; - // getSplitSharedClient is idempotent like factory.client: it returns the same client given the same factory, Split Key and TT - const client = factory ? getSplitSharedClient(factory, props.splitKey, props.trafficType) : null; + // getSplitClient is idempotent like factory.client: it returns the same client given the same factory, Split Key and TT + const client = factory ? getSplitClient(factory, props.splitKey, props.trafficType) : null; return ( ); diff --git a/src/SplitFactory.tsx b/src/SplitFactory.tsx index 6de0019..5460e0e 100644 --- a/src/SplitFactory.tsx +++ b/src/SplitFactory.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { SplitComponent } from './SplitClient'; import { ISplitFactoryProps } from './types'; import { WARN_SF_CONFIG_AND_FACTORY, ERROR_SF_NO_CONFIG_AND_FACTORY } from './constants'; -import { getSplitFactory, destroySplitFactory, IFactoryWithClients } from './utils'; +import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient } from './utils'; /** * SplitFactory will initialize the Split SDK and its main client, listen for its events in order to update the Split Context, @@ -54,7 +54,7 @@ export class SplitFactory extends React.Component { +test('useSplitClient', async () => { const outerFactory = SplitSdk(sdkBrowser); const mainClient = outerFactory.client() as any; const user2Client = outerFactory.client('user_2') as any; @@ -97,10 +97,12 @@ test('useSplitClient', () => { ); act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - act(() => mainClient.__emitter__.emit(Event.SDK_READY)); - act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + await new Promise(resolve => setTimeout(resolve, 10)); + act(() => mainClient.__emitter__.emit(Event.SDK_READY)); act(() => user2Client.__emitter__.emit(Event.SDK_READY)); + await new Promise(resolve => setTimeout(resolve, 10)); + act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); // SplitContext renders 3 times: initially, when ready from cache, and when ready. diff --git a/src/useSplitClient.ts b/src/useSplitClient.ts index 8ba403a..318dc92 100644 --- a/src/useSplitClient.ts +++ b/src/useSplitClient.ts @@ -1,7 +1,7 @@ import React from 'react'; import { SplitContext, INITIAL_CONTEXT } from './SplitContext'; import { ERROR_UC_NO_USECONTEXT } from './constants'; -import { getSplitSharedClient, checkHooks, initAttributes, IClientWithContext, getStatus } from './utils'; +import { getSplitClient, checkHooks, initAttributes, IClientWithContext, getStatus } from './utils'; import { ISplitContextValues, IUpdateProps } from './types'; const DEFAULT_OPTIONS = { @@ -30,7 +30,7 @@ export function useSplitClient(key?: SplitIO.SplitKey, trafficType?: string, att let client = contextClient as IClientWithContext; if (key && factory) { - client = getSplitSharedClient(factory, key, trafficType); + client = getSplitClient(factory, key, trafficType); } initAttributes(client, attributes); diff --git a/src/utils.ts b/src/utils.ts index 66b2f6e..c9d135d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -23,7 +23,7 @@ export interface IClientWithContext extends SplitIO.IBrowserClient { * FactoryWithClientInstances interface. */ export interface IFactoryWithClients extends SplitIO.IBrowserSDK { - sharedClientInstances: Set; + clientInstances: Set; config: SplitIO.IBrowserSettings; } @@ -38,7 +38,7 @@ export function getSplitFactory(config: SplitIO.IBrowserSettings): IFactoryWithC const newFactory = SplitSdk(config, (modules) => { modules.settings.version = VERSION; }) as IFactoryWithClients; - newFactory.sharedClientInstances = new Set(); + newFactory.clientInstances = new Set(); newFactory.config = config; __factories.set(config, newFactory); } @@ -46,22 +46,21 @@ export function getSplitFactory(config: SplitIO.IBrowserSettings): IFactoryWithC } // idempotent operation -export function getSplitSharedClient(factory: SplitIO.IBrowserSDK, key: SplitIO.SplitKey, trafficType?: string): IClientWithContext { +export function getSplitClient(factory: SplitIO.IBrowserSDK, key?: SplitIO.SplitKey, trafficType?: string): IClientWithContext { // factory.client is an idempotent operation - const client = factory.client(key, trafficType) as IClientWithContext; - if ((factory as IFactoryWithClients).sharedClientInstances) { - (factory as IFactoryWithClients).sharedClientInstances.add(client); + const client = (key ? factory.client(key, trafficType) : factory.client()) as IClientWithContext; + if ((factory as IFactoryWithClients).clientInstances) { + (factory as IFactoryWithClients).clientInstances.add(client); } return client; } export function destroySplitFactory(factory: IFactoryWithClients): Promise { // call destroy of shared clients and main one - const destroyPromises = []; - factory.sharedClientInstances.forEach((client) => destroyPromises.push(client.destroy())); - destroyPromises.push(factory.client().destroy()); + const destroyPromises: Promise[] = []; + factory.clientInstances.forEach((client) => destroyPromises.push(client.destroy())); // remove references to release allocated memory - factory.sharedClientInstances.clear(); + factory.clientInstances.clear(); __factories.delete(factory.config); return Promise.all(destroyPromises); } diff --git a/types/utils.d.ts b/types/utils.d.ts index 4a7a503..33615de 100644 --- a/types/utils.d.ts +++ b/types/utils.d.ts @@ -14,12 +14,12 @@ export interface IClientWithContext extends SplitIO.IBrowserClient { * FactoryWithClientInstances interface. */ export interface IFactoryWithClients extends SplitIO.IBrowserSDK { - sharedClientInstances: Set; + clientInstances: Set; config: SplitIO.IBrowserSettings; } export declare const __factories: Map; export declare function getSplitFactory(config: SplitIO.IBrowserSettings): IFactoryWithClients; -export declare function getSplitSharedClient(factory: SplitIO.IBrowserSDK, key: SplitIO.SplitKey, trafficType?: string): IClientWithContext; +export declare function getSplitClient(factory: SplitIO.IBrowserSDK, key?: SplitIO.SplitKey, trafficType?: string): IClientWithContext; export declare function destroySplitFactory(factory: IFactoryWithClients): Promise; export interface IClientStatus { isReady: boolean; From 91e4cc1b99d5541513c8e97f96756d4a1384ad99 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 18 Jul 2023 02:48:03 -0300 Subject: [PATCH 4/8] fix useSplitClient --- src/__tests__/useSplitTreatments.test.tsx | 6 ++++++ src/useSplitClient.ts | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/__tests__/useSplitTreatments.test.tsx b/src/__tests__/useSplitTreatments.test.tsx index a69394d..8ed4a9c 100644 --- a/src/__tests__/useSplitTreatments.test.tsx +++ b/src/__tests__/useSplitTreatments.test.tsx @@ -40,6 +40,8 @@ test('useSplitTreatments', async () => { const user2Client = outerFactory.client('user_2') as any; let countSplitContext = 0, countSplitTreatments = 0, countUseSplitTreatments = 0, countUseSplitTreatmentsUser2 = 0, countUseSplitTreatmentsUser2WithUpdate = 0; + const lastUpdateSetUser2 = new Set(); + const lastUpdateSetUser2WithUpdate = new Set(); render( @@ -61,6 +63,7 @@ test('useSplitTreatments', async () => { const context = useSplitTreatments(['split_test'], undefined, 'user_2'); expect(context.client).toBe(user2Client); validateTreatments(context); + lastUpdateSetUser2.add(context.lastUpdate); countUseSplitTreatmentsUser2++; return null; })} @@ -68,6 +71,7 @@ test('useSplitTreatments', async () => { const context = useSplitTreatments(['split_test'], undefined, 'user_2', { updateOnSdkUpdate: true }); expect(context.client).toBe(user2Client); validateTreatments(context); + lastUpdateSetUser2WithUpdate.add(context.lastUpdate); countUseSplitTreatmentsUser2WithUpdate++; return null; })} @@ -96,8 +100,10 @@ test('useSplitTreatments', async () => { // If useSplitTreatments uses a different client than the context one, it renders when the context renders and when the new client is ready and ready from cache. expect(countUseSplitTreatmentsUser2).toEqual(countSplitContext + 2); + expect(lastUpdateSetUser2.size).toEqual(3); // If it is used with `updateOnSdkUpdate: true`, it also renders when the client emits an SDK_UPDATE event. expect(countUseSplitTreatmentsUser2WithUpdate).toEqual(countSplitContext + 3); + expect(lastUpdateSetUser2WithUpdate.size).toEqual(4); expect(user2Client.getTreatmentsWithConfig).toHaveBeenCalledTimes(5); expect(user2Client.getTreatmentsWithConfig).toHaveBeenLastCalledWith(['split_test'], undefined); }); diff --git a/src/useSplitClient.ts b/src/useSplitClient.ts index 318dc92..02d4151 100644 --- a/src/useSplitClient.ts +++ b/src/useSplitClient.ts @@ -26,8 +26,6 @@ export function useSplitClient(key?: SplitIO.SplitKey, trafficType?: string, att const context = React.useContext(SplitContext); const { client: contextClient, factory } = context; - if (!factory) return context; - let client = contextClient as IClientWithContext; if (key && factory) { client = getSplitClient(factory, key, trafficType); @@ -39,6 +37,8 @@ export function useSplitClient(key?: SplitIO.SplitKey, trafficType?: string, att // Handle client events // NOTE: assuming that SDK events are scattered in time so that Date.now() timestamps are unique per event and trigger an update React.useEffect(() => { + if (!client) return; + const setReady = () => { if (options.updateOnSdkReady) setLastUpdate(Date.now()); } @@ -72,6 +72,6 @@ export function useSplitClient(key?: SplitIO.SplitKey, trafficType?: string, att }, [client]); return { - factory, client, ...getStatus(client), lastUpdate + factory, client, ...getStatus(client), lastUpdate: client === contextClient ? context.lastUpdate : lastUpdate, }; } From 2fdf2f063df4825bcab4d36588670793e817122f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Sat, 22 Jul 2023 01:50:46 -0300 Subject: [PATCH 5/8] added optimization tests --- src/__tests__/SplitTreatments.test.tsx | 48 +++++++++++++---------- src/__tests__/useSplitClient.test.tsx | 4 +- src/__tests__/useSplitTreatments.test.tsx | 23 ++++------- 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/__tests__/SplitTreatments.test.tsx b/src/__tests__/SplitTreatments.test.tsx index ce558e8..c37df09 100644 --- a/src/__tests__/SplitTreatments.test.tsx +++ b/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 }) => ( + + {() => { + renderTimes++; + return null; + }} + + ), + ({ 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 ( - - {() => { - renderTimes++; - return null; - }} - + ); @@ -224,7 +233,7 @@ describe('SplitTreatments optimization', () => { expect(outerFactory.client().getTreatmentsWithConfig).toBeCalledTimes(2); }); - it('rerenders and re-evaluates feature flags if lastUpdate timestamp changes (e.g., SDK_UPDATE event).', (done) => { + it('rerenders and re-evaluates feature flags if lastUpdate timestamp changes (e.g., SDK_UPDATE event).', () => { expect(renderTimes).toBe(1); // State update and split evaluation @@ -234,16 +243,13 @@ describe('SplitTreatments optimization', () => { (outerFactory as any).client().destroy(); wrapper.rerender(); - setTimeout(() => { - // Updates were batched as a single render, due to 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(2); + // Updates were batched as a single render, due to 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(2); - // Restore the client to be READY - (outerFactory as any).client().__restore(); - (outerFactory as any).client().__emitter__.emit(Event.SDK_READY); - done(); - }) + // Restore the client to be READY + (outerFactory as any).client().__restore(); + (outerFactory as any).client().__emitter__.emit(Event.SDK_READY); }); it('rerenders and re-evaluates feature flags if client changes.', () => { diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index d916a1b..19ff057 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -97,10 +97,10 @@ test('useSplitClient must update on SDK events', () => { ); act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); act(() => mainClient.__emitter__.emit(Event.SDK_READY)); - act(() => user2Client.__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)); act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); // SplitContext renders 3 times: initially, when ready from cache, and when ready. diff --git a/src/__tests__/useSplitTreatments.test.tsx b/src/__tests__/useSplitTreatments.test.tsx index 8ed4a9c..3c87720 100644 --- a/src/__tests__/useSplitTreatments.test.tsx +++ b/src/__tests__/useSplitTreatments.test.tsx @@ -34,14 +34,12 @@ 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; let countSplitContext = 0, countSplitTreatments = 0, countUseSplitTreatments = 0, countUseSplitTreatmentsUser2 = 0, countUseSplitTreatmentsUser2WithUpdate = 0; - const lastUpdateSetUser2 = new Set(); - const lastUpdateSetUser2WithUpdate = new Set(); render( @@ -63,7 +61,6 @@ test('useSplitTreatments', async () => { const context = useSplitTreatments(['split_test'], undefined, 'user_2'); expect(context.client).toBe(user2Client); validateTreatments(context); - lastUpdateSetUser2.add(context.lastUpdate); countUseSplitTreatmentsUser2++; return null; })} @@ -71,7 +68,6 @@ test('useSplitTreatments', async () => { const context = useSplitTreatments(['split_test'], undefined, 'user_2', { updateOnSdkUpdate: true }); expect(context.client).toBe(user2Client); validateTreatments(context); - lastUpdateSetUser2WithUpdate.add(context.lastUpdate); countUseSplitTreatmentsUser2WithUpdate++; return null; })} @@ -79,15 +75,12 @@ test('useSplitTreatments', async () => { ); - // Adding a delay between events to make sure they are processed with a different lastUpdate timestamp. - act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); - await new Promise(resolve => setTimeout(resolve, 10)); - act(() => mainClient.__emitter__.emit(Event.SDK_READY)); - act(() => user2Client.__emitter__.emit(Event.SDK_READY)); - await new Promise(resolve => setTimeout(resolve, 10)); - act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); - act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); + await act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + await act(() => mainClient.__emitter__.emit(Event.SDK_READY)); + await act(() => mainClient.__emitter__.emit(Event.SDK_UPDATE)); + await act(() => user2Client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + await act(() => user2Client.__emitter__.emit(Event.SDK_READY)); + await act(() => user2Client.__emitter__.emit(Event.SDK_UPDATE)); // SplitContext renders 3 times: initially, when ready from cache, and when ready. expect(countSplitContext).toEqual(3); @@ -100,10 +93,8 @@ test('useSplitTreatments', async () => { // If useSplitTreatments uses a different client than the context one, it renders when the context renders and when the new client is ready and ready from cache. expect(countUseSplitTreatmentsUser2).toEqual(countSplitContext + 2); - expect(lastUpdateSetUser2.size).toEqual(3); // If it is used with `updateOnSdkUpdate: true`, it also renders when the client emits an SDK_UPDATE event. expect(countUseSplitTreatmentsUser2WithUpdate).toEqual(countSplitContext + 3); - expect(lastUpdateSetUser2WithUpdate.size).toEqual(4); expect(user2Client.getTreatmentsWithConfig).toHaveBeenCalledTimes(5); expect(user2Client.getTreatmentsWithConfig).toHaveBeenLastCalledWith(['split_test'], undefined); }); From 261c038b279fc8dbdb2ee4aa756fce2cd5870144 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Sat, 22 Jul 2023 02:38:11 -0300 Subject: [PATCH 6/8] test polishing --- src/__tests__/SplitTreatments.test.tsx | 10 +++------- src/__tests__/useSplitClient.test.tsx | 5 +++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/__tests__/SplitTreatments.test.tsx b/src/__tests__/SplitTreatments.test.tsx index c37df09..e7145a4 100644 --- a/src/__tests__/SplitTreatments.test.tsx +++ b/src/__tests__/SplitTreatments.test.tsx @@ -252,9 +252,9 @@ describe.each([ (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(); - 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); @@ -262,7 +262,7 @@ describe.each([ 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']; @@ -307,8 +307,6 @@ describe.each([ expect(renderTimesComp1).toBe(2); expect(renderTimesComp2).toBe(2); // updateOnSdkReadyFromCache === false, in second component - // delay SDK events to guarantee a different lastUpdate timestamp for SplitTreatments to re-evaluate - await new Promise(resolve => setTimeout(resolve, 10)); act(() => { (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT); (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT); @@ -317,7 +315,6 @@ describe.each([ expect(renderTimesComp1).toBe(3); expect(renderTimesComp2).toBe(3); - await new Promise(resolve => setTimeout(resolve, 10)); act(() => { (outerFactory as any).client().__emitter__.emit(Event.SDK_READY); (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY); @@ -326,7 +323,6 @@ describe.each([ expect(renderTimesComp1).toBe(3); // updateOnSdkReady === false, in first component expect(renderTimesComp2).toBe(4); - await new Promise(resolve => setTimeout(resolve, 10)); act(() => { (outerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE); (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_UPDATE); diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index 19ff057..4d6e489 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -68,9 +68,10 @@ test('useSplitClient must update on SDK events', () => { {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); From 70c4a97fcce1996fc0e98fa80b374fa87404076a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 14 Sep 2023 11:31:27 -0300 Subject: [PATCH 7/8] Add changelog entry --- CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.txt b/CHANGES.txt index ebc5e62..9ec5191 100644 --- a/CHANGES.txt +++ b/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). From 6d8bd73b1876a78708b2289244e1d70d090245b2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 5 Oct 2023 16:39:38 -0300 Subject: [PATCH 8/8] fix typo --- src/__tests__/useTrack.test.tsx | 24 ++++++++++++------------ src/useTrack.ts | 2 +- types/useTrack.d.ts | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/__tests__/useTrack.test.tsx b/src/__tests__/useTrack.test.tsx index 317af05..11ecce4 100644 --- a/src/__tests__/useTrack.test.tsx +++ b/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( {React.createElement(() => { - bindedTrack = useTrack(); - trackResult = bindedTrack(tt, eventType, value, properties); + boundTrack = useTrack(); + trackResult = boundTrack(tt, eventType, value, properties); return null; })} , @@ -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( {React.createElement(() => { - bindedTrack = useTrack(); - trackResult = bindedTrack(tt, eventType, value, properties); + boundTrack = useTrack(); + trackResult = boundTrack(tt, eventType, value, properties); return null; })} @@ -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( {React.createElement(() => { - bindedTrack = useTrack('user2', tt); - trackResult = bindedTrack(eventType, value, properties); + boundTrack = useTrack('user2', tt); + trackResult = boundTrack(eventType, value, properties); return null; })} , diff --git a/src/useTrack.ts b/src/useTrack.ts index 01fe8a0..95b13df 100644 --- a/src/useTrack.ts +++ b/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'] { diff --git a/types/useTrack.d.ts b/types/useTrack.d.ts index 6479f79..e983136 100644 --- a/types/useTrack.d.ts +++ b/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'];