diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index 9253ce8..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 && config === (stateFactory as IFactoryWithClients).config ? stateFactory : null); + 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 a77d4af..6745a4d 100644 --- a/src/__tests__/SplitFactoryProvider.test.tsx +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -336,39 +336,43 @@ describe('SplitFactoryProvider', () => { logSpy.mockRestore(); }); - test('cleans up on update and unmount.', () => { + 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++; - if (factory) createdFactories.add(factory); switch (renderTimes) { case 1: - case 3: + 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 2: + case 3: case 4: + case 6: expect(isReady).toBe(true); expect(hasTimedout).toBe(true); - expect(__factories.size).toBe(1); - clientDestroySpies.push(jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy')); + expect(factory).not.toBe(null); + createdFactories.add(factory!); + clientDestroySpies.push(jest.spyOn(factory!.client(), 'destroy')); return ( {({ client }) => { - clientDestroySpies.push(jest.spyOn(client as SplitIO.IClient, 'destroy')); + clientDestroySpies.push(jest.spyOn(client!, 'destroy')); return null; }} ); - case 5: - throw new Error('Child must not be rerendered'); + case 7: + throw new Error('Must not rerender'); } }; @@ -378,24 +382,41 @@ describe('SplitFactoryProvider', () => { factory.client().__emitter__.emit(Event.SDK_READY) }; - // 1st render + // 1st render: factory provided const wrapper = render( + + {Component} + + ); + + // 2nd render: factory created, not ready (null) + wrapper.rerender( {Component} ); - // 2nd render: SDK ready (timeout is ignored due to updateOnSdkTimedout=false) + // 3rd render: SDK ready (timeout is ignored due to updateOnSdkTimedout=false) act(emitSdkEvents); - // 3rd render: Update config prop -> factory is recreated + // 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} ); - // 4th render: SDK timeout (ready is ignored due to updateOnSdkReady=false) + // 6th render: SDK timeout (ready is ignored due to updateOnSdkReady=false) act(emitSdkEvents); wrapper.unmount(); @@ -415,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; }}