diff --git a/.github/workflows/ci.yml b/.github/workflows/ci-cd.yml similarity index 98% rename from .github/workflows/ci.yml rename to .github/workflows/ci-cd.yml index bc3a8b1..3870a1f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci-cd.yml @@ -23,14 +23,14 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 - name: Set up nodejs uses: actions/setup-node@v3 with: - node-version: '16.16.0' + node-version: 'lts/*' cache: 'npm' - name: npm ci diff --git a/.github/workflows/update-license-year.yml b/.github/workflows/update-license-year.yml index 13aaac8..7e0a945 100644 --- a/.github/workflows/update-license-year.yml +++ b/.github/workflows/update-license-year.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 @@ -24,7 +24,7 @@ jobs: run: "echo PREVIOUS=$(($CURRENT-1)) >> $GITHUB_ENV" - name: Update LICENSE - uses: jacobtomlinson/gha-find-replace@v2 + uses: jacobtomlinson/gha-find-replace@v3 with: find: ${{ env.PREVIOUS }} replace: ${{ env.CURRENT }} @@ -38,7 +38,7 @@ jobs: git commit -m "Updated License Year" -a - name: Create Pull Request - uses: peter-evans/create-pull-request@v3 + uses: peter-evans/create-pull-request@v5 with: token: ${{ secrets.GITHUB_TOKEN }} title: Update License Year diff --git a/.nvmrc b/.nvmrc index f274881..b009dfb 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -v16.16.0 +lts/* diff --git a/CHANGES.txt b/CHANGES.txt index 8d31f0a..3ece218 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,10 @@ +1.11.0 (January 16, 2023) + - Added the new `SplitFactoryProvider` component as a replacement for the now deprecated `SplitFactory` component. + The new component is a revised version of `SplitFactory`, addressing improper handling of the SDK initialization side-effects in the `componentDidMount` and `componentDidUpdate` methods (commit phase), causing some issues like memory leaks and the SDK not reinitializing when component props change (Related to issue #11 and #148). + The `SplitFactoryProvider` component can be used as a drop-in replacement for `SplitFactory`. It utilizes the React Hooks API, that requires React 16.8.0 or later, and supports server-side rendering. See our documentation for more details (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). + - Updated @splitsoftware/splitio package to version 10.25.1 for vulnerability fixes. + 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. @@ -15,6 +22,7 @@ - `useSplitTreatments` optimizes feature flag evaluations by using the `useMemo` hook to memoize `getTreatmentsWithConfig` method calls from the SDK. This avoids re-evaluating feature flags when the hook is called with the same options and the feature flag definitions have not changed. - They fixed a bug in the deprecated `useClient` and `useTreatments` hooks, which caused them to not re-render and re-evaluate feature flags when they access a different SDK client than the context and its status updates (i.e., when it emits SDK_READY or other event). - Added TypeScript types and interfaces to the library index exports, allowing them to be imported from the library index, e.g., `import type { ISplitFactoryProps } from '@splitsoftware/splitio-react'` (Related to issue https://github.com/splitio/react-client/issues/162). + - Added `defaultTreatment` property to the `SplitView` object returned by the `split` and `splits` methods of the SDK manager (Related to issue https://github.com/splitio/javascript-commons/issues/225). - Updated type declarations of the library components to not restrict the type of the `children` prop to ReactElement, allowing to pass any valid ReactNode value (Related to issue https://github.com/splitio/react-client/issues/164). - Updated linter and other dependencies for vulnerability fixes. - Bugfixing - Removed conditional code within hooks to adhere to the rules of hooks and prevent React warnings. 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. diff --git a/LICENSE b/LICENSE index 65f5999..c022e92 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright © 2023 Split Software, Inc. +Copyright © 2024 Split Software, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/README.md b/README.md index a171ed1..53c4b7f 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Below is a simple example that describes the instantiation and most basic usage import React from 'react'; // Import SDK functions -import { SplitFactory, useSplitTreatments } from '@splitsoftware/splitio-react'; +import { SplitFactoryProvider, useSplitTreatments } from '@splitsoftware/splitio-react'; // Define your config object const CONFIG = { @@ -48,10 +48,10 @@ function MyComponent() { function MyApp() { return ( - // Use SplitFactory to instantiate the SDK and makes it available to nested components - + // Use SplitFactoryProvider to instantiate the SDK and makes it available to nested components + - + ); } ``` diff --git a/package-lock.json b/package-lock.json index 6e79190..377cab9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.11.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.11.0", "license": "Apache-2.0", "dependencies": { - "@splitsoftware/splitio": "10.24.1", + "@splitsoftware/splitio": "10.25.1", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, @@ -1547,17 +1547,17 @@ } }, "node_modules/@splitsoftware/splitio": { - "version": "10.24.1", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.24.1.tgz", - "integrity": "sha512-WzVZrP2IAqzNBywNXgmLxiS60qumkcnu6u1lUPlNgdVek82TzWeqyqW+htKmDMJ/ifsJPWrgT1VLMZJvOnBsVA==", + "version": "10.25.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1.tgz", + "integrity": "sha512-QmCOI2VNhjIMieibtMcc595oyQB5sgrYKSqw7wxKtwMX0VtuPbZ3Lw8fwd0nH2WSKq3Pcjyu3nVSYQRp1bGEvA==", "dependencies": { - "@splitsoftware/splitio-commons": "1.12.1", + "@splitsoftware/splitio-commons": "1.13.1", "@types/google.analytics": "0.0.40", "@types/ioredis": "^4.28.0", "bloom-filters": "^3.0.0", "ioredis": "^4.28.0", "js-yaml": "^3.13.1", - "node-fetch": "^2.6.7", + "node-fetch": "^2.7.0", "unfetch": "^4.2.0" }, "engines": { @@ -1569,9 +1569,9 @@ } }, "node_modules/@splitsoftware/splitio-commons": { - "version": "1.12.1", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio-commons/-/splitio-commons-1.12.1.tgz", - "integrity": "sha512-EkCcqlYvVafazs9c5i+pmhf6rIyj3A70dqQ4U3BKE646t7tf6mxGzqZz1sAl540xNyYI7CA/iIqisEWvDtJc0A==", + "version": "1.13.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio-commons/-/splitio-commons-1.13.1.tgz", + "integrity": "sha512-xGu94sLx+tJb6PeM26vH8/LEElsaVbh2BjoLvL5twR4gKsVezie5ZtHhejWT1+iCVCtJuhjZxKwOm4HGYoVIHQ==", "dependencies": { "tslib": "^2.3.1" }, @@ -8353,9 +8353,9 @@ "dev": true }, "node_modules/node-fetch": { - "version": "2.6.7", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.7.tgz", - "integrity": "sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "dependencies": { "whatwg-url": "^5.0.0" }, @@ -12089,25 +12089,25 @@ } }, "@splitsoftware/splitio": { - "version": "10.24.1", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.24.1.tgz", - "integrity": "sha512-WzVZrP2IAqzNBywNXgmLxiS60qumkcnu6u1lUPlNgdVek82TzWeqyqW+htKmDMJ/ifsJPWrgT1VLMZJvOnBsVA==", + "version": "10.25.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1.tgz", + "integrity": "sha512-QmCOI2VNhjIMieibtMcc595oyQB5sgrYKSqw7wxKtwMX0VtuPbZ3Lw8fwd0nH2WSKq3Pcjyu3nVSYQRp1bGEvA==", "requires": { - "@splitsoftware/splitio-commons": "1.12.1", + "@splitsoftware/splitio-commons": "1.13.1", "@types/google.analytics": "0.0.40", "@types/ioredis": "^4.28.0", "bloom-filters": "^3.0.0", "eventsource": "^1.1.2", "ioredis": "^4.28.0", "js-yaml": "^3.13.1", - "node-fetch": "^2.6.7", + "node-fetch": "^2.7.0", "unfetch": "^4.2.0" } }, "@splitsoftware/splitio-commons": { - "version": "1.12.1", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio-commons/-/splitio-commons-1.12.1.tgz", - "integrity": "sha512-EkCcqlYvVafazs9c5i+pmhf6rIyj3A70dqQ4U3BKE646t7tf6mxGzqZz1sAl540xNyYI7CA/iIqisEWvDtJc0A==", + "version": "1.13.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio-commons/-/splitio-commons-1.13.1.tgz", + "integrity": "sha512-xGu94sLx+tJb6PeM26vH8/LEElsaVbh2BjoLvL5twR4gKsVezie5ZtHhejWT1+iCVCtJuhjZxKwOm4HGYoVIHQ==", "requires": { "tslib": "^2.3.1" } @@ -17232,9 +17232,9 @@ "dev": true }, "node-fetch": { - "version": "2.6.7", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.7.tgz", - "integrity": "sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "requires": { "whatwg-url": "^5.0.0" }, diff --git a/package.json b/package.json index 41ed506..9085bdd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.11.0", "description": "A React library to easily integrate and use Split JS SDK", "main": "lib/index.js", "module": "es/index.js", @@ -62,7 +62,7 @@ }, "homepage": "https://github.com/splitio/react-client#readme", "dependencies": { - "@splitsoftware/splitio": "10.24.1", + "@splitsoftware/splitio": "10.25.1", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index 0e735a8..fa584f4 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -1,13 +1,12 @@ import React from 'react'; import { SplitContext } from './SplitContext'; import { ISplitClientProps, ISplitContextValues, IUpdateProps } from './types'; -import { ERROR_SC_NO_FACTORY } from './constants'; import { getStatus, getSplitClient, initAttributes, IClientWithContext } from './utils'; import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; /** * Common component used to handle the status and events of a Split client passed as prop. - * Reused by both SplitFactory (main client) and SplitClient (shared client) components. + * Reused by both SplitFactoryProvider (main client) and SplitClient (any client) components. */ export class SplitComponent extends React.Component { @@ -47,11 +46,6 @@ export class SplitComponent extends React.Component(INITIAL_CONTEXT); diff --git a/src/SplitFactory.tsx b/src/SplitFactory.tsx index ba2c12d..74d9236 100644 --- a/src/SplitFactory.tsx +++ b/src/SplitFactory.tsx @@ -9,12 +9,17 @@ import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; /** * SplitFactory will initialize the Split SDK and its main client, listen for its events in order to update the Split Context, * and automatically shutdown and release resources when it is unmounted. SplitFactory must wrap other components and functions - * from this library, since they access the Split Context and its elements (factory, clients, etc). + * from this library, since they access the Split Context and its properties (factory, client, isReady, etc). * * The underlying SDK factory and client is set on the constructor, and cannot be changed during the component lifecycle, * even if the component is updated with a different config or factory prop. * - * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK} + * @deprecated Replace with the new `SplitFactoryProvider` component. + * `SplitFactoryProvider` is a drop-in replacement that properly handles side effects (factory creation and destruction) within the React component lifecycle, avoiding issues with factory recreation and memory leaks. + * Note: There is a subtle breaking change in `SplitFactoryProvider`. When using the `config` prop, `factory` and `client` properties in the context are `null` in the first render, until the context is updated when some event is emitted on + * the SDK main client (ready, ready from cache, timeout or update depending on the configuration of the `updateOnXXX` props of the component). This differs from the previous behavior where `factory` and `client` were immediately available. + * + * @see {@link https://help.split.io/hc/en-us/articles/360038825091-React-SDK#2-instantiate-the-sdk-and-create-a-new-split-client} */ export class SplitFactory extends React.Component { diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx new file mode 100644 index 0000000..5f91df7 --- /dev/null +++ b/src/SplitFactoryProvider.tsx @@ -0,0 +1,86 @@ +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, __factories } from './utils'; +import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; + +/** + * SplitFactoryProvider will initialize the Split SDK and its main client when `config` prop is provided or updated, listen for its events in order to update the Split Context, + * and automatically destroy the SDK (shutdown and release resources) when it is unmounted or `config` prop updated. SplitFactoryProvider must wrap other library components and + * functions since they access the Split Context and its properties (factory, client, isReady, etc). + * + * NOTE: Either pass a factory instance or a config object. If both are passed, the config object will be ignored. + * Pass the same reference to the config or factory object rather than a new instance on each render, to avoid unnecessary props changes and SDK reinitializations. + * + * @see {@link https://help.split.io/hc/en-us/articles/360038825091-React-SDK#2-instantiate-the-sdk-and-create-a-new-split-client} + */ +export function SplitFactoryProvider(props: ISplitFactoryProps) { + let { + config, factory: propFactory, + updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate + } = { ...DEFAULT_UPDATE_OPTIONS, ...props }; + + if (config && propFactory) { + console.log(WARN_SF_CONFIG_AND_FACTORY); + config = undefined; + } + + 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); + + // 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); + } + + if (updateOnSdkReady) { + if (status.isReady) update(); + else client.once(client.Event.SDK_READY, update); + } + if (updateOnSdkReadyFromCache) { + if (status.isReadyFromCache) update(); + else client.once(client.Event.SDK_READY_FROM_CACHE, update); + } + if (updateOnSdkTimedout) { + if (status.hasTimedout) update(); + else client.once(client.Event.SDK_READY_TIMED_OUT, update); + } + if (updateOnSdkUpdate) client.on(client.Event.SDK_UPDATE, update); + + return unsubscribe; + } + }, [config, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]); + + return ( + + ); +} diff --git a/src/SplitTreatments.tsx b/src/SplitTreatments.tsx index 995f108..da803c3 100644 --- a/src/SplitTreatments.tsx +++ b/src/SplitTreatments.tsx @@ -1,7 +1,6 @@ import React from 'react'; import { SplitContext } from './SplitContext'; import { ISplitTreatmentsProps, ISplitContextValues } from './types'; -import { WARN_ST_NO_CLIENT } from './constants'; import { memoizeGetTreatmentsWithConfig } from './utils'; /** @@ -26,7 +25,7 @@ export class SplitTreatments extends React.Component { {(splitContext: ISplitContextValues) => { const { client, lastUpdate } = splitContext; const treatments = this.evaluateFeatureFlags(client, lastUpdate, names, attributes, client ? { ...client.getAttributes() } : {}, flagSets); - if (!client) { this.logWarning = true; } + // SplitTreatments only accepts a function as a child, not a React Element (JSX) return children({ ...splitContext, treatments, @@ -35,9 +34,4 @@ export class SplitTreatments extends React.Component { ); } - - componentDidMount() { - if (this.logWarning) { console.log(WARN_ST_NO_CLIENT); } - } - } diff --git a/src/__tests__/SplitClient.test.tsx b/src/__tests__/SplitClient.test.tsx index 482403e..3ab262c 100644 --- a/src/__tests__/SplitClient.test.tsx +++ b/src/__tests__/SplitClient.test.tsx @@ -11,17 +11,16 @@ import { sdkBrowser } from './testUtils/sdkConfigs'; /** Test target */ import { ISplitClientChildProps } from '../types'; -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { SplitClient } from '../SplitClient'; import { SplitContext } from '../SplitContext'; -import { ERROR_SC_NO_FACTORY } from '../constants'; import { testAttributesBinding, TestComponentProps } from './testUtils/utils'; describe('SplitClient', () => { test('passes no-ready props to the child if client is not ready.', () => { render( - + {({ isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => { expect(isReady).toBe(false); @@ -34,7 +33,7 @@ describe('SplitClient', () => { return null; }} - + ); }); @@ -46,7 +45,7 @@ describe('SplitClient', () => { await outerFactory.client().ready(); render( - + {/* Equivalent to */} {({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitClientChildProps) => { @@ -61,7 +60,7 @@ describe('SplitClient', () => { return null; }} - + ); }); @@ -76,7 +75,7 @@ describe('SplitClient', () => { let previousLastUpdate = -1; render( - + {({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitClientChildProps) => { const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; @@ -106,7 +105,7 @@ describe('SplitClient', () => { return null; }} - + ); act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT)); @@ -128,7 +127,7 @@ describe('SplitClient', () => { let previousLastUpdate = -1; render( - + {({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitClientChildProps) => { const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; @@ -152,7 +151,7 @@ describe('SplitClient', () => { return null; }} - + ); act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT)); @@ -172,7 +171,7 @@ describe('SplitClient', () => { let previousLastUpdate = -1; render( - + {({ client, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitClientChildProps) => { const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; @@ -193,7 +192,7 @@ describe('SplitClient', () => { return null; }} - + ); act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT)); @@ -207,7 +206,7 @@ describe('SplitClient', () => { let count = 0; render( - + {({ client }) => { count++; @@ -221,7 +220,7 @@ describe('SplitClient', () => { return null; }} - + ); expect(count).toEqual(2); @@ -246,26 +245,27 @@ describe('SplitClient', () => { }; render( - + - + ); }); - test('logs error and passes null client if rendered outside an SplitProvider component.', () => { - const errorSpy = jest.spyOn(console, 'error'); - render( - - {({ client }) => { - expect(client).toBe(null); - return null; - }} - - ); - expect(errorSpy).toBeCalledWith(ERROR_SC_NO_FACTORY); - }); + // @TODO Update test in breaking change, following common practice in React libraries, like React-redux and React-query: use a falsy value as default context value, and throw an error – instead of logging it – if components are not wrapped in a SplitContext.Provider, i.e., if the context is falsy. + // test('logs error and passes null client if rendered outside an SplitProvider component.', () => { + // const errorSpy = jest.spyOn(console, 'error'); + // render( + // + // {({ client }) => { + // expect(client).toBe(null); + // return null; + // }} + // + // ); + // expect(errorSpy).toBeCalledWith(ERROR_SC_NO_FACTORY); + // }); test(`passes a new client if re-rendered with a different splitKey. Only updates the state if the new client triggers an event, but not the previous one.`, (done) => { @@ -338,9 +338,9 @@ describe('SplitClient', () => { } render( - + - + ); }); @@ -348,14 +348,14 @@ describe('SplitClient', () => { function Component({ attributesFactory, attributesClient, splitKey, testSwitch, factory }: TestComponentProps) { return ( - + {() => { testSwitch(done, splitKey); return null; }} - + ); } diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx new file mode 100644 index 0000000..6745a4d --- /dev/null +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -0,0 +1,456 @@ +import React from 'react'; +import { render, act } from '@testing-library/react'; + +/** Mocks */ +import { mockSdk, Event } from './testUtils/mockSplitSdk'; +jest.mock('@splitsoftware/splitio/client', () => { + return { SplitFactory: mockSdk() }; +}); +import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client'; +import { sdkBrowser } from './testUtils/sdkConfigs'; +const logSpy = jest.spyOn(console, 'log'); + +/** Test target */ +import { ISplitFactoryChildProps } from '../types'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; +import { SplitClient } from '../SplitClient'; +import { SplitContext } from '../SplitContext'; +import { __factories } from '../utils'; +import { WARN_SF_CONFIG_AND_FACTORY } from '../constants'; + +describe('SplitFactoryProvider', () => { + + test('passes no-ready props to the child if initialized with a config.', () => { + render( + + {({ factory, client, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryChildProps) => { + 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); + return null; + }} + + ); + }); + + test('passes ready props to the child if initialized with a ready factory.', async () => { + const outerFactory = SplitSdk(sdkBrowser); + (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_FROM_CACHE); + (outerFactory as any).client().__emitter__.emit(Event.SDK_READY); + (outerFactory.manager().names as jest.Mock).mockReturnValue(['split1']); + await outerFactory.client().ready(); + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, isDestroyed, lastUpdate }: ISplitFactoryChildProps) => { + 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(0); + expect((factory as SplitIO.ISDK).settings.version).toBe(outerFactory.settings.version); + return null; + }} + + ); + }); + + test('rerenders child on SDK_READY_TIMEDOUT, SDK_READY_FROM_CACHE, SDK_READY and SDK_UPDATE events (config prop)', async () => { + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Timedout + expect(statusProps).toStrictEqual([false, false, true, true]); + break; + case 2: // Ready from cache + expect(statusProps).toStrictEqual([false, true, true, true]); + break; + case 3: // Ready + expect(statusProps).toStrictEqual([true, true, true, false]); + break; + case 4: // Updated + expect(statusProps).toStrictEqual([true, true, true, false]); + break; + default: + fail('Child must not be rerendered'); + } // eslint-disable-next-line no-use-before-define + if (factory) expect(factory).toBe(innerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + const innerFactory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + + expect(renderTimes).toBe(5); + }); + + test('rerenders child on SDK_READY_TIMEDOUT, SDK_READY_FROM_CACHE, SDK_READY and SDK_UPDATE events (factory prop)', async () => { + const outerFactory = SplitSdk(sdkBrowser); + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Timedout + expect(statusProps).toStrictEqual([false, false, true, true]); + break; + case 2: // Ready from cache + expect(statusProps).toStrictEqual([false, true, true, true]); + break; + case 3: // Ready + expect(statusProps).toStrictEqual([true, true, true, false]); + break; + case 4: // Updated + expect(statusProps).toStrictEqual([true, true, true, false]); + break; + default: + fail('Child must not be rerendered'); + } + expect(factory).toBe(outerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_FROM_CACHE)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + + expect(renderTimes).toBe(5); + }); + + test('rerenders child on SDK_READY_TIMED_OUT and SDK_UPDATE events, but not on SDK_READY (config prop)', async () => { + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Timedout + expect(statusProps).toStrictEqual([false, false, true, true]); + break; + case 2: // Updated. Although `updateOnSdkReady` is false, status props must reflect the current status of the client. + expect(statusProps).toStrictEqual([true, false, true, false]); + break; + default: + fail('Child must not be rerendered'); + } // eslint-disable-next-line no-use-before-define + if (factory) expect(factory).toBe(innerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + const innerFactory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + + expect(renderTimes).toBe(3); + }); + + test('rerenders child on SDK_READY_TIMED_OUT and SDK_UPDATE events, but not on SDK_READY (factory prop)', async () => { + const outerFactory = SplitSdk(sdkBrowser); + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Timedout + expect(statusProps).toStrictEqual([false, false, true, true]); + break; + case 2: // Updated. Although `updateOnSdkReady` is false, status props must reflect the current status of the client. + expect(statusProps).toStrictEqual([true, false, true, false]); + break; + default: + fail('Child must not be rerendered'); + } + expect(factory).toBe(outerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + + expect(renderTimes).toBe(3); + }); + + test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (config prop)', async () => { + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Ready + expect(statusProps).toStrictEqual([true, false, true, false]); // not rerendering on SDK_TIMEOUT, but hasTimedout reflects the current state + break; + default: + fail('Child must not be rerendered'); + } // eslint-disable-next-line no-use-before-define + if (factory) expect(factory).toBe(innerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + const innerFactory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (innerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + expect(renderTimes).toBe(2); + }); + + test('rerenders child only on SDK_READY and SDK_READY_FROM_CACHE event, as default behaviour (factory prop)', async () => { + const outerFactory = SplitSdk(sdkBrowser); + let renderTimes = 0; + let previousLastUpdate = -1; + + render( + + {({ factory, isReady, isReadyFromCache, hasTimedout, isTimedout, lastUpdate }: ISplitFactoryChildProps) => { + const statusProps = [isReady, isReadyFromCache, hasTimedout, isTimedout]; + switch (renderTimes) { + case 0: // No ready + expect(statusProps).toStrictEqual([false, false, false, false]); + break; + case 1: // Ready + expect(statusProps).toStrictEqual([true, false, true, false]); // not rerendering on SDK_TIMEOUT, but hasTimedout reflects the current state + break; + default: + fail('Child must not be rerendered'); + } + expect(factory).toBe(outerFactory); + expect(lastUpdate).toBeGreaterThan(previousLastUpdate); + renderTimes++; + previousLastUpdate = lastUpdate; + return null; + }} + + ); + + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_READY)); + act(() => (outerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE)); + expect(renderTimes).toBe(2); + }); + + test('renders a passed JSX.Element with a new SplitContext value.', (done) => { + const Component = () => { + return ( + + {(value) => { + expect(value.factory).toBe(null); + expect(value.client).toBe(null); + expect(value.isReady).toBe(false); + expect(value.isTimedout).toBe(false); + expect(value.lastUpdate).toBe(0); + done(); + return null; + }} + + ); + }; + + render( + + + + ); + }); + + test('logs warning if both a config and factory are passed as props.', () => { + const outerFactory = SplitSdk(sdkBrowser); + + render( + + {({ factory }) => { + expect(factory).toBe(outerFactory); + return null; + }} + + ); + + expect(logSpy).toBeCalledWith(WARN_SF_CONFIG_AND_FACTORY); + logSpy.mockRestore(); + }); + + 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++; + + 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 }) => { + clientDestroySpies.push(jest.spyOn(client!, 'destroy')); + return null; + }} + + ); + case 7: + throw new Error('Must not rerender'); + } + }; + + 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(); + + // Created factories are removed from `factories` cache and their clients are destroyed + expect(createdFactories.size).toBe(2); + expect(__factories.size).toBe(0); + clientDestroySpies.forEach(spy => expect(spy).toBeCalledTimes(1)); + }); + + test('doesn\'t clean up on unmount if the factory is provided as a prop.', () => { + let destroyMainClientSpy; + let destroySharedClientSpy; + const outerFactory = SplitSdk(sdkBrowser); + const wrapper = render( + + {({ factory }) => { + // if factory is provided as a prop, `factories` cache is not modified + expect(__factories.size).toBe(0); + destroyMainClientSpy = jest.spyOn(factory!.client(), 'destroy'); + return ( + + {({ client }) => { + destroySharedClientSpy = jest.spyOn(client!, 'destroy'); + return null; + }} + + ); + }} + + ); + wrapper.unmount(); + expect(destroyMainClientSpy).not.toBeCalled(); + expect(destroySharedClientSpy).not.toBeCalled(); + }); + +}); diff --git a/src/__tests__/SplitTreatments.test.tsx b/src/__tests__/SplitTreatments.test.tsx index 13d8990..308379c 100644 --- a/src/__tests__/SplitTreatments.test.tsx +++ b/src/__tests__/SplitTreatments.test.tsx @@ -10,13 +10,13 @@ import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client'; import { sdkBrowser } from './testUtils/sdkConfigs'; import { getStatus } from '../utils'; import { newSplitFactoryLocalhostInstance } from './testUtils/utils'; -import { CONTROL_WITH_CONFIG, WARN_ST_NO_CLIENT } from '../constants'; +import { CONTROL_WITH_CONFIG } from '../constants'; /** Test target */ import { ISplitTreatmentsChildProps, ISplitTreatmentsProps, ISplitClientProps } from '../types'; import { SplitTreatments } from '../SplitTreatments'; import { SplitClient } from '../SplitClient'; -import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { useSplitTreatments } from '../useSplitTreatments'; const logSpy = jest.spyOn(console, 'log'); @@ -28,24 +28,20 @@ describe('SplitTreatments', () => { afterEach(() => { logSpy.mockClear() }); - it('passes control treatments (empty object if flagSets is provided) if the SDK is not ready.', () => { + it('passes control treatments (empty object if flagSets is provided) if the SDK is not operational.', () => { render( - - {({ factory }) => { + + {() => { return ( {({ treatments }: ISplitTreatmentsChildProps) => { - const clientMock: any = factory?.client('user1'); - expect(clientMock.getTreatmentsWithConfig).not.toBeCalled(); expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG }); return null; }} {({ treatments }: ISplitTreatmentsChildProps) => { - const clientMock: any = factory?.client('user1'); - expect(clientMock.getTreatmentsWithConfigByFlagSets).not.toBeCalled(); expect(treatments).toEqual({}); return null; }} @@ -53,7 +49,7 @@ describe('SplitTreatments', () => { ); }} - + ); }); @@ -62,7 +58,7 @@ describe('SplitTreatments', () => { (outerFactory as any).client().__emitter__.emit(Event.SDK_READY); render( - + {({ factory, isReady }) => { expect(getStatus(outerFactory.client()).isReady).toBe(isReady); expect(isReady).toBe(true); @@ -90,22 +86,23 @@ describe('SplitTreatments', () => { ); }} - + ); }); - it('logs error and passes control treatments if rendered outside an SplitProvider component.', () => { - render( - - {({ treatments }: ISplitTreatmentsChildProps) => { - expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG }); - return null; - }} - - ); + // @TODO Update test in breaking change, following common practice in React libraries, like React-redux and React-query: use a falsy value as default context value, and throw an error – instead of logging it – if components are not wrapped in a SplitContext.Provider, i.e., if the context is falsy. + // it('logs error and passes control treatments if rendered outside an SplitProvider component.', () => { + // render( + // + // {({ treatments }: ISplitTreatmentsChildProps) => { + // expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG, split2: CONTROL_WITH_CONFIG }); + // return null; + // }} + // + // ); - expect(logSpy).toBeCalledWith(WARN_ST_NO_CLIENT); - }); + // expect(logSpy).toBeCalledWith(WARN_ST_NO_CLIENT); + // }); /** * Input validation. Passing invalid feature flag names or attributes while the Sdk @@ -113,7 +110,7 @@ describe('SplitTreatments', () => { */ it('Input validation: invalid "names" and "attributes" props in SplitTreatments.', (done) => { render( - + {() => { return ( <> @@ -141,7 +138,7 @@ describe('SplitTreatments', () => { ); }} - + ); expect(logSpy).toBeCalledWith('[ERROR] feature flag names must be a non-empty array.'); expect(logSpy).toBeCalledWith('[ERROR] you passed an invalid feature flag name, feature flag name must be a non-empty string.'); @@ -198,11 +195,11 @@ describe.each([ clientAttributes?: ISplitClientProps['attributes'] }) { return ( - + - + ); } @@ -302,27 +299,27 @@ describe.each([ expect(outerFactory.client('otherKey').getTreatmentsWithConfig).toBeCalledTimes(1); }); - it('rerenders and re-evaluate feature flags when Split context changes (in both SplitFactory and SplitClient components).', async () => { + it('rerenders and re-evaluate feature flags when Split context changes (in both SplitFactoryProvider 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); let renderTimesComp1 = 0; let renderTimesComp2 = 0; - // test context updates on SplitFactory + // test context updates on SplitFactoryProvider render( - + {() => { renderTimesComp1++; return null; }} - + ); // test context updates on SplitClient render( - + {() => { @@ -331,7 +328,7 @@ describe.each([ }} - + ); expect(renderTimesComp1).toBe(1); diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index c8bd2f3..b0b3538 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -3,6 +3,7 @@ import { SplitContext as ExportedSplitContext, SplitSdk as ExportedSplitSdk, SplitFactory as ExportedSplitFactory, + SplitFactoryProvider as ExportedSplitFactoryProvider, SplitClient as ExportedSplitClient, SplitTreatments as ExportedSplitTreatments, withSplitFactory as exportedWithSplitFactory, @@ -32,6 +33,7 @@ import { import { SplitContext } from '../SplitContext'; import { SplitFactory as SplitioEntrypoint } from '@splitsoftware/splitio/client'; import { SplitFactory } from '../SplitFactory'; +import { SplitFactoryProvider } from '../SplitFactoryProvider'; import { SplitClient } from '../SplitClient'; import { SplitTreatments } from '../SplitTreatments'; import { withSplitFactory } from '../withSplitFactory'; @@ -49,6 +51,7 @@ describe('index', () => { it('should export components', () => { expect(ExportedSplitFactory).toBe(SplitFactory); + expect(ExportedSplitFactoryProvider).toBe(SplitFactoryProvider); expect(ExportedSplitClient).toBe(SplitClient); expect(ExportedSplitTreatments).toBe(SplitTreatments); }); 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__/constants.test.ts b/src/__tests__/utils.test.ts similarity index 86% rename from src/__tests__/constants.test.ts rename to src/__tests__/utils.test.ts index c51913a..b5ef021 100644 --- a/src/__tests__/constants.test.ts +++ b/src/__tests__/utils.test.ts @@ -1,4 +1,5 @@ -import { getControlTreatmentsWithConfig, CONTROL_WITH_CONFIG } from '../constants'; +import { CONTROL_WITH_CONFIG } from '../constants'; +import { getControlTreatmentsWithConfig } from '../utils'; import SplitIO from '@splitsoftware/splitio/types/splitio'; describe('getControlTreatmentsWithConfig', () => { 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/__tests__/withSplitTreatments.test.tsx b/src/__tests__/withSplitTreatments.test.tsx index d1e665f..f5d523b 100644 --- a/src/__tests__/withSplitTreatments.test.tsx +++ b/src/__tests__/withSplitTreatments.test.tsx @@ -12,7 +12,7 @@ import { sdkBrowser } from './testUtils/sdkConfigs'; import { withSplitFactory } from '../withSplitFactory'; import { withSplitClient } from '../withSplitClient'; import { withSplitTreatments } from '../withSplitTreatments'; -import { getControlTreatmentsWithConfig } from '../constants'; +import { getControlTreatmentsWithConfig } from '../utils'; describe('withSplitTreatments', () => { diff --git a/src/constants.ts b/src/constants.ts index 47020ef..c29d802 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,5 +1,3 @@ -import { validateFeatureFlags } from './utils'; - // The string below is a marker and will be replaced by the real version number. DO NOT CHANGE export const VERSION: string = 'react-' + 'REACT_SDK_VERSION_NUMBER'; @@ -15,29 +13,12 @@ export const CONTROL_WITH_CONFIG: SplitIO.TreatmentWithConfig = { config: null, }; -export const getControlTreatmentsWithConfig = (featureFlagNames: unknown): SplitIO.TreatmentsWithConfig => { - // validate featureFlags Names - const validatedFeatureFlagNames = validateFeatureFlags(featureFlagNames); - - // return empty object if the returned value is false - if (!validatedFeatureFlagNames) return {}; - - // return control treatments for each validated feature flag name - return validatedFeatureFlagNames.reduce((pValue: SplitIO.TreatmentsWithConfig, cValue: string) => { - pValue[cValue] = CONTROL_WITH_CONFIG; - return pValue; - }, {}); -}; - // Warning and error messages -export const WARN_SF_CONFIG_AND_FACTORY: string = '[WARN] Both a config and factory props were provided to SplitFactory. Config prop will be ignored.'; +export const WARN_SF_CONFIG_AND_FACTORY: string = '[WARN] Both a config and factory props were provided to SplitFactoryProvider. Config prop will be ignored.'; +// @TODO remove with SplitFactory component in next major version. SplitFactoryProvider can accept no props and eventually only an initialState export const ERROR_SF_NO_CONFIG_AND_FACTORY: string = '[ERROR] SplitFactory must receive either a Split config or a Split factory as props.'; -export const ERROR_SC_NO_FACTORY: string = '[ERROR] SplitClient does not have access to a Split factory. This is because it is not inside the scope of a SplitFactory component or SplitFactory was not properly instantiated.'; - -export const WARN_ST_NO_CLIENT: string = '[WARN] SplitTreatments does not have access to a Split client. This is because it is not inside the scope of a SplitFactory component or SplitFactory was not properly instantiated.'; - export const EXCEPTION_NO_REACT_OR_CREATECONTEXT: string = 'React library is not available or its version is not supported. Check that it is properly installed or imported. Split SDK requires version 16.3.0+ of React.'; export const WARN_NAMES_AND_FLAGSETS: string = '[WARN] Both names and flagSets properties were provided. flagSets will be ignored.'; diff --git a/src/index.ts b/src/index.ts index 1e2dddd..10a5161 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,10 +6,11 @@ export { withSplitFactory } from './withSplitFactory'; export { withSplitClient } from './withSplitClient'; export { withSplitTreatments } from './withSplitTreatments'; -// Render props components +// Components export { SplitTreatments } from './SplitTreatments'; export { SplitClient } from './SplitClient'; export { SplitFactory } from './SplitFactory'; +export { SplitFactoryProvider } from './SplitFactoryProvider'; // Hooks export { useClient } from './useClient'; 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, diff --git a/src/utils.ts b/src/utils.ts index ee42523..760077a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,7 @@ import memoizeOne from 'memoize-one'; import shallowEqual from 'shallowequal'; import { SplitFactory as SplitSdk } from '@splitsoftware/splitio/client'; -import { VERSION, WARN_NAMES_AND_FLAGSETS, getControlTreatmentsWithConfig } from './constants'; +import { CONTROL_WITH_CONFIG, VERSION, WARN_NAMES_AND_FLAGSETS } from './constants'; import { ISplitStatus } from './types'; // Utils used to access singleton instances of Split factories and clients, and to gracefully shutdown all clients together. @@ -97,9 +97,16 @@ export function getStatus(client: SplitIO.IBrowserClient | null): ISplitStatus { }; } +/** + * Manage client attributes binding + */ +export function initAttributes(client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes) { + if (client && attributes) client.setAttributes(attributes); +} + // Input validation utils that will be replaced eventually -export function validateFeatureFlags(maybeFeatureFlags: unknown, listName = 'feature flag names'): false | string[] { +function validateFeatureFlags(maybeFeatureFlags: unknown, listName = 'feature flag names'): false | string[] { if (Array.isArray(maybeFeatureFlags) && maybeFeatureFlags.length > 0) { const validatedArray: string[] = []; // Remove invalid values @@ -116,13 +123,6 @@ export function validateFeatureFlags(maybeFeatureFlags: unknown, listName = 'fea return false; } -/** - * Manage client attributes binding - */ -export function initAttributes(client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes) { - if (client && attributes) client.setAttributes(attributes); -} - const TRIMMABLE_SPACES_REGEX = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/; function validateFeatureFlag(maybeFeatureFlag: unknown, item = 'feature flag name'): false | string { @@ -146,6 +146,20 @@ function validateFeatureFlag(maybeFeatureFlag: unknown, item = 'feature flag nam return false; } +export function getControlTreatmentsWithConfig(featureFlagNames: unknown): SplitIO.TreatmentsWithConfig { + // validate featureFlags Names + const validatedFeatureFlagNames = validateFeatureFlags(featureFlagNames); + + // return empty object if the returned value is false + if (!validatedFeatureFlagNames) return {}; + + // return control treatments for each validated feature flag name + return validatedFeatureFlagNames.reduce((pValue: SplitIO.TreatmentsWithConfig, cValue: string) => { + pValue[cValue] = CONTROL_WITH_CONFIG; + return pValue; + }, {}); +} + /** * Removes duplicate items on an array of strings. */ diff --git a/src/withSplitFactory.tsx b/src/withSplitFactory.tsx index bd939e2..45eb55f 100644 --- a/src/withSplitFactory.tsx +++ b/src/withSplitFactory.tsx @@ -9,6 +9,11 @@ import { SplitFactory } from './SplitFactory'; * * @param config Config object used to instantiate a Split factory * @param factory Split factory instance to use instead of creating a new one with the config object. + * + * @deprecated Use `SplitFactoryProvider` instead. + * `SplitFactoryProvider` is a drop-in replacement of `SplitFactory` that properly handles side effects (factory creation and destruction) within the React component lifecycle, avoiding issues with factory recreation and memory leaks. + * Note: There is a subtle breaking change in `SplitFactoryProvider`. When using the `config` prop, `factory` and `client` properties in the context are `null` in the first render, until the context is updated when some event is emitted on + * the SDK main client (ready, ready from cache, timeout or update depending on the configuration of the `updateOnXXX` props of the component). This differs from the previous behavior where `factory` and `client` were immediately available. */ export function withSplitFactory(config?: SplitIO.IBrowserSettings, factory?: SplitIO.IBrowserSDK, attributes?: SplitIO.Attributes) { diff --git a/umd.ts b/umd.ts index 1d67908..6beea58 100644 --- a/umd.ts +++ b/umd.ts @@ -1,15 +1,15 @@ import { SplitSdk, withSplitFactory, withSplitClient, withSplitTreatments, - SplitFactory, SplitClient, SplitTreatments, - useClient, useTreatments, useTrack, useManager, + SplitFactory, SplitFactoryProvider, SplitClient, SplitTreatments, + useClient, useSplitClient, useTreatments, useSplitTreatments, useTrack, useManager, useSplitManager, SplitContext, } from './src/index'; export default { SplitSdk, withSplitFactory, withSplitClient, withSplitTreatments, - SplitFactory, SplitClient, SplitTreatments, - useClient, useTreatments, useTrack, useManager, + SplitFactory, SplitFactoryProvider, SplitClient, SplitTreatments, + useClient, useSplitClient, useTreatments, useSplitTreatments, useTrack, useManager, useSplitManager, SplitContext, };