Skip to content

Commit

Permalink
Refactor context value to use undefined rather than null for factory …
Browse files Browse the repository at this point in the history
…and client properties
  • Loading branch information
EmilianoSanchez committed Oct 21, 2024
1 parent 6d65bf0 commit e8e0d2a
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 58 deletions.
6 changes: 3 additions & 3 deletions src/SplitFactoryProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ export function SplitFactoryProvider(props: ISplitFactoryProviderProps) {
config = undefined;
}

const [configFactory, setConfigFactory] = React.useState<IFactoryWithClients | null>(null);
const [configFactory, setConfigFactory] = React.useState<IFactoryWithClients>();
const factory = React.useMemo(() => {
return propFactory || (configFactory && config === configFactory.config ? configFactory : null);
return propFactory || (configFactory && config === configFactory.config ? configFactory : undefined);
}, [config, propFactory, configFactory]);
const client = factory ? getSplitClient(factory) : null;
const client = factory ? getSplitClient(factory) : undefined;

// Effect to initialize and destroy the factory
React.useEffect(() => {
Expand Down
43 changes: 20 additions & 23 deletions src/__tests__/SplitClient.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ISplitClientChildProps } from '../types';
import { SplitFactoryProvider } from '../SplitFactoryProvider';
import { SplitClient } from '../SplitClient';
import { SplitContext } from '../SplitContext';
import { testAttributesBinding, TestComponentProps } from './testUtils/utils';
import { INITIAL_CONTEXT, testAttributesBinding, TestComponentProps } from './testUtils/utils';
import { IClientWithContext } from '../utils';
import { EXCEPTION_NO_SFP } from '../constants';

Expand All @@ -24,13 +24,8 @@ describe('SplitClient', () => {
render(
<SplitFactoryProvider config={sdkBrowser} >
<SplitClient splitKey='user1' >
{({ isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => {
expect(isReady).toBe(false);
expect(isReadyFromCache).toBe(false);
expect(hasTimedout).toBe(false);
expect(isTimedout).toBe(false);
expect(isDestroyed).toBe(false);
expect(lastUpdate).toBe(0);
{(childProps: ISplitClientChildProps) => {
expect(childProps).toEqual(INITIAL_CONTEXT);

return null;
}}
Expand All @@ -50,14 +45,15 @@ describe('SplitClient', () => {
<SplitFactoryProvider factory={outerFactory} >
{/* Equivalent to <SplitClient splitKey={undefined} > */}
<SplitClient splitKey={sdkBrowser.core.key} >
{({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => {
expect(client).toBe(outerFactory.client());
expect(isReady).toBe(true);
expect(isReadyFromCache).toBe(true);
expect(hasTimedout).toBe(false);
expect(isTimedout).toBe(false);
expect(isDestroyed).toBe(false);
expect(lastUpdate).toBe((outerFactory.client() as IClientWithContext).__getStatus().lastUpdate);
{(childProps: ISplitClientChildProps) => {
expect(childProps).toEqual({
...INITIAL_CONTEXT,
factory : outerFactory,
client: outerFactory.client(),
isReady: true,
isReadyFromCache: true,
lastUpdate: (outerFactory.client() as IClientWithContext).__getStatus().lastUpdate
});

return null;
}}
Expand Down Expand Up @@ -162,7 +158,7 @@ describe('SplitClient', () => {
expect(renderTimes).toBe(3);
});

test('rerender child only on SDK_READY event, as default behaviour.', async () => {
test('rerender child only on SDK_READY event, as default behavior.', async () => {
const outerFactory = SplitFactory(sdkBrowser);
(outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
(outerFactory.manager().names as jest.Mock).mockReturnValue(['split1']);
Expand Down Expand Up @@ -228,18 +224,19 @@ describe('SplitClient', () => {
expect(count).toEqual(2);
});

test('renders a passed JSX.Element with a new SplitContext value.', (done) => {
test('renders a passed JSX.Element with a new SplitContext value.', () => {
const outerFactory = SplitFactory(sdkBrowser);

const Component = () => {
return (
<SplitContext.Consumer>
{(value) => {
expect(value.client).toBe(outerFactory.client('user2'));
expect(value.isReady).toBe(false);
expect(value.isTimedout).toBe(false);
expect(value.lastUpdate).toBe(0);
done();
expect(value).toEqual({
...INITIAL_CONTEXT,
factory: outerFactory,
client: outerFactory.client('user2'),
});

return null;
}}
</SplitContext.Consumer>
Expand Down
38 changes: 16 additions & 22 deletions src/__tests__/SplitFactoryProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,8 @@ describe('SplitFactoryProvider', () => {
test('passes no-ready props to the child if initialized with a config.', () => {
render(
<SplitFactoryProvider config={sdkBrowser} >
{({ factory, client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryProviderChildProps) => {
expect(factory).toBe(null);
expect(client).toBe(null);
expect(isReady).toBe(false);
expect(isReadyFromCache).toBe(false);
expect(hasTimedout).toBe(false);
expect(isTimedout).toBe(false);
expect(isDestroyed).toBe(false);
expect(lastUpdate).toBe(0);
{(childProps: ISplitFactoryProviderChildProps) => {
expect(childProps).toEqual(INITIAL_CONTEXT);
return null;
}}
</SplitFactoryProvider>
Expand All @@ -48,15 +41,16 @@ describe('SplitFactoryProvider', () => {

render(
<SplitFactoryProvider factory={outerFactory} >
{({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryProviderChildProps) => {
expect(factory).toBe(outerFactory);
expect(isReady).toBe(true);
expect(isReadyFromCache).toBe(true);
expect(hasTimedout).toBe(false);
expect(isTimedout).toBe(false);
expect(isDestroyed).toBe(false);
expect(lastUpdate).toBe((outerFactory.client() as IClientWithContext).__getStatus().lastUpdate);
expect((factory as SplitIO.IBrowserSDK).settings.version).toBe(outerFactory.settings.version);
{(childProps: ISplitFactoryProviderChildProps) => {
expect(childProps).toEqual({
...INITIAL_CONTEXT,
factory: outerFactory,
client: outerFactory.client(),
isReady: true,
isReadyFromCache: true,
lastUpdate: (outerFactory.client() as IClientWithContext).__getStatus().lastUpdate
});
expect((childProps.factory as SplitIO.IBrowserSDK).settings.version).toBe(outerFactory.settings.version);
return null;
}}
</SplitFactoryProvider>
Expand Down Expand Up @@ -229,7 +223,7 @@ describe('SplitFactoryProvider', () => {
expect(renderTimes).toBe(3);
});

test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (config prop)', async () => {
test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behavior (config prop)', async () => {
let renderTimes = 0;
let previousLastUpdate = -1;

Expand Down Expand Up @@ -263,7 +257,7 @@ describe('SplitFactoryProvider', () => {
expect(renderTimes).toBe(2);
});

test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (factory prop)', async () => {
test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behavior (factory prop)', async () => {
const outerFactory = SplitFactory(sdkBrowser);
let renderTimes = 0;
let previousLastUpdate = -1;
Expand Down Expand Up @@ -350,14 +344,14 @@ describe('SplitFactoryProvider', () => {
case 5:
expect(isReady).toBe(false);
expect(hasTimedout).toBe(false);
expect(factory).toBe(null);
expect(factory).toBe(undefined);
return null;
case 3:
case 4:
case 6:
expect(isReady).toBe(true);
expect(hasTimedout).toBe(true);
expect(factory).not.toBe(null);
expect(factory).not.toBe(undefined);
createdFactories.add(factory!);
clientDestroySpies.push(jest.spyOn(factory!.client(), 'destroy'));
return (
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/testUtils/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ export function testAttributesBinding(Component: React.FunctionComponent<TestCom
}

export const INITIAL_CONTEXT: ISplitContextValues = {
client: null,
factory: null,
client: undefined,
factory: undefined,
isReady: false,
isReadyFromCache: false,
isTimedout: false,
Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface ISplitContextValues extends ISplitStatus {
*
* NOTE: This property is not recommended for direct use, as better alternatives are available.
*/
factory: SplitIO.IBrowserSDK | null;
factory?: SplitIO.IBrowserSDK;

/**
* Split client instance.
Expand All @@ -59,7 +59,7 @@ export interface ISplitContextValues extends ISplitStatus {
*
* @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;
client?: SplitIO.IBrowserClient;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/useSplitManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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 SplitFactoryProvider component.
*
* @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.
* @returns An object containing the Split context and the Split Manager instance, which is undefined if factory is not ready.
*
* @example
* ```js
Expand All @@ -14,11 +14,11 @@ 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 } {
export function useSplitManager(): ISplitContextValues & { manager?: SplitIO.IManager } {
// Update options are not supported, because updates can be controlled at the SplitFactoryProvider component.
const context = useSplitContext();
return {
...context,
manager: context.factory ? context.factory.manager() : null
manager: context.factory ? context.factory.manager() : undefined
};
}
6 changes: 3 additions & 3 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function destroySplitFactory(factory: IFactoryWithClients): Promise<void>

// Util used to get client status.
// It might be removed in the future, if the JS SDK extends its public API with a `getStatus` method
export function getStatus(client: SplitIO.IBrowserClient | null): ISplitStatus {
export function getStatus(client?: SplitIO.IBrowserClient): ISplitStatus {
const status = client && (client as IClientWithContext).__getStatus();

return {
Expand All @@ -80,7 +80,7 @@ export function getStatus(client: SplitIO.IBrowserClient | null): ISplitStatus {
/**
* Manage client attributes binding
*/
export function initAttributes(client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes) {
export function initAttributes(client?: SplitIO.IBrowserClient, attributes?: SplitIO.Attributes) {
if (client && attributes) client.setAttributes(attributes);
}

Expand Down Expand Up @@ -174,7 +174,7 @@ function argsAreEqual(newArgs: any[], lastArgs: any[]): boolean {
shallowEqual(newArgs[5], lastArgs[5]); // flagSets
}

function evaluateFeatureFlags(client: SplitIO.IBrowserClient | null, _lastUpdate: number, names?: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes, flagSets?: string[]) {
function evaluateFeatureFlags(client: SplitIO.IBrowserClient | undefined, _lastUpdate: number, names?: SplitIO.SplitNames, attributes?: SplitIO.Attributes, _clientAttributes?: SplitIO.Attributes, flagSets?: string[]) {
if (names && flagSets) console.log(WARN_NAMES_AND_FLAGSETS);

return client && (client as IClientWithContext).__getStatus().isOperational && (names || flagSets) ?
Expand Down

0 comments on commit e8e0d2a

Please sign in to comment.