From 24edb89509ca0b634780f8364d7ae00af2ac0591 Mon Sep 17 00:00:00 2001 From: Cal Courtney Date: Fri, 16 Feb 2024 11:23:51 +0000 Subject: [PATCH 1/3] Upgrade React and add coverage to tests --- .gitignore | 2 ++ package.json | 10 +++++----- yarn.lock | 43 ++++++++++++++++++++++--------------------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/.gitignore b/.gitignore index d5a6e4a..07d77d6 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ /example /next-example .idea + +coverage \ No newline at end of file diff --git a/package.json b/package.json index f780219..e666472 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "build:esm": "tsc", "build:cjs": "tsc --module commonjs --outDir lib/cjs", "dev": "tsc -w", - "test": "jest" + "test": "jest --coverage" }, "devDependencies": { "@testing-library/jest-dom": "^5.16.5", @@ -34,12 +34,12 @@ "@testing-library/user-event": "^14.4.3", "@types/core-js": "^2.5.5", "@types/jest": "^29.2.3", - "@types/react": "^18.0.25", - "@types/react-dom": "^18.0.9", + "@types/react": "^18.2.55", + "@types/react-dom": "^18.2.19", "jest": "^29.3.1", "jest-environment-jsdom": "^29.3.1", - "react": "18.2", - "react-dom": "18.2", + "react": "^18.2.0", + "react-dom": "^18.2.0", "ts-jest": "^29.0.1", "ts-node": "^10.9.1", "typescript": "^4.8.3" diff --git a/yarn.lock b/yarn.lock index 59dce39..61365d9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -758,14 +758,14 @@ resolved "https://registry.yarnpkg.com/@types/prop-types/-/prop-types-15.7.11.tgz#2596fb352ee96a1379c657734d4b913a613ad563" integrity sha512-ga8y9v9uyeiLdpKddhxYQkxNDrfvuPrlFb0N1qnZZByvcElJaXthF1UhvCh9TLWJBEHeNtdnbysW7Y6Uq8CVng== -"@types/react-dom@^18.0.0", "@types/react-dom@^18.0.9": +"@types/react-dom@^18.0.0", "@types/react-dom@^18.2.19": version "18.2.19" resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.2.19.tgz#b84b7c30c635a6c26c6a6dfbb599b2da9788be58" integrity sha512-aZvQL6uUbIJpjZk4U8JZGbau9KDeAwMfmhyWorxgBkqDIEf6ROjRozcmPIicqsUwPUjbkDfHKgGee1Lq65APcA== dependencies: "@types/react" "*" -"@types/react@*", "@types/react@^18.0.25": +"@types/react@*", "@types/react@^18.2.55": version "18.2.55" resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.55.tgz#38141821b7084404b5013742bc4ae08e44da7a67" integrity sha512-Y2Tz5P4yz23brwm2d7jNon39qoAtMMmalOQv6+fEFt1mT+FcM3D841wDpoUvFXhaYenuROCy3FZYqdTjM7qVyA== @@ -1056,9 +1056,9 @@ camelcase@^6.2.0: integrity sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA== caniuse-lite@^1.0.30001580: - version "1.0.30001585" - resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001585.tgz#0b4e848d84919c783b2a41c13f7de8ce96744401" - integrity sha512-yr2BWR1yLXQ8fMpdS/4ZZXpseBgE7o4g41x3a6AJOqZuOi+iE/WdJYAuZ6Y95i4Ohd2Y+9MzIWRR+uGABH4s3Q== + version "1.0.30001587" + resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001587.tgz#a0bce920155fa56a1885a69c74e1163fc34b4881" + integrity sha512-HMFNotUmLXn71BQxg8cijvqxnIAofforZOwGsxyXJ0qugTdspUF4sPSJ2vhgprHCB996tIDzEq1ubumPDV8ULA== chalk@^2.4.2: version "2.4.2" @@ -1284,7 +1284,7 @@ define-data-property@^1.0.1, define-data-property@^1.1.2: gopd "^1.0.1" has-property-descriptors "^1.0.1" -define-properties@^1.1.3, define-properties@^1.2.0, define-properties@^1.2.1: +define-properties@^1.1.3, define-properties@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/define-properties/-/define-properties-1.2.1.tgz#10781cc616eb951a80a034bafcaa7377f6af2b6c" integrity sha512-8QmQKqEASLd5nx0U1B1okLElbUuuttJ/AnYmRXbbbGDWh6uS208EjD4Xqq/I9wK7u0v6O08XhTWnt5XtEbR6Dg== @@ -1331,9 +1331,9 @@ domexception@^4.0.0: webidl-conversions "^7.0.0" electron-to-chromium@^1.4.648: - version "1.4.664" - resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.664.tgz#b00fc67d5d4f124e429b0dcce5a02ae18ef33ede" - integrity sha512-k9VKKSkOSNPvSckZgDDl/IQx45E1quMjX8QfLzUsAs/zve8AyFDK+ByRynSP/OfEfryiKHpQeMf00z0leLCc3A== + version "1.4.665" + resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.665.tgz#681700bd590b0e5a3be66e3e2874ce62abcf5da5" + integrity sha512-UpyCWObBoD+nSZgOC2ToaIdZB0r9GhqT2WahPKiSki6ckkSuKhQNso8V2PrFcHBMleI/eqbKgVQgVC4Wni4ilw== emittery@^0.13.1: version "0.13.1" @@ -1612,9 +1612,9 @@ has-tostringtag@^1.0.0, has-tostringtag@^1.0.1: has-symbols "^1.0.3" hasown@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/hasown/-/hasown-2.0.0.tgz#f4c513d454a57b7c7e1650778de226b11700546c" - integrity sha512-vUptKVTpIJhcczKBbgnS+RtcuYMB8+oNzPK2/Hp3hanz8JmpATdmmgLgSaadVREkDm+e2giHwY3ZRkyjSIDDFA== + version "2.0.1" + resolved "https://registry.yarnpkg.com/hasown/-/hasown-2.0.1.tgz#26f48f039de2c0f8d3356c223fb8d50253519faa" + integrity sha512-1/th4MHjnwncwXsIW6QMzlvYL9kG5e/CpVvLRZe4XPa8TOUNbCELqmvhDmnkNsAjwaG4+I8gJJL0JBvTTLO9qA== dependencies: function-bind "^1.1.2" @@ -2669,7 +2669,7 @@ querystringify@^2.1.1: resolved "https://registry.yarnpkg.com/querystringify/-/querystringify-2.2.0.tgz#3345941b4153cb9d082d8eee4cda2016a9aef7f6" integrity sha512-FIqgj2EUvTa7R50u0rGsyTftzjYmv/a3hO345bZNrqabNqjtgiDMgmo4mkUjd+nzU5oF3dClKqFIPUKybUyqoQ== -react-dom@18.2: +react-dom@^18.2.0: version "18.2.0" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.2.0.tgz#22aaf38708db2674ed9ada224ca4aa708d821e3d" integrity sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g== @@ -2687,7 +2687,7 @@ react-is@^18.0.0: resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.2.0.tgz#199431eeaaa2e09f86427efbb4f1473edb47609b" integrity sha512-xWGDIW6x921xtzPkhiULtthJHoJvBbF3q26fzloPCK0hsvxtPVelvftw3zjbHWSkR2km9Z+4uxbDDK/6Zw9B8w== -react@18.2: +react@^18.2.0: version "18.2.0" resolved "https://registry.yarnpkg.com/react/-/react-18.2.0.tgz#555bd98592883255fa00de14f1151a917b5d77d5" integrity sha512-/3IjMdb2L9QbBdWiW5e3P2/npwMBaU9mHCSCUzNln0ZCYbcfTsGbTJrU/kGemdH2IWmB2ioZ+zkxtmq6g09fGQ== @@ -2708,13 +2708,14 @@ regenerator-runtime@^0.14.0: integrity sha512-dYnhHh0nJoMfnkZs6GmmhFknAGRrLznOu5nc9ML+EJxGvrx6H7teuevqVqCuPcPK//3eDrrjQhehXVx9cnkGdw== regexp.prototype.flags@^1.5.1: - version "1.5.1" - resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.5.1.tgz#90ce989138db209f81492edd734183ce99f9677e" - integrity sha512-sy6TXMN+hnP/wMy+ISxg3krXx7BAtWVO4UouuCN/ziM9UEne0euamVNafDfvC83bRNr95y0V5iijeDQFUNpvrg== + version "1.5.2" + resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.5.2.tgz#138f644a3350f981a858c44f6bb1a61ff59be334" + integrity sha512-NcDiDkTLuPR+++OCKB0nWafEmhg/Da8aUPLPMQbK+bxKKCm1/S5he+AqYa4PlMCVBalb4/yxIRub6qkEx5yJbw== dependencies: - call-bind "^1.0.2" - define-properties "^1.2.0" - set-function-name "^2.0.0" + call-bind "^1.0.6" + define-properties "^1.2.1" + es-errors "^1.3.0" + set-function-name "^2.0.1" require-directory@^2.1.1: version "2.1.1" @@ -2800,7 +2801,7 @@ set-function-length@^1.2.0: gopd "^1.0.1" has-property-descriptors "^1.0.1" -set-function-name@^2.0.0: +set-function-name@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/set-function-name/-/set-function-name-2.0.1.tgz#12ce38b7954310b9f61faa12701620a0c882793a" integrity sha512-tMNCiqYVkXIZgc2Hnoy2IvC/f8ezc5koaRFkCjrpWzGpCd3qbZXPzVy9MAZzK1ch/X0jvSkojys3oqJN0qCmdA== From f8c893a6e31d2c7e089a462a3fae9b6b1993a02f Mon Sep 17 00:00:00 2001 From: Cal Courtney Date: Fri, 16 Feb 2024 12:56:36 +0000 Subject: [PATCH 2/3] Allow all ReactNode elements as children --- src/components/Treatment/Treatment.tsx | 126 ++++++-------- tests/SDKProvider.spec.tsx | 229 +++++++++++++------------ tests/Treatment.spec.tsx | 26 ++- 3 files changed, 195 insertions(+), 186 deletions(-) diff --git a/src/components/Treatment/Treatment.tsx b/src/components/Treatment/Treatment.tsx index 90d1879..1da470a 100644 --- a/src/components/Treatment/Treatment.tsx +++ b/src/components/Treatment/Treatment.tsx @@ -1,24 +1,9 @@ -import React, { - cloneElement, - FC, - ReactElement, - ReactNode, - useEffect, - useState, -} from "react"; +import React, { FC, ReactNode, useEffect, useState } from "react"; import absmartly from "@absmartly/javascript-sdk"; import { Char } from "../../types"; import { convertLetterToNumber } from "../../utils/convertLetterToNumber"; -interface TreatmentProps { - name: string; - context: typeof absmartly.Context; - attributes?: Record; - loadingComponent?: ReactNode; - children?: ReactNode; -} - interface TreatmentFunctionProps { name: string; context: typeof absmartly.Context; @@ -27,14 +12,7 @@ interface TreatmentFunctionProps { children(variantAndVariables: { variant: number; variables: Record; - }): ReactNode | ReactNode; -} - -interface TreatmentVariantProps { - variant: number | Char | undefined; - name?: string; - context?: typeof absmartly.Context; - children?: ReactNode; + }): ReactNode; } export const TreatmentFunction: FC = ({ @@ -53,9 +31,7 @@ export const TreatmentFunction: FC = ({ variant: !loadingComponent ? 0 : undefined, variables: {}, }); - const [loading, setLoading] = useState( - context && !context.isReady() - ); + const [loading, setLoading] = useState(!context.isReady()); // Set variant number and variables in state useEffect(() => { @@ -64,9 +40,9 @@ export const TreatmentFunction: FC = ({ context .ready() .then(() => { - //Turning the variable keys and values into an array of arrays + // Turning the variable keys and values into an array of arrays const variablesArray = Object.keys(context.variableKeys()).map( - (key) => [key, context.peekVariableValue(key)] + (key) => [key, context.peekVariableValue(key, "")] ); // Converting the array of arrays into a regular object @@ -85,26 +61,34 @@ export const TreatmentFunction: FC = ({ setLoading(false); }) .catch((e: Error) => console.error(e)); - }, [context]); + }, [context, attributes]); + + if (loading) { + return loadingComponent != null ? ( + <>{loadingComponent} + ) : ( + <>{children({ ...variantAndVariables, variant: 0 })} + ); + } return ( - - {loading - ? loadingComponent - ? loadingComponent - : children({ variant: 0, variables: variantAndVariables.variables }) - : children({ - variant: variantAndVariables.variant || 0, - variables: variantAndVariables.variables, - })} - + <> + {children({ + ...variantAndVariables, + variant: variantAndVariables.variant ?? 0, + })} + ); }; +interface TreatmentProps { + name: string; + context: typeof absmartly.Context; + attributes?: Record; + loadingComponent?: ReactNode; + children?: ReactNode; +} + export const Treatment: FC = ({ children, loadingComponent, @@ -116,27 +100,33 @@ export const Treatment: FC = ({ context && !context.isReady() ); - // Turning the children array into objects and mapping them as variants - // and indexes - const childrenInfo = React.Children.map(children, (child, i) => { + // Turning the children into an array of objects and mapping them as variants + const childrenInfo = React.Children.map(children, (child) => { const obj = child?.valueOf() as { props: { variant: number | Char }; }; - return { variant: obj.props.variant, index: i }; + return { variant: obj.props.variant }; }); // Get the index of the first child with a variant matching the context treatment const getSelectedChildIndex = (context: typeof absmartly.Context) => { const treatment = context.treatment(name); - return childrenInfo?.filter( - (item) => convertLetterToNumber(item.variant) === (treatment || 0) - )[0]?.index; + + const index = childrenInfo?.findIndex( + (x) => convertLetterToNumber(x.variant) === (treatment || 0) + ); + + if (index === -1) { + return 0; + } + + return index ?? 0; }; // The index of the selected variant in the children array - const [selectedTreatment, setSelectedTreatment] = useState< - number | undefined - >(() => (context?.isReady() ? getSelectedChildIndex(context) : undefined)); + const [selectedTreatment, setSelectedTreatment] = useState( + context?.isReady() ? getSelectedChildIndex(context) : null + ); // Making the children prop into an array for selecting a single element later. const childrenArray = React.Children.toArray(children); @@ -155,28 +145,22 @@ export const Treatment: FC = ({ setLoading(false); }) .catch((e: Error) => console.error(e)); - }, [context]); + }, [context, attributes]); - // Return the selected Treatment (Or treatment 0 or loading component) + // Return the selected Treatment if (loading) { - if (loadingComponent) return loadingComponent as ReactElement; - return childrenArray[0] as ReactElement; + if (loadingComponent) return <>{loadingComponent}; + return <>{childrenArray[0]}; } - return cloneElement(childrenArray[selectedTreatment || 0] as ReactElement, { - context, - name, - }); + return <>{childrenArray[selectedTreatment || 0]}; }; +interface TreatmentVariantProps { + variant: number | Char | undefined; + children?: ReactNode; +} + export const TreatmentVariant: FC = ({ children }) => { - return ( - <> - {React.Children.map(children, (child) => { - if (child) - return cloneElement(child as ReactElement) - return null; - })} - - ); + return <>{children}; }; diff --git a/tests/SDKProvider.spec.tsx b/tests/SDKProvider.spec.tsx index 1f85d59..3c5c3e2 100644 --- a/tests/SDKProvider.spec.tsx +++ b/tests/SDKProvider.spec.tsx @@ -1,6 +1,12 @@ import React from "react"; import "@testing-library/jest-dom"; -import { screen, act, fireEvent, render, renderHook } from "@testing-library/react"; +import { + act, + fireEvent, + render, + renderHook, + screen, +} from "@testing-library/react"; import { Context, SDK } from "@absmartly/javascript-sdk"; @@ -9,130 +15,133 @@ import { SDKProvider, useABSmartly } from "../src/components/SDKProvider"; jest.mock("@absmartly/javascript-sdk"); const mockContextData = { - experiments: [], + experiments: [], }; const mockCreateContext = jest.fn().mockImplementation(() => { - return { - ...new Context(), - data: jest.fn().mockReturnValue(mockContextData), - }; + return { + ...new Context(), + data: jest.fn().mockReturnValue(mockContextData), + }; }); const mockCreateContextWith = jest.fn().mockImplementation(() => { - return new Context(); + return new Context(); }); SDK.mockImplementation(() => { - return { - createContext: mockCreateContext, - createContextWith: mockCreateContextWith, - attributes: jest.fn().mockImplementation(), - overrides: jest.fn().mockImplementation(), - }; + return { + createContext: mockCreateContext, + createContextWith: mockCreateContextWith, + attributes: jest.fn().mockImplementation(), + overrides: jest.fn().mockImplementation(), + }; }); describe("SDKProvider", () => { - const TestComponent = jest.fn(); - - const context = { - test: 2, - }; - - const attrs = { - attr1: "value1", - attr2: "value2", - }; - - const overrides = { - not_found: 2, - }; - - const sdkOptions = { - endpoint: "https://sandbox.absmartly.io/v1", - apiKey: "salkjdhclkjsdbca", - application: "www", - environment: "Environment 5", - retries: 5, - timeout: 3000, - overrides: overrides, - attributes: attrs, - data: mockContextData, - context: context, - }; - - const contextOptions = { - units: { - user_id: "sdchjbaiclrbkj", - anonymous_id: "sdchjbaiclrbkj", - }, + const TestComponent = jest.fn(); + + const context = { + test: 2, + }; + + const attrs = { + attr1: "value1", + attr2: "value2", + }; + + const overrides = { + not_found: 2, + }; + + const sdkOptions = { + endpoint: "https://sandbox.absmartly.io/v1", + apiKey: "salkjdhclkjsdbca", + application: "www", + environment: "Environment 5", + retries: 5, + timeout: 3000, + overrides: overrides, + attributes: attrs, + data: mockContextData, + context: context, + }; + + const contextOptions = { + units: { + user_id: "sdchjbaiclrbkj", + anonymous_id: "sdchjbaiclrbkj", + }, + }; + + test("Whether it creates an instance of the ABSmartly JS-SDK and an ABSmartly Context", async () => { + render( + + + + ); + + expect(SDK).toBeCalledTimes(1); + expect(SDK).toHaveBeenLastCalledWith(sdkOptions); + + expect(mockCreateContext).toBeCalledTimes(1); + expect(mockCreateContext).toHaveBeenLastCalledWith(contextOptions); + }); + + test("Whether it will create an SDK instance with a context that has prefetched context data", async () => { + render( + + + + ); + + expect(SDK).not.toBeCalled(); + expect(mockCreateContext).not.toBeCalled(); + }); + + test("Whether useABSmartly hook works", async () => { + const { result } = renderHook(() => useABSmartly()); + + expect(result.current.context).toBeUndefined(); + expect(result.current.sdk).toBeUndefined(); + }); + + test("resetContext function works as expected", async () => { + const TestComponent = () => { + const { resetContext } = useABSmartly(); + return ( + + ); }; - test("Whether it creates an instance of the ABSmartly JS-SDK and an ABSmartly Context", async () => { - render( - - - - ); - - expect(SDK).toBeCalledTimes(1); - expect(SDK).toHaveBeenLastCalledWith(sdkOptions); - - expect(mockCreateContext).toBeCalledTimes(1); - expect(mockCreateContext).toHaveBeenLastCalledWith(contextOptions); - }); - - test("Whether it will create an SDK instance with a context that has prefetched context data", async () => { - render( - - - - ); + render( + + + + ); - expect(SDK).not.toBeCalled(); - expect(mockCreateContext).not.toBeCalled(); + const button = screen.getByText("Reset Context"); + await act(async () => { + fireEvent.click(button); }); - test("Whether useABSmartly hook works", async () => { - const { result } = renderHook(() => useABSmartly()); - - expect(result.current.context).toBeUndefined(); - expect(result.current.sdk).toBeUndefined(); - }); - - test("resetContext function works as expected", async () => { - const TestComponent = () => { - const { resetContext } = useABSmartly(); - return ( - - ); - }; - - render( - - - - ); - - const button = screen.getByText("Reset Context"); - await act(async () => { - fireEvent.click(button); - }); - - expect(mockCreateContextWith).toHaveBeenCalledTimes(1); - expect(mockCreateContextWith).toHaveBeenCalledWith( - { units: { user_id: "newUserID" } }, - mockContextData, - { - publishDelay: 5000, - refreshPeriod: 5000 - } - ); - }); + expect(mockCreateContextWith).toHaveBeenCalledTimes(1); + expect(mockCreateContextWith).toHaveBeenCalledWith( + { units: { user_id: "newUserID" } }, + mockContextData, + { + publishDelay: 5000, + refreshPeriod: 5000, + } + ); + }); }); diff --git a/tests/Treatment.spec.tsx b/tests/Treatment.spec.tsx index c78449f..2cb2f79 100644 --- a/tests/Treatment.spec.tsx +++ b/tests/Treatment.spec.tsx @@ -1,11 +1,7 @@ import React from "react"; import { cleanup, render, waitFor } from "@testing-library/react"; -import { - Treatment, - TreatmentFunction, - TreatmentVariant, -} from "../src/components/Treatment"; +import { Treatment, TreatmentFunction, TreatmentVariant } from "../src"; import { Char, TreatmentProps } from "../src/types"; jest.mock("@absmartly/javascript-sdk"); @@ -256,6 +252,16 @@ describe("Treatment Component (TreatmentVariants as children)", () => { expect(TestComponent).toHaveBeenCalledTimes(1); }); }); + + it("should accept strings as TreatmentVariant children", async () => { + mocks.context.treatment.mockReturnValue(0); + + render( + + Hello world + + ); + }); }); describe("TreatmentFunction Component", () => { @@ -442,4 +448,14 @@ describe("TreatmentFunction Component", () => { }); } ); + + it("should accept a string as a child", async () => { + mocks.context.treatment.mockReturnValue(1); + + render( + + {({ variant }: TreatmentProps) => variant === 1 && "Hello world"} + + ); + }); }); From d83e46330a5f9a4fe8540eb73ed3d34808c8650e Mon Sep 17 00:00:00 2001 From: Cal Courtney Date: Fri, 16 Feb 2024 12:57:54 +0000 Subject: [PATCH 3/3] Bump version to v1.2.6 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e666472..fcec3b6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@absmartly/react-sdk", - "version": "1.2.5", + "version": "1.2.6", "homepage": "https://github.com/absmartly/react-sdk#README.md", "bugs": "https://github.com/absmartly/react-sdk/issues", "keywords": [