From 1634c343341c4a1c6534bee1f341a163685a9397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Thu, 26 Sep 2024 16:23:50 +0200 Subject: [PATCH 1/6] feat(packages/sui-pde): upgrade optimizely version and add use-decision hook --- package-lock.json | 135 ++++++++---------- packages/sui-pde/package.json | 2 +- packages/sui-pde/src/adapters/default.js | 4 + .../sui-pde/src/adapters/optimizely/index.js | 21 ++- .../src/adapters/optimizely/multiple.js | 4 + .../src/hooks/common/platformStrategies.js | 6 + packages/sui-pde/src/hooks/useDecision.js | 58 ++++++++ packages/sui-pde/src/index.js | 1 + packages/sui-pde/src/pde.js | 9 ++ 9 files changed, 160 insertions(+), 80 deletions(-) create mode 100644 packages/sui-pde/src/hooks/useDecision.js diff --git a/package-lock.json b/package-lock.json index 69251490f..46a3706fb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4132,40 +4132,26 @@ "integrity": "sha512-Aq58f5HiWdyDlFffbbSjAlv596h/cOnt2DO1w3DOC7OJ5EHs0hd/nycJfiu9RJbT6Yk6F1knnRRXNSpxoIVZ9Q==", "license": "MIT" }, - "node_modules/@optimizely/js-sdk-datafile-manager": { - "version": "0.9.5", - "resolved": "https://registry.npmjs.org/@optimizely/js-sdk-datafile-manager/-/js-sdk-datafile-manager-0.9.5.tgz", - "integrity": "sha512-O4ujr1nBBAQBtx8YoKNpzzaEZgsE+aU4dxubT17ePqv/YVUWE+JOY21tSRrqZy/BlbbyzL+ElT8hrGB5ZzVoIQ==", - "license": "Apache-2.0", + "node_modules/@optimizely/optimizely-sdk": { + "version": "5.3.4", + "resolved": "https://registry.npmjs.org/@optimizely/optimizely-sdk/-/optimizely-sdk-5.3.4.tgz", + "integrity": "sha512-N9BVFBoWY//cgrZu4dnUCXbbvFtx8bJURvsvQurCqdKn0pqAawDbWpm4mDTl8H3W5J4fXC5s+8xlDywiGHCY6Q==", "dependencies": { - "@optimizely/js-sdk-logging": "^0.3.1", - "@optimizely/js-sdk-utils": "^0.4.0", - "decompress-response": "^4.2.1" + "decompress-response": "^4.2.1", + "json-schema": "^0.4.0", + "murmurhash": "^2.0.1", + "ua-parser-js": "^1.0.37", + "uuid": "^9.0.1" }, "engines": { - "node": ">=8.0.0" - }, - "peerDependencies": { - "@react-native-async-storage/async-storage": "^1.2.0" - }, - "peerDependenciesMeta": { - "@react-native-async-storage/async-storage": { - "optional": true - } - } - }, - "node_modules/@optimizely/js-sdk-event-processor": { - "version": "0.9.5", - "resolved": "https://registry.npmjs.org/@optimizely/js-sdk-event-processor/-/js-sdk-event-processor-0.9.5.tgz", - "integrity": "sha512-g5zqAjJuexxgbNvn7dacFkQXQxH3+OtjELfmSswvhxP9EHkyNR0ZdQF/kBxFxr335F2/RRPvAJ9tQBPkwaBg8g==", - "license": "Apache-2.0", - "dependencies": { - "@optimizely/js-sdk-logging": "^0.3.1", - "@optimizely/js-sdk-utils": "^0.4.0" + "node": ">=14.0.0" }, "peerDependencies": { + "@babel/runtime": "^7.0.0", "@react-native-async-storage/async-storage": "^1.2.0", - "@react-native-community/netinfo": "5.9.4" + "@react-native-community/netinfo": "^11.3.2", + "fast-text-encoding": "^1.0.6", + "react-native-get-random-values": "^1.11.0" }, "peerDependenciesMeta": { "@react-native-async-storage/async-storage": { @@ -4173,42 +4159,38 @@ }, "@react-native-community/netinfo": { "optional": true + }, + "fast-text-encoding": { + "optional": true + }, + "react-native-get-random-values": { + "optional": true } } }, - "node_modules/@optimizely/js-sdk-logging": { - "version": "0.3.1", - "resolved": "https://registry.npmjs.org/@optimizely/js-sdk-logging/-/js-sdk-logging-0.3.1.tgz", - "integrity": "sha512-K71Jf283FP0E4oXehcXTTM3gvgHZHr7FUrIsw//0mdJlotHJT4Nss4hE0CWPbBxO7LJAtwNnO+VIA/YOcO4vHg==", - "license": "Apache-2.0", - "dependencies": { - "@optimizely/js-sdk-utils": "^0.4.0" - } - }, - "node_modules/@optimizely/js-sdk-utils": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/@optimizely/js-sdk-utils/-/js-sdk-utils-0.4.0.tgz", - "integrity": "sha512-QG2oytnITW+VKTJK+l0RxjaS5VrA6W+AZMzpeg4LCB4Rn4BEKtF+EcW/5S1fBDLAviGq/0TLpkjM3DlFkJ9/Gw==", - "license": "Apache-2.0", - "dependencies": { - "uuid": "^3.3.2" - } - }, - "node_modules/@optimizely/optimizely-sdk": { - "version": "4.9.4", - "resolved": "https://registry.npmjs.org/@optimizely/optimizely-sdk/-/optimizely-sdk-4.9.4.tgz", - "integrity": "sha512-aYxndR6RahnLdX7SQR1YO2dklfNjbCGUUvRaYJZ50LIsDqhkAu426vxHwYO+V+QJxqipypPG5SVdG1m32AgDvw==", - "license": "Apache-2.0", - "dependencies": { - "@optimizely/js-sdk-datafile-manager": "^0.9.5", - "@optimizely/js-sdk-event-processor": "^0.9.2", - "@optimizely/js-sdk-logging": "^0.3.1", - "json-schema": "^0.4.0", - "murmurhash": "0.0.2", - "uuid": "^3.3.2" + "node_modules/@optimizely/optimizely-sdk/node_modules/ua-parser-js": { + "version": "1.0.39", + "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-1.0.39.tgz", + "integrity": "sha512-k24RCVWlEcjkdOxYmVJgeD/0a1TiSpqLg+ZalVGV9lsnr4yqu0w7tX/x2xX6G4zpkgQnRf89lxuZ1wsbjXM8lw==", + "funding": [ + { + "type": "opencollective", + "url": "https://opencollective.com/ua-parser-js" + }, + { + "type": "paypal", + "url": "https://paypal.me/faisalman" + }, + { + "type": "github", + "url": "https://github.com/sponsors/faisalman" + } + ], + "bin": { + "ua-parser-js": "script/cli.js" }, "engines": { - "node": ">=8.0.0" + "node": "*" } }, "node_modules/@pact-foundation/pact": { @@ -10175,7 +10157,6 @@ "version": "4.2.1", "resolved": "https://registry.npmjs.org/decompress-response/-/decompress-response-4.2.1.tgz", "integrity": "sha512-jOSne2qbyE+/r8G1VU+G/82LBs2Fs4LAsTiLSHOCOMZQl2OKZ6i8i4IyHemTe+/yIXOtTcRQMzPcgyhoFlqPkw==", - "license": "MIT", "dependencies": { "mimic-response": "^2.0.0" }, @@ -20281,7 +20262,6 @@ "version": "2.1.0", "resolved": "https://registry.npmjs.org/mimic-response/-/mimic-response-2.1.0.tgz", "integrity": "sha512-wXqjST+SLt7R009ySCglWBCFpjUygmCIfD790/kVbiGmUgfYGuB14PiTd5DwVxSV4NcYHjzMkoj5LjQZwTQLEA==", - "license": "MIT", "engines": { "node": ">=8" }, @@ -20911,12 +20891,9 @@ } }, "node_modules/murmurhash": { - "version": "0.0.2", - "resolved": "https://registry.npmjs.org/murmurhash/-/murmurhash-0.0.2.tgz", - "integrity": "sha512-LKlwdZKWzvCQpMszb2HO5leJ7P9T4m5XuDKku8bM0uElrzqK9cn0+iozwQS8jO4SNjrp4w7olalgd8WgsIjhWA==", - "engines": { - "node": "*" - } + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/murmurhash/-/murmurhash-2.0.1.tgz", + "integrity": "sha512-5vQEh3y+DG/lMPM0mCGPDnyV8chYg/g7rl6v3Gd8WMF9S429ox3Xk8qrk174kWhG767KQMqqxLD1WnGd77hiew==" }, "node_modules/mute-stream": { "version": "0.0.8", @@ -26852,13 +26829,15 @@ } }, "node_modules/uuid": { - "version": "3.4.0", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", - "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", - "deprecated": "Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.", - "license": "MIT", + "version": "9.0.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-9.0.1.tgz", + "integrity": "sha512-b+1eJOlsR9K8HJpow9Ok3fiWOWSIcIzXodvv0rQjVoOVNpWMpxf1wZNpt4y9h10odCNrqnYp1OBzRktckBe3sA==", + "funding": [ + "https://github.com/sponsors/broofa", + "https://github.com/sponsors/ctavan" + ], "bin": { - "uuid": "bin/uuid" + "uuid": "dist/bin/uuid" } }, "node_modules/v8-compile-cache": { @@ -28749,7 +28728,7 @@ }, "packages/sui-jest": { "name": "@s-ui/jest", - "version": "1.2.0", + "version": "1.3.0", "license": "MIT", "dependencies": { "@swc/jest": "0.2.24", @@ -28893,7 +28872,7 @@ }, "packages/sui-js": { "name": "@s-ui/js", - "version": "2.34.0", + "version": "2.35.0", "license": "MIT", "dependencies": { "bowser": "2.11.0", @@ -28954,7 +28933,7 @@ }, "packages/sui-lint": { "name": "@s-ui/lint", - "version": "4.49.0", + "version": "4.50.0", "hasInstallScript": true, "license": "MIT", "dependencies": { @@ -29400,7 +29379,7 @@ "version": "2.25.0", "license": "MIT", "dependencies": { - "@optimizely/optimizely-sdk": "4.9.4", + "@optimizely/optimizely-sdk": "5.3.4", "@s-ui/js": "2" }, "devDependencies": { @@ -29659,7 +29638,7 @@ }, "packages/sui-segment-wrapper": { "name": "@s-ui/segment-wrapper", - "version": "4.1.0", + "version": "4.3.0", "license": "ISC", "dependencies": { "@s-ui/js": "2", diff --git a/packages/sui-pde/package.json b/packages/sui-pde/package.json index 9ce8e0941..916c1417a 100644 --- a/packages/sui-pde/package.json +++ b/packages/sui-pde/package.json @@ -17,7 +17,7 @@ "author": "", "license": "MIT", "dependencies": { - "@optimizely/optimizely-sdk": "4.9.4", + "@optimizely/optimizely-sdk": "5.3.4", "@s-ui/js": "2" }, "peerDependencies": { diff --git a/packages/sui-pde/src/adapters/default.js b/packages/sui-pde/src/adapters/default.js index b5c8eb6a0..c89c0f185 100644 --- a/packages/sui-pde/src/adapters/default.js +++ b/packages/sui-pde/src/adapters/default.js @@ -15,6 +15,10 @@ export default class DefaultAdapter { return null } + decide() { + return null + } + updateConsents() { return null } diff --git a/packages/sui-pde/src/adapters/optimizely/index.js b/packages/sui-pde/src/adapters/optimizely/index.js index 1f784d3d5..3ba274c50 100644 --- a/packages/sui-pde/src/adapters/optimizely/index.js +++ b/packages/sui-pde/src/adapters/optimizely/index.js @@ -62,12 +62,14 @@ export default class OptimizelyAdapter { sdkKey = undefined } + const isServer = typeof window === 'undefined' const optimizelyInstance = optimizely.createInstance({ sdkKey, datafileOptions: options, datafile, eventDispatcher, - ...DEFAULT_EVENTS_OPTIONS + ...DEFAULT_EVENTS_OPTIONS, + defaultDecideOptions: isServer ? [optimizely.OptimizelyDecideOption.DISABLE_DECISION_EVENT] : [] }) return optimizelyInstance @@ -117,6 +119,23 @@ export default class OptimizelyAdapter { }) } + /** + * @param {Object} params + * @param {string} params.name + * @param {object} [params.attributes] + * @returns {string=} variation name + */ + decide({name, attributes}) { + if (!this._hasUserConsents) return null + + const user = this._optimizely.createUserContext(this._userId, { + ...this._applicationAttributes, + ...attributes + }) + + return user.decide(name) + } + /** * Gets the variation without tracking the impression * @param {Object} params diff --git a/packages/sui-pde/src/adapters/optimizely/multiple.js b/packages/sui-pde/src/adapters/optimizely/multiple.js index a259a7a00..741afcb08 100644 --- a/packages/sui-pde/src/adapters/optimizely/multiple.js +++ b/packages/sui-pde/src/adapters/optimizely/multiple.js @@ -60,6 +60,10 @@ class MultipleOptimizelyAdapter { return this.#adapters[adapterId].activateExperiment(props) } + decide({adapterId = defaultAdapterId, ...props}) { + return this.#adapters[adapterId].decide(props) + } + getVariation({adapterId = defaultAdapterId, ...props}) { return this.#adapters[adapterId].getVariation(props) } diff --git a/packages/sui-pde/src/hooks/common/platformStrategies.js b/packages/sui-pde/src/hooks/common/platformStrategies.js index f56b4a540..c8a6842a5 100644 --- a/packages/sui-pde/src/hooks/common/platformStrategies.js +++ b/packages/sui-pde/src/hooks/common/platformStrategies.js @@ -6,6 +6,9 @@ const getServerStrategy = () => ({ getVariation: ({pde, experimentName, attributes, adapterId}) => { return pde.getVariation({pde, name: experimentName, attributes, adapterId}) }, + decide: ({pde, name, attributes, adapterId}) => { + return pde.decide({pde, name, attributes, adapterId}) + }, trackExperiment: () => {}, getForcedValue: ({key, queryString}) => { if (!queryString) { @@ -27,6 +30,9 @@ const getBrowserStrategy = ({customTrackExperimentViewed, cache}) => ({ return variationName }, + decide: ({pde, name, attributes, adapterId}) => { + return pde.decide({pde, name, attributes, adapterId}) + }, trackExperiment: ({variationName, experimentName}) => { if (customTrackExperimentViewed) { return customTrackExperimentViewed({variationName, experimentName}) diff --git a/packages/sui-pde/src/hooks/useDecision.js b/packages/sui-pde/src/hooks/useDecision.js new file mode 100644 index 000000000..18d2cad62 --- /dev/null +++ b/packages/sui-pde/src/hooks/useDecision.js @@ -0,0 +1,58 @@ +import {useContext, useMemo} from 'react' + +import PdeContext from '../contexts/PdeContext.js' +import {getPlatformStrategy} from './common/platformStrategies.js' + +/** + * Hook to use a feature test + * @param {string} name + * @param {object} param + * @param {object} param.attributes + * @param {function} param.trackExperimentViewed + * @param {string} param.queryString + * @param {string} param.adapterId Adapter id to be executed + * @return {object} + */ +export default function useDecision(name, {attributes, trackExperimentViewed, queryString, adapterId} = {}) { + const {pde} = useContext(PdeContext) + + if (pde === null) { + throw new Error('[sui-pde: useDecision] sui-pde provider is required to work') + } + + const variation = useMemo(() => { + const strategy = getPlatformStrategy({ + customTrackExperimentViewed: trackExperimentViewed + }) + + const forced = strategy.getForcedValue({ + key: name, + queryString + }) + + const data = strategy.decide({ + pde, + name, + attributes, + adapterId + }) + + const {ruleKey, variationKey} = data || {} + + if (forced) { + if (!ruleKey) { + return {...data, enabled: forced === 'on'} + } + + return {...data, enabled: true, variationKey: forced} + } + + if (ruleKey) { + strategy.trackExperiment({variationName: variationKey, experimentName: name}) + } + + return data + }, [trackExperimentViewed, name, queryString, pde, attributes, adapterId]) + + return {variation} +} diff --git a/packages/sui-pde/src/index.js b/packages/sui-pde/src/index.js index 3401602ea..6fe1f39f5 100644 --- a/packages/sui-pde/src/index.js +++ b/packages/sui-pde/src/index.js @@ -2,5 +2,6 @@ export {default as PDE} from './pde.js' export {default as useFeature} from './hooks/useFeature.js' export {default as PdeContext} from './contexts/PdeContext.js' export {default as useExperiment} from './hooks/useExperiment.js' +export {default as useDecision} from './hooks/useDecision.js' export {default as Experiment} from './components/experiment.js' export {default as Feature} from './components/feature.js' diff --git a/packages/sui-pde/src/pde.js b/packages/sui-pde/src/pde.js index e0bf815af..9d4298d85 100644 --- a/packages/sui-pde/src/pde.js +++ b/packages/sui-pde/src/pde.js @@ -28,6 +28,15 @@ export default class PDE { return this._adapter.activateExperiment({name, attributes, adapterId}) } + /** + * @param {object} param + * @param {string} param.name + * @param {object} param.attributes + */ + decide({name, attributes, adapterId}) { + return this._adapter.decide({name, attributes, adapterId}) + } + getInitialData() { return this._adapter.getInitialData() } From ecbb326688b44aacea900f655454a6856cb7bb1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Wed, 2 Oct 2024 11:18:15 +0200 Subject: [PATCH 2/6] feat(packages/sui-pde): update usedecision hook --- packages/sui-pde/src/hooks/useDecision.js | 64 ++-- packages/sui-pde/test/common/index.js | 2 +- .../sui-pde/test/common/useDecisionSpec.js | 325 ++++++++++++++++++ 3 files changed, 361 insertions(+), 30 deletions(-) create mode 100644 packages/sui-pde/test/common/useDecisionSpec.js diff --git a/packages/sui-pde/src/hooks/useDecision.js b/packages/sui-pde/src/hooks/useDecision.js index 18d2cad62..ba4928bcd 100644 --- a/packages/sui-pde/src/hooks/useDecision.js +++ b/packages/sui-pde/src/hooks/useDecision.js @@ -20,39 +20,45 @@ export default function useDecision(name, {attributes, trackExperimentViewed, qu throw new Error('[sui-pde: useDecision] sui-pde provider is required to work') } - const variation = useMemo(() => { - const strategy = getPlatformStrategy({ - customTrackExperimentViewed: trackExperimentViewed - }) - - const forced = strategy.getForcedValue({ - key: name, - queryString - }) - - const data = strategy.decide({ - pde, - name, - attributes, - adapterId - }) - - const {ruleKey, variationKey} = data || {} - - if (forced) { - if (!ruleKey) { - return {...data, enabled: forced === 'on'} + const data = useMemo(() => { + try { + const strategy = getPlatformStrategy({ + customTrackExperimentViewed: trackExperimentViewed + }) + + const forced = strategy.getForcedValue({ + key: name, + queryString + }) + + const data = strategy.decide({ + pde, + name, + attributes, + adapterId + }) + + const {ruleKey, variationKey} = data || {} + + const isExperiment = !!ruleKey + + if (forced) { + if (!isExperiment) { + return {...data, enabled: forced === 'on'} + } + + return {...data, enabled: true, variationKey: forced} } - return {...data, enabled: true, variationKey: forced} - } + if (isExperiment) { + strategy.trackExperiment({variationName: variationKey, experimentName: ruleKey}) + } - if (ruleKey) { - strategy.trackExperiment({variationName: variationKey, experimentName: name}) + return data + } catch (error) { + return {enabled: false, flagKey: name} } - - return data }, [trackExperimentViewed, name, queryString, pde, attributes, adapterId]) - return {variation} + return data } diff --git a/packages/sui-pde/test/common/index.js b/packages/sui-pde/test/common/index.js index 8de254ec2..2c775ef02 100644 --- a/packages/sui-pde/test/common/index.js +++ b/packages/sui-pde/test/common/index.js @@ -1,3 +1,3 @@ import './pdeSpec.js' -import './useExperimentSpec' // This file has no extension due to sui-test server problem +import './useExperimentSpec.js' // This file has no extension due to sui-test server problem import './useFeatureSpec.js' diff --git a/packages/sui-pde/test/common/useDecisionSpec.js b/packages/sui-pde/test/common/useDecisionSpec.js new file mode 100644 index 000000000..30e5a8495 --- /dev/null +++ b/packages/sui-pde/test/common/useDecisionSpec.js @@ -0,0 +1,325 @@ +/* eslint-disable no-console */ +import {expect} from 'chai' +import sinon from 'sinon' + +import {cleanup, renderHook} from '@testing-library/react-hooks' +import {descriptorsByEnvironmentPatcher} from '@s-ui/test/lib/descriptor-environment-patcher.js' + +import PdeContext from '../../src/contexts/PdeContext.js' +import {SESSION_STORAGE_KEY as PDE_CACHE_STORAGE_KEY} from '../../src/hooks/common/trackedEventsLocalCache.js' +import useDecision from '../../src/hooks/useDecision.js' + +descriptorsByEnvironmentPatcher() + +describe('useDecision hook', () => { + afterEach(() => { + cleanup() + if (typeof window === 'undefined') return + window.sessionStorage.removeItem(PDE_CACHE_STORAGE_KEY) + }) + + describe('when no pde context is set', () => { + it('should throw an error', () => { + const {result} = renderHook(() => useDecision()) + expect(result.error).to.not.be.null + }) + }) + + describe('when pde context is set', () => { + let wrapper + let decide + + before(() => { + decide = sinon.stub().returns({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + // eslint-disable-next-line react/prop-types + wrapper = ({children}) => ( + {children} + ) + }) + + describe.client('and the hook is executed by the browser', () => { + describe('and window.analytics.track exists', () => { + beforeEach(() => { + window.analytics = { + ready: cb => cb(), + track: sinon.spy() + } + sinon.spy(console, 'error') + }) + + afterEach(() => { + delete window.analytics + console.error.restore() + }) + + it('should return the right variationName and launch the Experiment Viewed event', () => { + const {result} = renderHook(() => useDecision('flag'), { + wrapper + }) + expect(result.current).to.be.deep.equal({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + sinon.assert.callCount(console.error, 0) + sinon.assert.callCount(window.analytics.track, 1) + expect(window.analytics.track.args[0][0]).to.equal('Experiment Viewed') + expect(window.analytics.track.args[0][1]).to.deep.equal({ + variationName: 'variation', + experimentName: 'rule' + }) + }) + + describe('when the variation is forced by query param', () => { + it('should return the forced variation of an experiment', () => { + const {result} = renderHook( + () => + useDecision('experiment1', { + queryString: '?suipde_experiment1=variation_a' + }), + {wrapper} + ) + expect(result.current).to.be.deep.equal({ + variationKey: 'variation_a', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + }) + }) + + describe('when the same experiment is loaded more than once', () => { + it('should only track once', () => { + renderHook(() => useDecision('flag'), { + wrapper + }) + renderHook(() => useDecision('flag'), { + wrapper + }) + expect(window.analytics.track.args.length).to.equal(1) + }) + }) + }) + + describe('and window.analytics.track does not exist', () => { + beforeEach(() => { + sinon.spy(console, 'error') + }) + + afterEach(() => { + console.error.restore() + }) + + it('should return the right variationName and log an error', () => { + delete window.analytics + const {result} = renderHook(() => useDecision('flag'), { + wrapper + }) + expect(result.current).to.be.deep.equal({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + sinon.assert.callCount(console.error, 1) + expect(console.error.args[0][0]).to.include('window.analytics.track expected') + }) + }) + + describe('and use a custom track function', () => { + let customTrack + + before(() => { + customTrack = sinon.spy() + sinon.spy(console, 'error') + }) + + after(() => { + customTrack = undefined + console.error.restore() + }) + + it('should return the right variationName and execute custom track function', () => { + const {result} = renderHook( + () => + useDecision('test_experiment_id', { + trackExperimentViewed: customTrack + }), + { + wrapper + } + ) + expect(result.current).to.be.deep.equal({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + sinon.assert.callCount(console.error, 0) + sinon.assert.callCount(customTrack, 1) + }) + }) + }) + + describe.server('and the hook is executed by the server', () => { + before(() => { + sinon.spy(console, 'error') + }) + + after(() => { + console.error.restore() + }) + + it('should return the right variationName and launch the Experiment Viewed event', () => { + const {result} = renderHook(() => useDecision('flag'), { + wrapper + }) + expect(result.current.variation).to.be.deep.equal({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + sinon.assert.callCount(console.error, 0) + }) + }) + }) + + describe('when the activation returns an error', () => { + let decide + let wrapper + beforeEach(() => { + decide = sinon.stub().throws(new Error('fake activation error')) + // eslint-disable-next-line react/prop-types + wrapper = ({children}) => ( + {children} + ) + }) + + it('should return the default variation', () => { + const {result} = renderHook(() => useDecision('flag'), {wrapper}) + expect(result.current).to.be.deep.equal({enabled: false, flagKey: 'flag'}) + }) + }) + + describe.client('when calling twice the useDecision hook with the same feature key', () => { + let wrapper + let stubFactory + + beforeEach(() => { + window.analytics = { + ready: cb => cb(), + track: sinon.spy() + } + + stubFactory = decide => { + // eslint-disable-next-line react/prop-types + wrapper = ({children}) => ( + {children} + ) + } + }) + + afterEach(() => { + delete window.analytics + }) + + describe('when the second time returns the same value as the first time', () => { + beforeEach(() => { + const decide = sinon.stub() + + decide.onCall(0).returns({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + decide.onCall(1).returns({ + variationKey: 'variation', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + + stubFactory(decide) + }) + + it('should send only one experiment viewed event', () => { + renderHook(() => useDecision('flag'), { + wrapper + }) + renderHook(() => useDecision('flag'), { + wrapper + }) + expect(window.analytics.track.args.length).to.equal(1) + }) + }) + + describe('when the second time returns a different value as the first time', () => { + beforeEach(() => { + const decide = sinon.stub() + + decide.onCall(0).returns({ + variationKey: 'variation_a', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + decide.onCall(1).returns({ + variationKey: 'variation_b', + enabled: true, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + + stubFactory(decide) + }) + + it('should send two experiment viewed events', () => { + renderHook(() => useDecision('flag'), { + wrapper + }) + renderHook(() => useDecision('flag'), { + wrapper + }) + expect(window.analytics.track.args.length).to.equal(2) + }) + }) + }) +}) From 2b86724647690d7657eb93df8c8ac6974270edce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Wed, 2 Oct 2024 11:41:41 +0200 Subject: [PATCH 3/6] feat(packages/sui-pde): add decision component --- packages/sui-pde/src/components/decision.js | 24 +++++++++++++++++ packages/sui-pde/src/hooks/useDecision.js | 2 +- packages/sui-pde/src/index.js | 1 + .../sui-pde/test/common/useDecisionSpec.js | 27 ++++++++++++++++--- 4 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 packages/sui-pde/src/components/decision.js diff --git a/packages/sui-pde/src/components/decision.js b/packages/sui-pde/src/components/decision.js new file mode 100644 index 000000000..c2b698bc0 --- /dev/null +++ b/packages/sui-pde/src/components/decision.js @@ -0,0 +1,24 @@ +import PropTypes from 'prop-types' + +import useDecision from '../hooks/useDecision.js' + +export default function Decision({adapterId, name, attributes, trackExperimentViewed, queryString, children}) { + const data = useDecision(name, { + attributes, + trackExperimentViewed, + queryString, + adapterId + }) + + return children(data) +} + +Decision.propTypes = { + name: PropTypes.string.isRequired, + attributes: PropTypes.object, + trackExperimentViewed: PropTypes.func, + queryString: PropTypes.string, + children: PropTypes.func, + adapterId: PropTypes.string +} +Decision.displayName = 'Decision' diff --git a/packages/sui-pde/src/hooks/useDecision.js b/packages/sui-pde/src/hooks/useDecision.js index ba4928bcd..4ff426e3d 100644 --- a/packages/sui-pde/src/hooks/useDecision.js +++ b/packages/sui-pde/src/hooks/useDecision.js @@ -43,7 +43,7 @@ export default function useDecision(name, {attributes, trackExperimentViewed, qu const isExperiment = !!ruleKey if (forced) { - if (!isExperiment) { + if (!isExperiment || ['on', 'off'].includes(forced)) { return {...data, enabled: forced === 'on'} } diff --git a/packages/sui-pde/src/index.js b/packages/sui-pde/src/index.js index 6fe1f39f5..d23c8f35f 100644 --- a/packages/sui-pde/src/index.js +++ b/packages/sui-pde/src/index.js @@ -5,3 +5,4 @@ export {default as useExperiment} from './hooks/useExperiment.js' export {default as useDecision} from './hooks/useDecision.js' export {default as Experiment} from './components/experiment.js' export {default as Feature} from './components/feature.js' +export {default as Decision} from './components/decision.js' diff --git a/packages/sui-pde/test/common/useDecisionSpec.js b/packages/sui-pde/test/common/useDecisionSpec.js index 30e5a8495..c83a45729 100644 --- a/packages/sui-pde/test/common/useDecisionSpec.js +++ b/packages/sui-pde/test/common/useDecisionSpec.js @@ -82,12 +82,33 @@ describe('useDecision hook', () => { }) }) + describe('when the flag is forced by query param', () => { + it('should return the forced flag of a feature test', () => { + const {result} = renderHook( + () => + useDecision('flag', { + queryString: '?suipde_flag=off' + }), + {wrapper} + ) + expect(result.current).to.be.deep.equal({ + variationKey: 'variation', + enabled: false, + variables: {}, + ruleKey: 'rule', + flagKey: 'flag', + userContext: {}, + reasons: [] + }) + }) + }) + describe('when the variation is forced by query param', () => { - it('should return the forced variation of an experiment', () => { + it('should return the forced variation of a feature test', () => { const {result} = renderHook( () => - useDecision('experiment1', { - queryString: '?suipde_experiment1=variation_a' + useDecision('rule', { + queryString: '?suipde_rule=variation_a' }), {wrapper} ) From e00f881376cd45488b3edc28458a74ecb165275c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Wed, 2 Oct 2024 12:13:17 +0200 Subject: [PATCH 4/6] feat(packages/sui-pde): update doc --- packages/sui-pde/README.md | 254 ++++--------------------------------- 1 file changed, 28 insertions(+), 226 deletions(-) diff --git a/packages/sui-pde/README.md b/packages/sui-pde/README.md index 04ed7354b..8aced1416 100644 --- a/packages/sui-pde/README.md +++ b/packages/sui-pde/README.md @@ -62,261 +62,63 @@ const pde = new PDE({ When client-side rendering, sui-pde will load the datafile saved as `window.__INITIAL_CONTEXT_VALUE__.pde` as initial datafile. Therefore, you'll need to inject the output of the `pde.getInitialContextData()` function in your html when server side rendering. -### Experiments +### Feature Test -Given experiment `experimentX` with 2 variations `variationA` and `variationB` render `MyVariationA` or `MyVariationB` component depending on the variation the user has being assigned. Render `MyVariationA` by default +Feature tests are similar to A/B/n tests that allow you to control whether for each variation the associated feature is on or off via feature flags (aka feature toggles). It also allows you to control the feature variable values for the various variables associated with the feature. -⚠️ If the user did not consent to or if optimizely decides that the user will not be part of the experiment of something goes wrong, `useExperiment` will return as variation value `null` - -⚠️ The `useExperiment` hook will call a global window.analytics.track method with `Experiment Viewed` as event name with the experiment properties so you are able to replicate the experiment in your analytics tool +The `useDecision` hook retrieves the result for a given decision, based on logic from the decision-making tool you are using (e.g., Optimizely). ```js -import {useExperiment} from '@s-ui/pde' - -const EXPERIMENT_NAME = 'experimentX' +import {useDecision} from '@s-ui/pde' -const MyComponent = () => { - const {variation} = useExperiment({experimentName: EXPERIMENT_NAME}) +function Component() { + const decision = useDecision('feature_test', {}) - if (variation === 'variationB') return - return + return ( + <> + {decision.enabled &&

My feature is enabled

} + {!decision.enabled &&

My feature is disabled

} + {decision.variationKey === 'variantion_a' &&

Current Variation

} + {decision.variationKey === 'variantion_b' &&

Better Variation

} + + ); } ``` -You can also use `Experiment` component which takes the same props as the hook +You can also use the `Decision` component: ```js -import {Experiment} from '@s-ui/pde' - -const EXPERIMENT_NAME = 'experimentX' +import {Decision} from '@s-ui/pde' const MyComponent = () => { return ( - - {({variation}) => variation === 'variationB' ? : } - + + {({enabled}) => enabled ?

My feature is enabled

:

My feature is disabled

} + ) } ``` -**Special cases for useExperiment `Experiment Viewed` track** - -Given useExperiment sends `Experiment Viewed` on being executed, some facts could happen: - -- Root: Analytics SDK is loaded async and loads after useExperiment hook has been called -- Cause: `Experiment Viewed` won't be sent. - -- Root: `Experiment Viewed` should has a different name or properties. -- Cause: Send a track with wrong values. - -In order to have a higher controll about that, useExperiment accepts a `trackExperimentViewed` callback to customize it - -```js -import {useExperiment} from '@s-ui/pde' - -const EXPERIMENT_NAME = 'experimentX' - -const trackExperiment = ({experimentName, variationName}) => { - window.analytics.track('Experiment Viewed', { - experimentName, - variationName, - customProperty: 'yay' - }) -} - -const MyComponent = () => { - const {variation} = useExperiment({ - experimentName: EXPERIMENT_NAME, - trackExperimentViewed: trackExperiment - }) - - if (variation === 'variationB') return - return -} -``` - #### Attributes -In order to pass by attributes, you'll able to do so by adding the named parameter `attributes` when using the useExperiment hook. Something like this: +You can pass additional attributes to refine your decision logic: ```js -import {useExperiment} from '@s-ui/pde' - -const EXPERIMENT_NAME = 'experimentX' +import {useDecision} from '@s-ui/pde' const MyComponent = () => { - const {variation} = useExperiment({ - experimentName: EXPERIMENT_NAME, - attributes: { // this will send these attributes + const {variation} = useDecision('feature_test', { + attributes: { isLoggedIn: true } }) - - if (variation === 'variationB') return - return -} -``` - -⚠️ Remember that common attributes (those attributes that every experiment should send by) are set with the `applicationAttributes` when creating the optimizely adapter. Check out the [react context section](#React-context) - -#### Force experiment variation - -It's possible to force a variation for our experiment in the browser. For example, lets assume we want to QA a specific variation for our test called `abtest2_recommender` and the test is running in `http://myweb.com`. In order to force a variation you'll have to add a query param using the experiment name but adding `suipde_` as prefix, for example, for our recommender test, the url to open in order to force a variation would be `http://myweb.com?suipde_abtest2_recommender=default`. This would force the default variation. If forced, optimizely impression will not be triggered. - -### Feature Flags and Feature Tests - -⚠️ user consent do apply to feature flags only when used as feature test -⚠️ The `useFeature` hook will call a global window.analytics.track method with `Experiment Viewed` as event name with the experiment properties so you are able to replicate the experiment in your analytics tool. For each linked experiment (feature tests), an extra `Experiment Viewed` event will be send. - -```js -import {useFeature} from '@s-ui/pde' - -const MyComponent = () => { - const {isActive} = useFeature('myFeatureKey') // isActive = true when the feature flag is activated - - return

The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}

-} -``` - -You can also use `Feature` component which takes the following optional props - -- `featureName` -- `attributes` -- `queryString` - -```js -import {Feature} from '@s-ui/pde' - -const MyComponent = () => { - return ( - - {({isActive}) => ( -

The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}

- )} - - ) -} -``` - -#### Feature Flags Variables - -Returns all feature variables for the specified feature flag - -```js -import {useFeature} from '@s-ui/pde' - -const MyComponent = () => { - const {isActive, variables} = useFeature('myFeatureKey') // variables = an object with all the feature variables - - return ( -

- The feature 'myFeatureKey' is{' '} - {isActive ? `active and price value is ${variables.price}` : 'inactive'} -

- ) -} -``` - -#### Segment integration - -By default, segment integration will be active, this means that a global `window.optimizelyClientInstance` reference to the `optimizelyIntance` object passed by to the PDE constructor will be created. In case you want to turn this option off, create the optimizely adapter as follows: - -```js -const optimizelyAdapter = new OptimizelyAdapter({ - optimizely: optimizelyInstance, - userId, - activeIntegrations: {segment: false} -}) -``` - -#### Track Experiment Viewed - -In order to reduce unnecessary calls to Segment, the `Experiment Viewed` event is disabled by default. - -If you need to track how many times your experiment has been viewed, you should set the `shouldTrackExperimentViewed` argument to true. - -```js -const {isActive, variables} = useFeature('myFeatureKey', undefined, undefined, undefined, true) -``` - -A refactoring task is pending to transition the hook's positional parameters to named parameters. - -#### Attributes - -In order to pass by attributes, you'll able to do so by adding the second argument as `attributes` when using the useFeature hook. Something like this: - -```js -import {useFeature} from '@s-ui/pde' - -const MyComponent = () => { - const {isActive} = useFeature('myFeatureKey', { - isLoggedIn: true // this second parameter are the attributes - }) - - return

The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}

-} -``` - -⚠️ Remember that common attributes (those attributes that every experiment should send by) are set with the `applicationAttributes` when creating the optimizely adapter. Check out the [react context section](#React-context) - -#### Force feature flag to be on/off - -It's slighty different to force a feature flag to be activated or deactivated. Lets assume we have our feature flag `ff_skills_field` running under `http://myweb.com`. In order to force the flag to be on or off you'll have to add a query param using the flag's name but adding `suipde_` as prefix same way we force an experiment, but the only valid values are on or off. For example, in this case, the url to open in order to force would be `http://myweb.com?suipde_ff_skills_field=on`. This would force the feature flag to be on. `http://myweb.com?suipde_ff_skills_field=off` would set the feature flag as off. If forced, optimizely impression will not be triggered. - -### Multiple Optimizely Adapters - -Meant to exist if you need more than one decision taking optimizely sdk. - -When initializing PDE use `MultipleOptimizelyAdapter` instead of `OptimizelyAdapter` -```js - import MultipleOptimizelyAdapter from '@s-ui/pde/lib/adapters/optimizely/multiple' -... - const optimizelyInstances = MultipleOptimizelyAdapter.createMultipleOptimizelyInstances({ - default: { - sdkKey: DEFAULT_INSTANCE_SDK_KEY, - options: {} // options for default instance - }, - alternate: { - sdkKey: ALTERNATIVE_INSTANCE_SDK_KEY, - options: {} // options for alternative instance - } - }) - - // first id will be used as default adapterId, in this case 'default' but is open to any id - const optimizelyAdapter = new MultipleOptimizelyAdapter({ - default: { - optimizely: optimizelyInstances.default, - ...adapterOptions // like creating single adapter - }, - alternate: { - optimizely: optimizelyInstances.alternative, - ...adapterOptions // like creating single adapter - } - }) - - const pde = new PDE({ - adapter: optimizelyAdapter, - ... - }) -``` - -Using the hooks - -```js -const MyComponent = () => { - const defaultFeature = useFeature('myFeatureKey') // will return the {isActive, variables} object from the default optimizely instance - const alsoDefaultFeature = useFeature('myFeatureKey', null, null, 'default') // will return the {isActive, variables} object from the default optimizely instance - const alternateFeature = useFeature('myFeatureKey', null, null, 'alternative') // will return the {isActive, variables} object from the alternate optimizely instance - - const defaultExperiment = useExperiment({experimentName: 'myExperimentName'}) // will return the experiment object from the default optimizely instance - const alsoDefaultExperiment = useExperiment({experimentName: 'myExperimentName', adapterId: 'default'}) // will return the experiment object from the default optimizely instance - const alternateExperiment = useExperiment({experimentName: 'myExperimentName', adapterId: 'alternate'}) // will return the experiment object from the alternate optimizely instance - ... } ``` -#### :warning: Using segment integration +#### Forcing a decision -Regarding to [Segment documentation](https://segment.com/docs/connections/destinations/catalog/optimizely-web/#optimizely-full-stack-javascript-sdk) +You can force specific decision outcomes during testing by adding a query parameter. -Segment expects a single `window.optimizelyClientInstance` to exist in the browser, so when using multiple optimizely instances, events from multiple instances will be sent to a single Segment source, so the Segment destinations should be properly configured having this in consideration. +- `http://www.fotocasa.es/es?suipde_example=on` will enable the `example` feature test +- `http://www.fotocasa.es/es?suipde_example=off` will disable the `example` feature test +- `http://www.fotocasa.es/es?suipde_example=variation_a` will enable the `example` feature test and will force the variation `variation_a` \ No newline at end of file From ac678bda1274e9f2463f28b40f86016e069dc00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Wed, 2 Oct 2024 12:25:57 +0200 Subject: [PATCH 5/6] feat(packages/sui-pde): update use decision hook --- packages/sui-pde/src/hooks/useDecision.js | 16 ++++++++-------- packages/sui-pde/test/common/useDecisionSpec.js | 13 ++----------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/packages/sui-pde/src/hooks/useDecision.js b/packages/sui-pde/src/hooks/useDecision.js index 4ff426e3d..41bce419f 100644 --- a/packages/sui-pde/src/hooks/useDecision.js +++ b/packages/sui-pde/src/hooks/useDecision.js @@ -31,6 +31,14 @@ export default function useDecision(name, {attributes, trackExperimentViewed, qu queryString }) + if (forced) { + if (['on', 'off'].includes(forced)) { + return {enabled: forced === 'on', flagKey: name} + } + + return {enabled: true, flagKey: name, variationKey: forced} + } + const data = strategy.decide({ pde, name, @@ -42,14 +50,6 @@ export default function useDecision(name, {attributes, trackExperimentViewed, qu const isExperiment = !!ruleKey - if (forced) { - if (!isExperiment || ['on', 'off'].includes(forced)) { - return {...data, enabled: forced === 'on'} - } - - return {...data, enabled: true, variationKey: forced} - } - if (isExperiment) { strategy.trackExperiment({variationName: variationKey, experimentName: ruleKey}) } diff --git a/packages/sui-pde/test/common/useDecisionSpec.js b/packages/sui-pde/test/common/useDecisionSpec.js index c83a45729..7ab093891 100644 --- a/packages/sui-pde/test/common/useDecisionSpec.js +++ b/packages/sui-pde/test/common/useDecisionSpec.js @@ -92,13 +92,8 @@ describe('useDecision hook', () => { {wrapper} ) expect(result.current).to.be.deep.equal({ - variationKey: 'variation', enabled: false, - variables: {}, - ruleKey: 'rule', - flagKey: 'flag', - userContext: {}, - reasons: [] + flagKey: 'flag' }) }) }) @@ -115,11 +110,7 @@ describe('useDecision hook', () => { expect(result.current).to.be.deep.equal({ variationKey: 'variation_a', enabled: true, - variables: {}, - ruleKey: 'rule', - flagKey: 'flag', - userContext: {}, - reasons: [] + flagKey: 'rule' }) }) }) From d8b892fe8b22459df10229360db261df29ff0bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Alvarez?= Date: Thu, 3 Oct 2024 09:12:38 +0200 Subject: [PATCH 6/6] test(packages/sui-pde): update tests --- packages/sui-pde/test/common/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/sui-pde/test/common/index.js b/packages/sui-pde/test/common/index.js index 2c775ef02..4839d40d0 100644 --- a/packages/sui-pde/test/common/index.js +++ b/packages/sui-pde/test/common/index.js @@ -1,3 +1,4 @@ import './pdeSpec.js' -import './useExperimentSpec.js' // This file has no extension due to sui-test server problem +import './useExperimentSpec.js' import './useFeatureSpec.js' +import './useDecisionSpec.js'