From bd29434b4e16ffcb080e53f160acf159af5f4ee9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 1 Jan 2024 03:03:15 +0000 Subject: [PATCH 01/21] Updated License Year --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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. From 6c85f264f39327ea27c600fdda524d60896b997e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 2 Jan 2024 14:40:47 -0300 Subject: [PATCH 02/21] Refactor code to avoid circular dependency --- CHANGES.txt | 1 + src/__tests__/constants.test.ts | 3 +- src/__tests__/withSplitTreatments.test.tsx | 2 +- src/constants.ts | 16 ----------- src/utils.ts | 32 ++++++++++++++++------ 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 8d31f0a..7f32a73 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -15,6 +15,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/src/__tests__/constants.test.ts b/src/__tests__/constants.test.ts index c51913a..b5ef021 100644 --- a/src/__tests__/constants.test.ts +++ b/src/__tests__/constants.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__/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..7d81db7 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,20 +13,6 @@ 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.'; 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. */ From c75b1ced9b3a5fe8cc2f0cdbc7a33e1ef4d8325f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 2 Jan 2024 14:45:02 -0300 Subject: [PATCH 03/21] Rename test file --- src/__tests__/{constants.test.ts => utils.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/__tests__/{constants.test.ts => utils.test.ts} (100%) diff --git a/src/__tests__/constants.test.ts b/src/__tests__/utils.test.ts similarity index 100% rename from src/__tests__/constants.test.ts rename to src/__tests__/utils.test.ts From 4c32921a1a3a955d5917ea9663961f95644222e1 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 3 Jan 2024 23:13:29 -0300 Subject: [PATCH 04/21] Update script --- .github/workflows/update-license-year.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 From 329b52ba383f752616db67b5cdf64b052d7b71cf Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Jan 2024 15:56:24 -0300 Subject: [PATCH 05/21] Add new SplitFactoryProvider component --- .github/workflows/{ci.yml => ci-cd.yml} | 4 +- .nvmrc | 2 +- package-lock.json | 50 ++++++++--------- package.json | 4 +- src/SplitClient.tsx | 6 -- src/SplitFactory.tsx | 2 + src/SplitFactoryProvider.tsx | 73 +++++++++++++++++++++++++ src/SplitTreatments.tsx | 8 +-- src/constants.ts | 7 +-- src/index.ts | 3 +- src/types.ts | 9 ++- 11 files changed, 117 insertions(+), 51 deletions(-) rename .github/workflows/{ci.yml => ci-cd.yml} (98%) create mode 100644 src/SplitFactoryProvider.tsx 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/.nvmrc b/.nvmrc index f274881..b009dfb 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -v16.16.0 +lts/* diff --git a/package-lock.json b/package-lock.json index 6e79190..99d919c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "license": "Apache-2.0", "dependencies": { - "@splitsoftware/splitio": "10.24.1", + "@splitsoftware/splitio": "10.25.1-rc.0", "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-rc.0", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", + "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", "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-rc.0", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", + "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", "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..f1608a7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "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-rc.0", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index 0e735a8..c76a6e0 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -1,7 +1,6 @@ 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'; @@ -47,11 +46,6 @@ export class SplitComponent extends React.Component { diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx new file mode 100644 index 0000000..7c1437b --- /dev/null +++ b/src/SplitFactoryProvider.tsx @@ -0,0 +1,73 @@ +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 { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; + +/** + * SplitFactoryProvider will initialize the Split SDK and its main client when it is mounted, listen for its events in order to update the Split Context, + * and automatically shutdown and release resources when it is unmounted. SplitFactoryProvider must wrap other library components and functions + * since they access the Split Context and its elements (factory, clients, etc). + * + * NOTE: Either pass a factory instance or a config object. If both are passed, the config object will be ignored. + * Pass a 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 [stateFactory, setStateFactory] = React.useState(propFactory || null); + const factory = propFactory || stateFactory; + const client = factory ? getSplitClient(factory) : null; + + React.useEffect(() => { + if (config) { + const factory = getSplitFactory(config); + const client = getSplitClient(factory); + const status = getStatus(client); + + // Update state and unsubscribe from events when first event is emitted + const update = () => { + 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) { + 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 () => { + destroySplitFactory(factory as IFactoryWithClients); + } + } + }, [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/constants.ts b/src/constants.ts index 7d81db7..e8df4c5 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -14,14 +14,11 @@ export const CONTROL_WITH_CONFIG: SplitIO.TreatmentWithConfig = { }; // 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 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..f4dabb9 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; From 1dae653816b44bf88d7962cd0b6d5884cbf4aefa Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Jan 2024 16:17:41 -0300 Subject: [PATCH 06/21] Update comments --- src/SplitClient.tsx | 7 +++---- src/SplitContext.ts | 2 +- src/types.ts | 10 +++++----- src/useClient.ts | 4 ++-- src/useManager.ts | 4 ++-- src/useSplitClient.ts | 2 +- src/useSplitManager.ts | 6 +++--- src/withSplitFactory.tsx | 2 ++ 8 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/SplitClient.tsx b/src/SplitClient.tsx index c76a6e0..fa584f4 100644 --- a/src/SplitClient.tsx +++ b/src/SplitClient.tsx @@ -6,7 +6,7 @@ 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 { @@ -123,9 +123,8 @@ export class SplitComponent extends React.Component(INITIAL_CONTEXT); diff --git a/src/types.ts b/src/types.ts index f4dabb9..b165f2d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -99,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 { @@ -128,7 +128,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; } @@ -170,7 +170,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/withSplitFactory.tsx b/src/withSplitFactory.tsx index bd939e2..076ecea 100644 --- a/src/withSplitFactory.tsx +++ b/src/withSplitFactory.tsx @@ -9,6 +9,8 @@ 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. */ export function withSplitFactory(config?: SplitIO.IBrowserSettings, factory?: SplitIO.IBrowserSDK, attributes?: SplitIO.Attributes) { From 8bb224bccb8cc68ccb4930fa9ab9700e8a553d48 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Jan 2024 17:40:03 -0300 Subject: [PATCH 07/21] Update unit tests to use the new SplitFactoryProvider --- src/__tests__/SplitClient.test.tsx | 64 +++++++++++------------ src/__tests__/SplitTreatments.test.tsx | 63 +++++++++++----------- src/__tests__/index.test.ts | 3 ++ src/__tests__/useSplitClient.test.tsx | 36 ++++++------- src/__tests__/useSplitManager.test.tsx | 6 +-- src/__tests__/useSplitTreatments.test.tsx | 18 +++---- src/__tests__/useTrack.test.tsx | 16 +++--- src/__tests__/withSplitFactory.test.tsx | 2 +- 8 files changed, 104 insertions(+), 104 deletions(-) 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__/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__/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)( From 0410c0c625eed88c3de0b6a890398274f2a05d36 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Jan 2024 17:41:47 -0300 Subject: [PATCH 08/21] Add unit test for the new SplitFactoryProvider --- src/__tests__/SplitFactoryProvider.test.tsx | 401 ++++++++++++++++++++ 1 file changed, 401 insertions(+) create mode 100644 src/__tests__/SplitFactoryProvider.test.tsx diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx new file mode 100644 index 0000000..2964a60 --- /dev/null +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -0,0 +1,401 @@ +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 unmount.', () => { + let destroyMainClientSpy; + let destroySharedClientSpy; + const wrapper = render( + + {({ factory }) => { + if (!factory) return null; // 1st render + + // 2nd render (SDK ready) + expect(__factories.size).toBe(1); + destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy'); + return ( + + {({ client }) => { + destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + return null; + }} + + ); + }} + + ); + + // SDK ready to re-render + act(() => { + const factory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + factory.client().__emitter__.emit(Event.SDK_READY) + }); + + wrapper.unmount(); + // the factory created by the component is removed from `factories` cache and its clients are destroyed + expect(__factories.size).toBe(0); + expect(destroyMainClientSpy).toBeCalledTimes(1); + expect(destroySharedClientSpy).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 as SplitIO.ISDK).client(), 'destroy'); + return ( + + {({ client }) => { + destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + return null; + }} + + ); + }} + + ); + wrapper.unmount(); + expect(destroyMainClientSpy).not.toBeCalled(); + expect(destroySharedClientSpy).not.toBeCalled(); + }); + +}); From 68dd9fabc96c0575bcf5aa4569dce6a8568a6f7d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 11:12:10 -0300 Subject: [PATCH 09/21] Polishing --- README.md | 8 ++++---- src/SplitFactoryProvider.tsx | 1 + src/types.ts | 2 ++ umd.ts | 8 ++++---- 4 files changed, 11 insertions(+), 8 deletions(-) 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/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index 7c1437b..afc3e95 100644 --- a/src/SplitFactoryProvider.tsx +++ b/src/SplitFactoryProvider.tsx @@ -62,6 +62,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); } } diff --git a/src/types.ts b/src/types.ts index b165f2d..5fc7cac 100644 --- a/src/types.ts +++ b/src/types.ts @@ -119,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; 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, }; From cf1f63673d7a5b269efbb558932d59a8e5da6dff Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 11:12:35 -0300 Subject: [PATCH 10/21] Add CHANGES entry --- CHANGES.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 7f32a73..a69655e 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 (issue #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. From 6bf6b3c723688903bdb04b1390d15c05332f3a3c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 11:14:13 -0300 Subject: [PATCH 11/21] Prepare rc --- .github/workflows/ci-cd.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 3870a1f..57499ea 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -77,7 +77,7 @@ jobs: -Dsonar.pullrequest.base=${{ github.event.pull_request.base.ref }} - name: Store assets - if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/add_SplitFactoryProvider') uses: actions/upload-artifact@v3 with: name: assets @@ -88,7 +88,7 @@ jobs: name: Upload assets runs-on: ubuntu-latest needs: build - if: github.event_name == 'push' && github.ref == 'refs/heads/development' + if: github.event_name == 'push' && github.ref == 'refs/heads/add_SplitFactoryProvider' strategy: matrix: environment: From 1c0b65a13e9cd600a9ba54337a7f7319ca96524b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 12:25:33 -0300 Subject: [PATCH 12/21] Polishing --- .github/workflows/ci-cd.yml | 4 ++-- src/SplitFactoryProvider.tsx | 2 +- src/constants.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 57499ea..3870a1f 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -77,7 +77,7 @@ jobs: -Dsonar.pullrequest.base=${{ github.event.pull_request.base.ref }} - name: Store assets - if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/add_SplitFactoryProvider') + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') uses: actions/upload-artifact@v3 with: name: assets @@ -88,7 +88,7 @@ jobs: name: Upload assets runs-on: ubuntu-latest needs: build - if: github.event_name == 'push' && github.ref == 'refs/heads/add_SplitFactoryProvider' + if: github.event_name == 'push' && github.ref == 'refs/heads/development' strategy: matrix: environment: diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index afc3e95..d3ac78b 100644 --- a/src/SplitFactoryProvider.tsx +++ b/src/SplitFactoryProvider.tsx @@ -12,7 +12,7 @@ import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; * since they access the Split Context and its elements (factory, clients, etc). * * NOTE: Either pass a factory instance or a config object. If both are passed, the config object will be ignored. - * Pass a reference to the config or factory object rather than a new instance on each render, to avoid unnecessary props changes and SDK reinitializations. + * 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} */ diff --git a/src/constants.ts b/src/constants.ts index e8df4c5..c29d802 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -16,7 +16,7 @@ export const CONTROL_WITH_CONFIG: SplitIO.TreatmentWithConfig = { // Warning and error messages 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 +// @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 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.'; From 326aa6db4a7ab4f88302e654c57e2309a88a14e3 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 13:51:23 -0300 Subject: [PATCH 13/21] Add new SplitFactoryProvider component and deprecate SplitFactory component and withSplitFactory HOC --- package-lock.json | 50 +-- package.json | 4 +- src/SplitFactory.tsx | 2 + src/SplitFactoryProvider.tsx | 74 ++++ src/__tests__/SplitFactoryProvider.test.tsx | 401 ++++++++++++++++++++ src/__tests__/index.test.ts | 3 + src/index.ts | 3 +- src/withSplitFactory.tsx | 2 + umd.ts | 8 +- 9 files changed, 515 insertions(+), 32 deletions(-) create mode 100644 src/SplitFactoryProvider.tsx create mode 100644 src/__tests__/SplitFactoryProvider.test.tsx diff --git a/package-lock.json b/package-lock.json index 6e79190..99d919c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "license": "Apache-2.0", "dependencies": { - "@splitsoftware/splitio": "10.24.1", + "@splitsoftware/splitio": "10.25.1-rc.0", "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-rc.0", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", + "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", "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-rc.0", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", + "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", "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..f1608a7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.2", + "version": "1.10.3-rc.3", "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-rc.0", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, diff --git a/src/SplitFactory.tsx b/src/SplitFactory.tsx index ba2c12d..f9e5e03 100644 --- a/src/SplitFactory.tsx +++ b/src/SplitFactory.tsx @@ -15,6 +15,8 @@ import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; * 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. */ export class SplitFactory extends React.Component { diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx new file mode 100644 index 0000000..d3ac78b --- /dev/null +++ b/src/SplitFactoryProvider.tsx @@ -0,0 +1,74 @@ +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 { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; + +/** + * SplitFactoryProvider will initialize the Split SDK and its main client when it is mounted, listen for its events in order to update the Split Context, + * and automatically shutdown and release resources when it is unmounted. SplitFactoryProvider must wrap other library components and functions + * since they access the Split Context and its elements (factory, clients, 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 [stateFactory, setStateFactory] = React.useState(propFactory || null); + const factory = propFactory || stateFactory; + const client = factory ? getSplitClient(factory) : null; + + React.useEffect(() => { + if (config) { + const factory = getSplitFactory(config); + const client = getSplitClient(factory); + const status = getStatus(client); + + // Update state and unsubscribe from events when first event is emitted + const update = () => { + 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) { + 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 () => { + // Factory destroy unsubscribes from events + destroySplitFactory(factory as IFactoryWithClients); + } + } + }, [config, updateOnSdkReady, updateOnSdkReadyFromCache, updateOnSdkTimedout, updateOnSdkUpdate]); + + return ( + + ); +} diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx new file mode 100644 index 0000000..2964a60 --- /dev/null +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -0,0 +1,401 @@ +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 unmount.', () => { + let destroyMainClientSpy; + let destroySharedClientSpy; + const wrapper = render( + + {({ factory }) => { + if (!factory) return null; // 1st render + + // 2nd render (SDK ready) + expect(__factories.size).toBe(1); + destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy'); + return ( + + {({ client }) => { + destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + return null; + }} + + ); + }} + + ); + + // SDK ready to re-render + act(() => { + const factory = (SplitSdk as jest.Mock).mock.results.slice(-1)[0].value; + factory.client().__emitter__.emit(Event.SDK_READY) + }); + + wrapper.unmount(); + // the factory created by the component is removed from `factories` cache and its clients are destroyed + expect(__factories.size).toBe(0); + expect(destroyMainClientSpy).toBeCalledTimes(1); + expect(destroySharedClientSpy).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 as SplitIO.ISDK).client(), 'destroy'); + return ( + + {({ client }) => { + destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + return null; + }} + + ); + }} + + ); + wrapper.unmount(); + expect(destroyMainClientSpy).not.toBeCalled(); + expect(destroySharedClientSpy).not.toBeCalled(); + }); + +}); 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/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/withSplitFactory.tsx b/src/withSplitFactory.tsx index bd939e2..076ecea 100644 --- a/src/withSplitFactory.tsx +++ b/src/withSplitFactory.tsx @@ -9,6 +9,8 @@ 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. */ 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, }; From c59fe5e22a7b3974b044f930ff10af2bd6f4c4e8 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 13:54:52 -0300 Subject: [PATCH 14/21] Fix logs for new SplitFactoryProvider component --- .github/workflows/ci-cd.yml | 163 +++++++++++++++++++++++++ .nvmrc | 2 +- README.md | 8 +- src/SplitClient.tsx | 13 +- src/SplitTreatments.tsx | 8 +- src/__tests__/SplitClient.test.tsx | 64 +++++----- src/__tests__/SplitTreatments.test.tsx | 63 +++++----- src/constants.ts | 7 +- 8 files changed, 236 insertions(+), 92 deletions(-) create mode 100644 .github/workflows/ci-cd.yml diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml new file mode 100644 index 0000000..3870a1f --- /dev/null +++ b/.github/workflows/ci-cd.yml @@ -0,0 +1,163 @@ +name: ci + +on: + push: + branches: + - '**' + pull_request: + branches: + - master + - development + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + id-token: write + +jobs: + build: + name: Build + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up nodejs + uses: actions/setup-node@v3 + with: + node-version: 'lts/*' + cache: 'npm' + + - name: npm ci + run: npm ci + + - name: npm check + run: npm run check + + - name: npm test + run: npm run test -- --coverage + + - name: npm build + run: BUILD_BRANCH=$(echo "${GITHUB_REF#refs/heads/}") npm run build + + - name: Set VERSION env + run: echo "VERSION=$(cat package.json | jq -r .version)" >> $GITHUB_ENV + + - name: SonarQube Scan (Push) + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') + uses: SonarSource/sonarcloud-github-action@v1.9 + env: + SONAR_TOKEN: ${{ secrets.SONARQUBE_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + projectBaseDir: . + args: > + -Dsonar.host.url=${{ secrets.SONARQUBE_HOST }} + -Dsonar.projectVersion=${{ env.VERSION }} + -Dsonar.branch.name=${{ github.ref_name }} + + - name: SonarQube Scan (Pull Request) + if: github.event_name == 'pull_request' + uses: SonarSource/sonarcloud-github-action@v1.9 + env: + SONAR_TOKEN: ${{ secrets.SONARQUBE_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + projectBaseDir: . + args: > + -Dsonar.host.url=${{ secrets.SONARQUBE_HOST }} + -Dsonar.projectVersion=${{ env.VERSION }} + -Dsonar.pullrequest.key=${{ github.event.pull_request.number }} + -Dsonar.pullrequest.branch=${{ github.event.pull_request.head.ref }} + -Dsonar.pullrequest.base=${{ github.event.pull_request.base.ref }} + + - name: Store assets + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') + uses: actions/upload-artifact@v3 + with: + name: assets + path: umd/ + retention-days: 1 + + upload-stage: + name: Upload assets + runs-on: ubuntu-latest + needs: build + if: github.event_name == 'push' && github.ref == 'refs/heads/development' + strategy: + matrix: + environment: + - stage + include: + - environment: stage + account_id: "079419646996" + bucket: split-public-stage + + steps: + - name: Download assets + uses: actions/download-artifact@v3 + with: + name: assets + path: umd + + - name: Display structure of assets + run: ls -R + working-directory: umd + + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v1-node16 + with: + role-to-assume: arn:aws:iam::${{ matrix.account_id }}:role/gha-public-assets-role + aws-region: us-east-1 + + - name: Upload to S3 + run: aws s3 sync $SOURCE_DIR s3://$BUCKET/$DEST_DIR $ARGS + env: + BUCKET: ${{ matrix.bucket }} + SOURCE_DIR: ./umd + DEST_DIR: sdk + ARGS: --acl public-read --follow-symlinks --cache-control max-age=31536000,public + + upload-prod: + name: Upload assets + runs-on: ubuntu-latest + needs: build + if: github.event_name == 'push' && github.ref == 'refs/heads/master' + strategy: + matrix: + environment: + - prod + include: + - environment: prod + account_id: "825951051969" + bucket: split-public + + steps: + - name: Download assets + uses: actions/download-artifact@v3 + with: + name: assets + path: umd + + - name: Display structure of assets + run: ls -R + working-directory: umd + + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v1-node16 + with: + role-to-assume: arn:aws:iam::${{ matrix.account_id }}:role/gha-public-assets-role + aws-region: us-east-1 + + - name: Upload to S3 + run: aws s3 sync $SOURCE_DIR s3://$BUCKET/$DEST_DIR $ARGS + env: + BUCKET: ${{ matrix.bucket }} + SOURCE_DIR: ./umd + DEST_DIR: sdk + ARGS: --acl public-read --follow-symlinks --cache-control max-age=31536000,public diff --git a/.nvmrc b/.nvmrc index f274881..b009dfb 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -v16.16.0 +lts/* 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/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 { {(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__/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/constants.ts b/src/constants.ts index 7d81db7..c29d802 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -14,14 +14,11 @@ export const CONTROL_WITH_CONFIG: SplitIO.TreatmentWithConfig = { }; // 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.'; From 068f2a2b50f5f7bcd29d0ac640e8c79a00eb77cd Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 14:20:44 -0300 Subject: [PATCH 15/21] Rename ci.yml to ci-cd.yml --- .github/workflows/ci.yml | 163 --------------------------------------- 1 file changed, 163 deletions(-) delete mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index bc3a8b1..0000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,163 +0,0 @@ -name: ci - -on: - push: - branches: - - '**' - pull_request: - branches: - - master - - development - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number }} - cancel-in-progress: true - -permissions: - contents: read - id-token: write - -jobs: - build: - name: Build - runs-on: ubuntu-latest - steps: - - name: Checkout code - uses: actions/checkout@v3 - with: - fetch-depth: 0 - - - name: Set up nodejs - uses: actions/setup-node@v3 - with: - node-version: '16.16.0' - cache: 'npm' - - - name: npm ci - run: npm ci - - - name: npm check - run: npm run check - - - name: npm test - run: npm run test -- --coverage - - - name: npm build - run: BUILD_BRANCH=$(echo "${GITHUB_REF#refs/heads/}") npm run build - - - name: Set VERSION env - run: echo "VERSION=$(cat package.json | jq -r .version)" >> $GITHUB_ENV - - - name: SonarQube Scan (Push) - if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') - uses: SonarSource/sonarcloud-github-action@v1.9 - env: - SONAR_TOKEN: ${{ secrets.SONARQUBE_TOKEN }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - projectBaseDir: . - args: > - -Dsonar.host.url=${{ secrets.SONARQUBE_HOST }} - -Dsonar.projectVersion=${{ env.VERSION }} - -Dsonar.branch.name=${{ github.ref_name }} - - - name: SonarQube Scan (Pull Request) - if: github.event_name == 'pull_request' - uses: SonarSource/sonarcloud-github-action@v1.9 - env: - SONAR_TOKEN: ${{ secrets.SONARQUBE_TOKEN }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - projectBaseDir: . - args: > - -Dsonar.host.url=${{ secrets.SONARQUBE_HOST }} - -Dsonar.projectVersion=${{ env.VERSION }} - -Dsonar.pullrequest.key=${{ github.event.pull_request.number }} - -Dsonar.pullrequest.branch=${{ github.event.pull_request.head.ref }} - -Dsonar.pullrequest.base=${{ github.event.pull_request.base.ref }} - - - name: Store assets - if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/development') - uses: actions/upload-artifact@v3 - with: - name: assets - path: umd/ - retention-days: 1 - - upload-stage: - name: Upload assets - runs-on: ubuntu-latest - needs: build - if: github.event_name == 'push' && github.ref == 'refs/heads/development' - strategy: - matrix: - environment: - - stage - include: - - environment: stage - account_id: "079419646996" - bucket: split-public-stage - - steps: - - name: Download assets - uses: actions/download-artifact@v3 - with: - name: assets - path: umd - - - name: Display structure of assets - run: ls -R - working-directory: umd - - - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v1-node16 - with: - role-to-assume: arn:aws:iam::${{ matrix.account_id }}:role/gha-public-assets-role - aws-region: us-east-1 - - - name: Upload to S3 - run: aws s3 sync $SOURCE_DIR s3://$BUCKET/$DEST_DIR $ARGS - env: - BUCKET: ${{ matrix.bucket }} - SOURCE_DIR: ./umd - DEST_DIR: sdk - ARGS: --acl public-read --follow-symlinks --cache-control max-age=31536000,public - - upload-prod: - name: Upload assets - runs-on: ubuntu-latest - needs: build - if: github.event_name == 'push' && github.ref == 'refs/heads/master' - strategy: - matrix: - environment: - - prod - include: - - environment: prod - account_id: "825951051969" - bucket: split-public - - steps: - - name: Download assets - uses: actions/download-artifact@v3 - with: - name: assets - path: umd - - - name: Display structure of assets - run: ls -R - working-directory: umd - - - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v1-node16 - with: - role-to-assume: arn:aws:iam::${{ matrix.account_id }}:role/gha-public-assets-role - aws-region: us-east-1 - - - name: Upload to S3 - run: aws s3 sync $SOURCE_DIR s3://$BUCKET/$DEST_DIR $ARGS - env: - BUCKET: ${{ matrix.bucket }} - SOURCE_DIR: ./umd - DEST_DIR: sdk - ARGS: --acl public-read --follow-symlinks --cache-control max-age=31536000,public From 53a0a90785a21518f478204553f687bfaaa89a9c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 17:39:15 -0300 Subject: [PATCH 16/21] Update changelog entry --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index a69655e..dd1026b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,6 @@ 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 (issue #148). + 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). From 0b869f9a86e5094b5dc5d79bc772c7e64fef8beb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Jan 2024 14:57:53 -0300 Subject: [PATCH 17/21] Update deprecation comment to add more info --- src/SplitFactory.tsx | 6 ++++-- src/withSplitFactory.tsx | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/SplitFactory.tsx b/src/SplitFactory.tsx index f9e5e03..4beb60d 100644 --- a/src/SplitFactory.tsx +++ b/src/SplitFactory.tsx @@ -14,9 +14,11 @@ import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; * 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 the factory is ready. 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/withSplitFactory.tsx b/src/withSplitFactory.tsx index 076ecea..76f2827 100644 --- a/src/withSplitFactory.tsx +++ b/src/withSplitFactory.tsx @@ -11,6 +11,8 @@ import { SplitFactory } from './SplitFactory'; * @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 the factory is ready. 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) { From 561d83d5b3480f4cd4bf797404617b6b3c032caa Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 15 Jan 2024 17:16:46 -0300 Subject: [PATCH 18/21] Component prop update test --- src/SplitFactoryProvider.tsx | 2 +- src/__tests__/SplitFactoryProvider.test.tsx | 72 +++++++++++++++------ 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index d3ac78b..9253ce8 100644 --- a/src/SplitFactoryProvider.tsx +++ b/src/SplitFactoryProvider.tsx @@ -28,7 +28,7 @@ export function SplitFactoryProvider(props: ISplitFactoryProps) { } const [stateFactory, setStateFactory] = React.useState(propFactory || null); - const factory = propFactory || stateFactory; + const factory = propFactory || (stateFactory && config === (stateFactory as IFactoryWithClients).config ? stateFactory : null); const client = factory ? getSplitClient(factory) : null; React.useEffect(() => { diff --git a/src/__tests__/SplitFactoryProvider.test.tsx b/src/__tests__/SplitFactoryProvider.test.tsx index 2964a60..a77d4af 100644 --- a/src/__tests__/SplitFactoryProvider.test.tsx +++ b/src/__tests__/SplitFactoryProvider.test.tsx @@ -336,40 +336,74 @@ 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.', () => { + let renderTimes = 0; + const createdFactories = new Set(); + const clientDestroySpies: jest.SpyInstance[] = []; + + const Component = ({ factory, isReady, hasTimedout }: ISplitFactoryChildProps) => { + renderTimes++; + if (factory) createdFactories.add(factory); + + switch (renderTimes) { + case 1: + case 3: + expect(isReady).toBe(false); + expect(hasTimedout).toBe(false); + expect(factory).toBe(null); + return null; - // 2nd render (SDK ready) + case 2: + case 4: + expect(isReady).toBe(true); + expect(hasTimedout).toBe(true); expect(__factories.size).toBe(1); - destroyMainClientSpy = jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy'); + clientDestroySpies.push(jest.spyOn((factory as SplitIO.ISDK).client(), 'destroy')); return ( {({ client }) => { - destroySharedClientSpy = jest.spyOn(client as SplitIO.IClient, 'destroy'); + clientDestroySpies.push(jest.spyOn(client as SplitIO.IClient, 'destroy')); return null; }} ); - }} - - ); + case 5: + throw new Error('Child must not be rerendered'); + } + }; - // 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 + const wrapper = render( + + {Component} + + ); + + // 2nd render: SDK ready (timeout is ignored due to updateOnSdkTimedout=false) + act(emitSdkEvents); + + // 3rd render: Update config prop -> factory is recreated + wrapper.rerender( + + {Component} + + ); + + // 4th 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.', () => { From 1504d59f1db190ac5388b064284c976547816a08 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 16 Jan 2024 10:46:50 -0300 Subject: [PATCH 19/21] Optimize SplitFactoryProvider: do not recreate factory if only updateOn props change --- src/SplitFactoryProvider.tsx | 34 ++++++++----- src/__tests__/SplitFactoryProvider.test.tsx | 53 ++++++++++++++------- 2 files changed, 60 insertions(+), 27 deletions(-) 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; }} From 8af925cc73ce3d1c593465db174f1dea4c0917c2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 16 Jan 2024 11:55:10 -0300 Subject: [PATCH 20/21] Update comments and CHANGES file --- CHANGES.txt | 9 +++++---- package-lock.json | 18 +++++++++--------- package.json | 4 ++-- src/SplitFactory.tsx | 5 +++-- src/SplitFactoryProvider.tsx | 6 +++--- src/withSplitFactory.tsx | 3 ++- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index dd1026b..3ece218 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,8 +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). +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. diff --git a/package-lock.json b/package-lock.json index 99d919c..ebb053f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.3", + "version": "1.10.3-rc.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.3", + "version": "1.10.3-rc.4", "license": "Apache-2.0", "dependencies": { - "@splitsoftware/splitio": "10.25.1-rc.0", + "@splitsoftware/splitio": "10.25.1", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, @@ -1547,9 +1547,9 @@ } }, "node_modules/@splitsoftware/splitio": { - "version": "10.25.1-rc.0", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", - "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", + "version": "10.25.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1.tgz", + "integrity": "sha512-QmCOI2VNhjIMieibtMcc595oyQB5sgrYKSqw7wxKtwMX0VtuPbZ3Lw8fwd0nH2WSKq3Pcjyu3nVSYQRp1bGEvA==", "dependencies": { "@splitsoftware/splitio-commons": "1.13.1", "@types/google.analytics": "0.0.40", @@ -12089,9 +12089,9 @@ } }, "@splitsoftware/splitio": { - "version": "10.25.1-rc.0", - "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1-rc.0.tgz", - "integrity": "sha512-lSfNKoloima/kjtn1W/fJFj9l0WuwE+iSOKgRn13AgWdSE7vdr71qJ0ZLuwmk0HLjv52OnAgSGi0DbS+0qM6Ow==", + "version": "10.25.1", + "resolved": "https://registry.npmjs.org/@splitsoftware/splitio/-/splitio-10.25.1.tgz", + "integrity": "sha512-QmCOI2VNhjIMieibtMcc595oyQB5sgrYKSqw7wxKtwMX0VtuPbZ3Lw8fwd0nH2WSKq3Pcjyu3nVSYQRp1bGEvA==", "requires": { "@splitsoftware/splitio-commons": "1.13.1", "@types/google.analytics": "0.0.40", diff --git a/package.json b/package.json index f1608a7..0e08796 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.3", + "version": "1.10.3-rc.4", "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.25.1-rc.0", + "@splitsoftware/splitio": "10.25.1", "memoize-one": "^5.1.1", "shallowequal": "^1.1.0" }, diff --git a/src/SplitFactory.tsx b/src/SplitFactory.tsx index 4beb60d..74d9236 100644 --- a/src/SplitFactory.tsx +++ b/src/SplitFactory.tsx @@ -9,14 +9,15 @@ 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. * * @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 the factory is ready. This differs from the previous behavior where `factory` and `client` were immediately available. + * 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} */ diff --git a/src/SplitFactoryProvider.tsx b/src/SplitFactoryProvider.tsx index 4ecfd3c..5f91df7 100644 --- a/src/SplitFactoryProvider.tsx +++ b/src/SplitFactoryProvider.tsx @@ -7,9 +7,9 @@ import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClie import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient'; /** - * SplitFactoryProvider will initialize the Split SDK and its main client when it is mounted, listen for its events in order to update the Split Context, - * and automatically shutdown and release resources when it is unmounted. SplitFactoryProvider must wrap other library components and functions - * since they access the Split Context and its elements (factory, clients, etc). + * 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. diff --git a/src/withSplitFactory.tsx b/src/withSplitFactory.tsx index 76f2827..45eb55f 100644 --- a/src/withSplitFactory.tsx +++ b/src/withSplitFactory.tsx @@ -12,7 +12,8 @@ import { SplitFactory } from './SplitFactory'; * * @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 the factory is ready. This differs from the previous behavior where `factory` and `client` were immediately available. + * 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) { From b56d33d919127c583dada73e06c6274f722e93d8 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 16 Jan 2024 12:18:44 -0300 Subject: [PATCH 21/21] Prepare stable version --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index ebb053f..377cab9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.4", + "version": "1.11.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.4", + "version": "1.11.0", "license": "Apache-2.0", "dependencies": { "@splitsoftware/splitio": "10.25.1", diff --git a/package.json b/package.json index 0e08796..9085bdd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-react", - "version": "1.10.3-rc.4", + "version": "1.11.0", "description": "A React library to easily integrate and use Split JS SDK", "main": "lib/index.js", "module": "es/index.js",