diff --git a/CHANGES.txt b/CHANGES.txt index 7f32a73..dd1026b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,9 @@ +1.11.0 (January 15, 2023) + - Added new `SplitFactoryProvider` component as replacement for the now deprecated `SplitFactory` component. + This new component is a fixed version of the `SplitFactory` component, which is not handling the SDK initialization side-effects in the `componentDidMount` and `componentDidUpdate` methods (commit phase), causing some issues like the SDK not being reinitialized when component props change (Related to issue #11 and #148). + The new component also supports server-side rendering. See our documentation for more details: https://help.split.io/hc/en-us/articles/360038825091-React-SDK#server-side-rendering (Related to issue #11 and #109). + - Updated internal code to remove a circular dependency and avoid warning messages with tools like PNPM (Related to issue #176). + 1.10.2 (December 12, 2023) - Updated @splitsoftware/splitio package to version 10.24.1 that updates localStorage usage to clear cached feature flag definitions before initiating the synchronization process, if the cache was previously synchronized with a different SDK key (i.e., a different environment) or different Split Filter criteria, to avoid using invalid cached data when the SDK is ready from cache. diff --git a/src/SplitContext.ts b/src/SplitContext.ts index 9d98c02..812a0b4 100644 --- a/src/SplitContext.ts +++ b/src/SplitContext.ts @@ -18,6 +18,6 @@ export const INITIAL_CONTEXT: ISplitContextValues = { /** * Split Context is the React Context instance that represents our SplitIO global state. * It contains Split SDK objects, such as a factory instance, a client and its status (isReady, isTimedout, lastUpdate) - * The context is created with default empty values, that eventually SplitFactory and SplitClient access and update. + * The context is created with default empty values, that SplitFactoryProvider and SplitClient access and update. */ export const SplitContext = React.createContext(INITIAL_CONTEXT); diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index d3ac78b..4ecfd3c 100644 --- a/src/SplitFactoryProvider.tsx +++ b/src/SplitFactoryProvider.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { SplitComponent } from './SplitClient'; import { ISplitFactoryProps } from './types'; import { WARN_SF_CONFIG_AND_FACTORY } from './constants'; -import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient, getStatus } from './utils'; +import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient, getStatus, __factories } from './utils'; import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; /** @@ -27,24 +27,39 @@ export function SplitFactoryProvider(props: ISplitFactoryProps) { config = undefined; } - const [stateFactory, setStateFactory] = React.useState(propFactory || null); - const factory = propFactory || stateFactory; + const [configFactory, setConfigFactory] = React.useState(null); + const factory = propFactory || (configFactory && config === configFactory.config ? configFactory : null); const client = factory ? getSplitClient(factory) : null; + // Effect to initialize and destroy the factory React.useEffect(() => { if (config) { const factory = getSplitFactory(config); + + return () => { + destroySplitFactory(factory); + } + } + }, [config]); + + // Effect to subscribe/unsubscribe to events + React.useEffect(() => { + const factory = config && __factories.get(config); + if (factory) { const client = getSplitClient(factory); const status = getStatus(client); - // Update state and unsubscribe from events when first event is emitted - const update = () => { + // Unsubscribe from events and update state when first event is emitted + const update = () => { // eslint-disable-next-line no-use-before-define + unsubscribe(); + setConfigFactory(factory); + } + + const unsubscribe = () => { client.off(client.Event.SDK_READY, update); client.off(client.Event.SDK_READY_FROM_CACHE, update); client.off(client.Event.SDK_READY_TIMED_OUT, update); client.off(client.Event.SDK_UPDATE, update); - - setStateFactory(factory); } if (updateOnSdkReady) { @@ -61,10 +76,7 @@ export function SplitFactoryProvider(props: ISplitFactoryProps) { } if (updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, update); - return () => { - // Factory destroy unsubscribes from events - destroySplitFactory(factory as IFactoryWithClients); - } + return unsubscribe; } }, [config, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]); diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx index 2964a60..6745a4d 100644 --- a/src/__tests__/SplitFactoryProvider.test.tsx +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -336,40 +336,95 @@ describe('SplitFactoryProvider', () => { logSpy.mockRestore(); }); - test('cleans up on unmount.', () => { - let destroyMainClientSpy; - let destroySharedClientSpy; - const wrapper = render( - - {({ factory }) => { - if (!factory) return null; // 1st render + test('cleans up on update and unmount if config prop is provided.', () => { + let renderTimes = 0; + const createdFactories = new Set(); + const clientDestroySpies: jest.SpyInstance[] = []; + const outerFactory = SplitSdk(sdkBrowser); + + const Component = ({ factory, isReady, hasTimedout }: ISplitFactoryChildProps) => { + renderTimes++; - // 2nd render (SDK ready) - expect(__factories.size).toBe(1); - destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy'); + switch (renderTimes) { + case 1: + expect(factory).toBe(outerFactory); + return null; + case 2: + case 5: + expect(isReady).toBe(false); + expect(hasTimedout).toBe(false); + expect(factory).toBe(null); + return null; + case 3: + case 4: + case 6: + expect(isReady).toBe(true); + expect(hasTimedout).toBe(true); + expect(factory).not.toBe(null); + createdFactories.add(factory!); + clientDestroySpies.push(jest.spyOn(factory!.client(), 'destroy')); return ( {({ client }) => { - destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + clientDestroySpies.push(jest.spyOn(client!, 'destroy')); return null; }} ); - }} - - ); + case 7: + throw new Error('Must not rerender'); + } + }; - // SDK ready to re-render - act(() => { + const emitSdkEvents = () => { const factory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + factory.client().__emitter__.emit(Event.SDK_READY_TIMED_OUT) factory.client().__emitter__.emit(Event.SDK_READY) - }); + }; + + // 1st render: factory provided + const wrapper = render( + + {Component} + + ); + + // 2nd render: factory created, not ready (null) + wrapper.rerender( + + {Component} + + ); + + // 3rd render: SDK ready (timeout is ignored due to updateOnSdkTimedout=false) + act(emitSdkEvents); + + // 4th render: same config prop -> factory is not recreated + wrapper.rerender( + + {Component} + + ); + + act(emitSdkEvents); // Emitting events again has no effect + expect(createdFactories.size).toBe(1); + + // 5th render: Update config prop -> factory is recreated, not ready yet (null) + wrapper.rerender( + + {Component} + + ); + + // 6th render: SDK timeout (ready is ignored due to updateOnSdkReady=false) + act(emitSdkEvents); wrapper.unmount(); - // the factory created by the component is removed from `factories` cache and its clients are destroyed + + // Created factories are removed from `factories` cache and their clients are destroyed + expect(createdFactories.size).toBe(2); expect(__factories.size).toBe(0); - expect(destroyMainClientSpy).toBeCalledTimes(1); - expect(destroySharedClientSpy).toBeCalledTimes(1); + clientDestroySpies.forEach(spy => expect(spy).toBeCalledTimes(1)); }); test('doesn\'t clean up on unmount if the factory is provided as a prop.', () => { @@ -381,11 +436,11 @@ describe('SplitFactoryProvider', () => { {({ factory }) => { // if factory is provided as a prop, `factories` cache is not modified expect(__factories.size).toBe(0); - destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy'); + destroyMainClientSpy = jest.spyOn(factory!.client(), 'destroy'); return ( {({ client }) => { - destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + destroySharedClientSpy = jest.spyOn(client!, 'destroy'); return null; }} diff --git a/src/__tests__/useSplitClient.test.tsx b/src/__tests__/useSplitClient.test.tsx index a4a32e8..29b894e 100644 --- a/src/__tests__/useSplitClient.test.tsx +++ b/src/__tests__/useSplitClient.test.tsx @@ -11,23 +11,23 @@ import { sdkBrowser } from './testUtils/sdkConfigs'; /** Test target */ import { useSplitClient } from '../useSplitClient'; -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { SplitClient } from '../SplitClient'; import { SplitContext } from '../SplitContext'; import { testAttributesBinding, TestComponentProps } from './testUtils/utils'; describe('useSplitClient', () => { - test('returns the main client from the context updated by SplitFactory.', () => { + test('returns the main client from the context updated by SplitFactoryProvider.', () => { const outerFactory = SplitSdk(sdkBrowser); let client; render( - + {React.createElement(() => { client = useSplitClient().client; return null; })} - + ); expect(client).toBe(outerFactory.client()); }); @@ -36,14 +36,14 @@ describe('useSplitClient', () => { const outerFactory = SplitSdk(sdkBrowser); let client; render( - + {React.createElement(() => { client = useSplitClient().client; return null; })} - + ); expect(client).toBe(outerFactory.client('user2')); }); @@ -52,13 +52,13 @@ describe('useSplitClient', () => { const outerFactory = SplitSdk(sdkBrowser); let client; render( - + {React.createElement(() => { (outerFactory.client as jest.Mock).mockClear(); client = useSplitClient({ splitKey: 'user2', trafficType: 'user' }).client; return null; })} - + ); expect(outerFactory.client as jest.Mock).toBeCalledWith('user2', 'user'); expect(outerFactory.client as jest.Mock).toHaveReturnedWith(client); @@ -89,9 +89,9 @@ describe('useSplitClient', () => { function Component({ attributesFactory, attributesClient, splitKey, testSwitch, factory }: TestComponentProps) { return ( - + - + ); } @@ -108,13 +108,13 @@ describe('useSplitClient', () => { let countNestedComponent = 0; render( - + <> {() => countSplitContext++} {() => { countSplitClient++; return null }} @@ -122,7 +122,7 @@ describe('useSplitClient', () => { {React.createElement(() => { // Equivalent to // - Using config key and traffic type: `const { client } = useSplitClient(sdkBrowser.core.key, sdkBrowser.core.trafficType, { att1: 'att1' });` - // - Disabling update props, since the wrapping SplitFactory has them enabled: `const { client } = useSplitClient(undefined, undefined, { att1: 'att1' }, { updateOnSdkReady: false, updateOnSdkReadyFromCache: false });` + // - Disabling update props, since the wrapping SplitFactoryProvider has them enabled: `const { client } = useSplitClient(undefined, undefined, { att1: 'att1' }, { updateOnSdkReady: false, updateOnSdkReadyFromCache: false });` const { client } = useSplitClient({ attributes: { att1: 'att1' } }); expect(client).toBe(mainClient); // Assert that the main client was retrieved. expect(client!.getAttributes()).toEqual({ att1: 'att1' }); // Assert that the client was retrieved with the provided attributes. @@ -183,7 +183,7 @@ describe('useSplitClient', () => { })} - + ); act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); @@ -226,7 +226,7 @@ describe('useSplitClient', () => { let count = 0; render( - + {React.createElement(() => { useSplitClient({ splitKey: 'some_user' }); count++; @@ -237,7 +237,7 @@ describe('useSplitClient', () => { return null; })} - + ) expect(count).toEqual(2); @@ -257,9 +257,9 @@ describe('useSplitClient', () => { function Component(updateOptions) { return ( - + - + ) } diff --git a/src/__tests__/useSplitManager.test.tsx b/src/__tests__/useSplitManager.test.tsx index 9ec5367..4705a48 100644 --- a/src/__tests__/useSplitManager.test.tsx +++ b/src/__tests__/useSplitManager.test.tsx @@ -11,7 +11,7 @@ import { sdkBrowser } from './testUtils/sdkConfigs'; import { getStatus } from '../utils'; /** Test target */ -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { useSplitManager } from '../useSplitManager'; describe('useSplitManager', () => { @@ -20,12 +20,12 @@ describe('useSplitManager', () => { const outerFactory = SplitSdk(sdkBrowser); let hookResult; render( - + {React.createElement(() => { hookResult = useSplitManager(); return null; })} - + ); expect(hookResult).toStrictEqual({ diff --git a/src/__tests__/useSplitTreatments.test.tsx b/src/__tests__/useSplitTreatments.test.tsx index cbdc62d..13a54b6 100644 --- a/src/__tests__/useSplitTreatments.test.tsx +++ b/src/__tests__/useSplitTreatments.test.tsx @@ -11,7 +11,7 @@ import { sdkBrowser } from './testUtils/sdkConfigs'; import { CONTROL_WITH_CONFIG } from '../constants'; /** Test target */ -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { SplitClient } from '../SplitClient'; import { useSplitTreatments } from '../useSplitTreatments'; import { SplitTreatments } from '../SplitTreatments'; @@ -33,7 +33,7 @@ describe('useSplitTreatments', () => { let treatmentsByFlagSets: SplitIO.TreatmentsWithConfig; render( - + {React.createElement(() => { treatments = useSplitTreatments({ names: featureFlagNames, attributes }).treatments; treatmentsByFlagSets = useSplitTreatments({ flagSets, attributes }).treatments; @@ -42,7 +42,7 @@ describe('useSplitTreatments', () => { expect(useSplitTreatments({}).treatments).toEqual({}); return null; })} - + ); // returns control treatment if not operational (SDK not ready or destroyed), without calling `getTreatmentsWithConfig` method @@ -69,14 +69,14 @@ describe('useSplitTreatments', () => { let treatments: SplitIO.TreatmentsWithConfig; render( - + {React.createElement(() => { treatments = useSplitTreatments({ names: featureFlagNames, attributes }).treatments; return null; })} - + ); // returns control treatment if not operational (SDK not ready or destroyed), without calling `getTreatmentsWithConfig` method @@ -96,7 +96,7 @@ describe('useSplitTreatments', () => { let renderTimes = 0; render( - + {React.createElement(() => { const treatments = useSplitTreatments({ names: featureFlagNames, attributes, splitKey: 'user2' }).treatments; @@ -119,7 +119,7 @@ describe('useSplitTreatments', () => { return null; })} - + ); act(() => client.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); @@ -189,7 +189,7 @@ describe('useSplitTreatments', () => { } render( - + <> {() => countSplitContext++} @@ -219,7 +219,7 @@ describe('useSplitTreatments', () => { return null; })} - + ); act(() => mainClient.__emitter__.emit(Event.SDK_READY_FROM_CACHE)); diff --git a/src/__tests__/useTrack.test.tsx b/src/__tests__/useTrack.test.tsx index 11ecce4..4447189 100644 --- a/src/__tests__/useTrack.test.tsx +++ b/src/__tests__/useTrack.test.tsx @@ -10,7 +10,7 @@ import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client'; import { sdkBrowser } from './testUtils/sdkConfigs'; /** Test target */ -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { SplitClient } from '../SplitClient'; import { useTrack } from '../useTrack'; @@ -21,19 +21,19 @@ describe('useTrack', () => { const value = 10; const properties = { prop1: 'prop1' }; - test('returns the track method bound to the client at Split context updated by SplitFactory.', () => { + test('returns the track method bound to the client at Split context updated by SplitFactoryProvider.', () => { const outerFactory = SplitSdk(sdkBrowser); let boundTrack; let trackResult; render( - + {React.createElement(() => { boundTrack = useTrack(); trackResult = boundTrack(tt, eventType, value, properties); return null; })} - , + , ); const track = outerFactory.client().track as jest.Mock; expect(track).toBeCalledWith(tt, eventType, value, properties); @@ -46,7 +46,7 @@ describe('useTrack', () => { let trackResult; render( - + {React.createElement(() => { boundTrack = useTrack(); @@ -54,7 +54,7 @@ describe('useTrack', () => { return null; })} - + ); const track = outerFactory.client('user2').track as jest.Mock; expect(track).toBeCalledWith(tt, eventType, value, properties); @@ -67,13 +67,13 @@ describe('useTrack', () => { let trackResult; render( - + {React.createElement(() => { boundTrack = useTrack('user2', tt); trackResult = boundTrack(eventType, value, properties); return null; })} - , + , ); const track = outerFactory.client('user2', tt).track as jest.Mock; expect(track).toBeCalledWith(eventType, value, properties); diff --git a/src/__tests__/withSplitFactory.test.tsx b/src/__tests__/withSplitFactory.test.tsx index 32813b0..6df93f6 100644 --- a/src/__tests__/withSplitFactory.test.tsx +++ b/src/__tests__/withSplitFactory.test.tsx @@ -15,7 +15,7 @@ jest.mock('../SplitFactory'); import { ISplitFactoryChildProps } from '../types'; import { withSplitFactory } from '../withSplitFactory'; -describe('SplitFactory', () => { +describe('withSplitFactory', () => { test('passes no-ready props to the child if initialized with a no ready factory (e.g., using config object).', () => { const Component = withSplitFactory(sdkBrowser)( diff --git a/src/types.ts b/src/types.ts index 83d4b26..5fc7cac 100644 --- a/src/types.ts +++ b/src/types.ts @@ -45,12 +45,17 @@ export interface ISplitStatus { export interface ISplitContextValues extends ISplitStatus { /** - * Split factory instance + * Split factory instance. + * + * NOTE: This property is not recommended for direct use, as better alternatives are available. */ factory: SplitIO.IBrowserSDK | null; /** - * Split client instance + * Split client instance. + * + * NOTE: This property is not recommended for direct use, as better alternatives are available. + * * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#2-instantiate-the-sdk-and-create-a-new-split-client} */ client: SplitIO.IBrowserClient | null; @@ -94,14 +99,14 @@ export interface IUpdateProps { } /** - * SplitFactory Child Props interface. These are the props that the child component receives from the 'SplitFactory' component. + * SplitFactoryProvider Child Props interface. These are the props that the child component receives from the 'SplitFactoryProvider' component. */ -// @TODO remove next type (breaking-change) +// @TODO rename/remove next type (breaking-change) // eslint-disable-next-line @typescript-eslint/no-empty-interface export interface ISplitFactoryChildProps extends ISplitContextValues { } /** - * SplitFactory Props interface. These are the props accepted by SplitFactory component, + * SplitFactoryProvider Props interface. These are the props accepted by SplitFactoryProvider component, * used to instantiate a factory and client instance, update the Split context, and listen for SDK events. */ export interface ISplitFactoryProps extends IUpdateProps { @@ -114,6 +119,8 @@ export interface ISplitFactoryProps extends IUpdateProps { /** * Split factory instance to use instead of creating a new one with the config object. + * + * If both `factory` and `config` are provided, the `config` option is ignored. */ factory?: SplitIO.IBrowserSDK; @@ -123,7 +130,7 @@ export interface ISplitFactoryProps extends IUpdateProps { attributes?: SplitIO.Attributes; /** - * Children of the SplitFactory component. It can be a functional component (child as a function) or a React element. + * Children of the SplitFactoryProvider component. It can be a functional component (child as a function) or a React element. */ children: ((props: ISplitFactoryChildProps) => ReactNode) | ReactNode; } @@ -165,7 +172,7 @@ export interface ISplitClientChildProps extends ISplitContextValues { } export interface ISplitClientProps extends IUseSplitClientOptions { /** - * Children of the SplitFactory component. It can be a functional component (child as a function) or a React element. + * Children of the SplitClient component. It can be a functional component (child as a function) or a React element. */ children: ((props: ISplitClientChildProps) => ReactNode) | ReactNode; } diff --git a/src/useClient.ts b/src/useClient.ts index 535eb98..7428b60 100644 --- a/src/useClient.ts +++ b/src/useClient.ts @@ -3,9 +3,9 @@ import { useSplitClient } from './useSplitClient'; /** * 'useClient' is a hook that returns a client from the Split context. * It uses the 'useContext' hook to access the context, which is updated by - * SplitFactory and SplitClient components in the hierarchy of components. + * SplitFactoryProvider and SplitClient components in the hierarchy of components. * - * @returns A Split Client instance, or null if used outside the scope of SplitFactory + * @returns A Split Client instance, or null if used outside the scope of SplitFactoryProvider or factory is not ready. * * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#advanced-instantiate-multiple-sdk-clients} * diff --git a/src/useManager.ts b/src/useManager.ts index 9ba101b..094ec99 100644 --- a/src/useManager.ts +++ b/src/useManager.ts @@ -3,9 +3,9 @@ import { useSplitManager } from './useSplitManager'; /** * 'useManager' is a hook that returns the Manager instance from the Split factory. * It uses the 'useContext' hook to access the factory at Split context, which is updated by - * the SplitFactory component. + * the SplitFactoryProvider component. * - * @returns A Split Manager instance, or null if used outside the scope of SplitFactory + * @returns A Split Manager instance, or null if used outside the scope of SplitFactoryProvider or factory is not ready. * * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#manager} * diff --git a/src/useSplitClient.ts b/src/useSplitClient.ts index 3ff8562..5428aa7 100644 --- a/src/useSplitClient.ts +++ b/src/useSplitClient.ts @@ -12,7 +12,7 @@ export const DEFAULT_UPDATE_OPTIONS = { /** * 'useSplitClient' is a hook that returns an Split Context object with the client and its status corresponding to the provided key and trafficType. - * It uses the 'useContext' hook to access the context, which is updated by SplitFactory and SplitClient components in the hierarchy of components. + * It uses the 'useContext' hook to access the context, which is updated by SplitFactoryProvider and SplitClient components in the hierarchy of components. * * @returns A Split Context object * diff --git a/src/useSplitManager.ts b/src/useSplitManager.ts index 1ba6a23..13aa9b1 100644 --- a/src/useSplitManager.ts +++ b/src/useSplitManager.ts @@ -4,9 +4,9 @@ import { ISplitContextValues } from './types'; /** * 'useSplitManager' is a hook that returns an Split Context object with the Manager instance from the Split factory. - * It uses the 'useContext' hook to access the factory at Split context, which is updated by the SplitFactory component. + * It uses the 'useContext' hook to access the factory at Split context, which is updated by the SplitFactoryProvider component. * - * @returns An object containing the Split context and the Split Manager instance, which is null if used outside the scope of SplitFactory + * @returns An object containing the Split context and the Split Manager instance, which is null if used outside the scope of SplitFactoryProvider or factory is not ready. * * @example * ```js @@ -16,7 +16,7 @@ import { ISplitContextValues } from './types'; * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#manager} */ export function useSplitManager(): ISplitContextValues & { manager: SplitIO.IManager | null } { - // Update options are not supported, because updates can be controlled at the SplitFactory component. + // Update options are not supported, because updates can be controlled at the SplitFactoryProvider component. const context = React.useContext(SplitContext); return { ...context,