From c22e8cd12f3c5abd2764c1472287f8608d4d5517 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 17 Nov 2023 10:09:18 -0300 Subject: [PATCH 1/6] Code refactor --- src/SplitClient.tsx | 32 +++++++++------------------ src/__tests__/useSplitClient.test.tsx | 9 ++++++-- src/useSplitClient.ts | 11 ++++----- 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index 9e8f88a..961386a 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -64,36 +64,24 @@ export class SplitComponent extends React.Component { - 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() { diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index 3897182..fe6d228 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -240,19 +240,24 @@ describe('useSplitClient', () => { } const wrapper = render(); + 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(); // 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(); // 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); }); diff --git a/src/useSplitClient.ts b/src/useSplitClient.ts index a0c696c..3c59c6c 100644 --- a/src/useSplitClient.ts +++ b/src/useSplitClient.ts @@ -37,7 +37,8 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV } 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(() => { @@ -47,9 +48,9 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV // 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 && !status.isReady) client.once(client.Event.SDK_READY, update); + if (updateOnSdkReadyFromCache && !status.isReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, update); + if (updateOnSdkTimedout && !status.hasTimedout) client.once(client.Event.SDK_READY_TIMED_OUT, update); if (updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, update); return () => { @@ -62,6 +63,6 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV }, [client, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]); return { - factory, client, ...getStatus(client) + factory, client, ...status }; } From 327a8dad470e45deca17d44fc5f5ffda1190ece6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 17 Nov 2023 14:11:29 -0300 Subject: [PATCH 2/6] Bugfix: hook doesn't re-render when an SDK event is emitted between the hook call and the internal effect execution. --- CHANGES.txt | 3 +++ src/SplitClient.tsx | 19 +++++++++++++++---- src/useSplitClient.ts | 20 +++++++++++++++----- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 053cfd6..be66777 100644 --- a/CHANGES.txt +++ b/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 and the execution of its internal effect. + 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. diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index 961386a..0e735a8 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -63,10 +63,21 @@ export class SplitComponent extends React.Component setLastUpdate(client.lastUpdate); // Subscribe to SDK events - const status = getStatus(client); - if (updateOnSdkReady && !status.isReady) client.once(client.Event.SDK_READY, update); - if (updateOnSdkReadyFromCache && !status.isReadyFromCache) client.once(client.Event.SDK_READY_FROM_CACHE, update); - if (updateOnSdkTimedout && !status.hasTimedout) client.once(client.Event.SDK_READY_TIMED_OUT, update); + const statusOnEffect = getStatus(client); // Effect call is not synchronous, so the status may have changed + + 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 () => { @@ -60,7 +70,7 @@ 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, ...status From 5fea2f47086a10a29ca6e3cd92a087a2c677019d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 17 Nov 2023 14:13:21 -0300 Subject: [PATCH 3/6] prepare rc --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f1c9717..c6c86e1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.0", + "version": "1.10.1-rc.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.0", + "version": "1.10.1-rc.0", "license": "Apache-2.0", "dependencies": { "@splitsoftware/splitio": "10.24.0-beta", diff --git a/package.json b/package.json index 4a497b6..e4d104f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.0", + "version": "1.10.1-rc.0", "description": "A React library to easily integrate and use Split JS SDK", "main": "lib/index.js", "module": "es/index.js", From e37c41ac63cb1fd610415df8170c89ee5cf9d2b7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 21 Nov 2023 12:03:31 -0300 Subject: [PATCH 4/6] prepare stable version --- CHANGES.txt | 2 +- package-lock.json | 4 ++-- package.json | 2 +- src/__tests__/useSplitClient.test.tsx | 2 +- src/useSplitClient.ts | 6 ++++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index be66777..fa41185 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,5 @@ 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 and the execution of its internal effect. + - 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): diff --git a/package-lock.json b/package-lock.json index c6c86e1..3510e55 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.1-rc.0", + "version": "1.10.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.1-rc.0", + "version": "1.10.1", "license": "Apache-2.0", "dependencies": { "@splitsoftware/splitio": "10.24.0-beta", diff --git a/package.json b/package.json index e4d104f..2559917 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.1-rc.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", diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index fe6d228..0dee928 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/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; }; diff --git a/src/useSplitClient.ts b/src/useSplitClient.ts index 5f1b1d1..3ff8562 100644 --- a/src/useSplitClient.ts +++ b/src/useSplitClient.ts @@ -33,6 +33,7 @@ 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); @@ -46,9 +47,10 @@ export function useSplitClient(options?: IUseSplitClientOptions): ISplitContextV const update = () => setLastUpdate(client.lastUpdate); - // Subscribe to SDK events - const statusOnEffect = getStatus(client); // Effect call is not synchronous, so the status may have changed + // Clients are created on the hook's call, so the status may have changed + const statusOnEffect = getStatus(client); + // Subscribe to SDK events if (updateOnSdkReady) { if (!statusOnEffect.isReady) client.once(client.Event.SDK_READY, update); else if (!status.isReady) update(); From af6d0a8455c8569b6697cfa76e3bff1990db8305 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 21 Nov 2023 12:15:36 -0300 Subject: [PATCH 5/6] Add unit test --- src/__tests__/useSplitClient.test.tsx | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index 0dee928..b1000df 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -98,7 +98,7 @@ 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; @@ -219,7 +219,30 @@ describe('useSplitClient', () => { 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( + + {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; + })} + + ) + + expect(count).toEqual(2); + }); + + test('must support changes in update props', () => { const outerFactory = SplitSdk(sdkBrowser); const mainClient = outerFactory.client() as any; From 9595c961d429fb8eee3c70cde84ac0765913476c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 21 Nov 2023 13:00:20 -0300 Subject: [PATCH 6/6] Extend test coverage validating that SDK_READY_TIMED_OUT event is handled --- src/__tests__/useSplitClient.test.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index b1000df..a4a32e8 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -104,7 +104,7 @@ describe('useSplitClient', () => { 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 }} {React.createElement(() => { - useSplitClient({ splitKey: 'user_2', updateOnSdkUpdate: true }); - countUseSplitClientUser2WithUpdate++; + useSplitClient({ splitKey: 'user_2', updateOnSdkTimedout: true }); + countUseSplitClientUser2WithTimeout++; return null; })} @@ -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,7 +215,7 @@ 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); });