Fix for Issue 1418: [@s-ui/pde] use structured args in useFeature

CONTRIBUTING.md

@@ -36,9 +36,9 @@ Packages must be properly named. 3 name are
 * Folder: `./packages/sui-my-example-package`
 * NPM name: `@s-ui/my-example-package`
 
-### Versionning
+### Versioning
 
-We follow the [comver](https://github.com/staltz/comver) versionning this system (X.y.0).
+We follow the [comver](https://github.com/staltz/comver) versioning this system (X.y.0).
 
 Packages first version must be `1.0.0` (not 0.0.0)
 

packages/sui-pde/README.md

@@ -172,7 +172,7 @@ It's possible to force a variation for our experiment in the browser. For exampl
 import {useFeature} from '@s-ui/pde'
 
 const MyComponent = () => {
-  const {isActive} = useFeature('myFeatureKey') // isActive = true when the feature flag is activated
+  const {isActive} = useFeature({featureKey: 'myFeatureKey'}) // isActive = true when the feature flag is activated
 
   return <p>The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}</p>
 }
@@ -206,7 +206,7 @@ Returns all feature variables for the specified feature flag
 import {useFeature} from '@s-ui/pde'
 
 const MyComponent = () => {
-  const {isActive, variables} = useFeature('myFeatureKey') // variables = an object with all the feature variables
+  const {isActive, variables} = useFeature({featureKey: 'myFeatureKey'}) // variables = an object with all the feature variables
 
   return (
     <p>
@@ -237,8 +237,11 @@ In order to pass by attributes, you'll able to do so by adding the second argume
 import {useFeature} from '@s-ui/pde'
 
 const MyComponent = () => {
-  const {isActive} = useFeature('myFeatureKey', {
-    isLoggedIn: true // this second parameter are the attributes
+  const {isActive} = useFeature({
+    featureKey: 'myFeatureKey', 
+    attributes: {
+      isLoggedIn: true // this second parameter are the attributes
+    }
   })
 
   return <p>The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}</p>
@@ -292,9 +295,9 @@ 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 defaultFeature = useFeature({featureKey: 'myFeatureKey'}) // will return the {isActive, variables} object from the default optimizely instance
+  const alsoDefaultFeature = useFeature({featureKey: 'myFeatureKey', adapterId: 'default'}) // will return the {isActive, variables} object from the default optimizely instance
+  const alternateFeature = useFeature({featureKey: 'myFeatureKey', adapterId: '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

packages/sui-pde/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@s-ui/pde",
-  "version": "2.20.0",
+  "version": "3.0.0",
   "description": "",
   "type": "module",
   "main": "lib/index.js",
@@ -29,5 +29,14 @@
     "@testing-library/react-hooks": "4.0.1",
     "react": "17",
     "react-test-renderer": "17"
+  },
+  "eslintConfig": {
+    "extends": [
+      "./node_modules/@s-ui/lint/eslintrc.js"
+    ]
+  },
+  "prettier": "./node_modules/@s-ui/lint/.prettierrc.js",
+  "stylelint": {
+    "extends": "./node_modules/@s-ui/lint/stylelint.config.js"
   }
-}
+}

packages/sui-pde/src/components/feature.js

@@ -8,7 +8,11 @@ export default function Feature({
   attributes,
   queryString
 }) {
-  const {isActive, variables} = useFeature(featureKey, attributes, queryString)
+  const {isActive, variables} = useFeature({
+    featureKey,
+    attributes,
+    queryString
+  })
   return children({isActive, variables})
 }
 

packages/sui-pde/src/hooks/useFeature.js

@@ -51,18 +51,19 @@ const trackLinkedExperimentsViewed = ({
 
 /**
  * Hook to use a feature toggle
- * @param {string} featureKey
- * @param {object} attributes
- * @param {string} [queryString] test purposes only
- * @param {string} adapterId
+ * @param {object} param
+ * @param {string} param.featureKey
+ * @param {object} param.attributes
+ * @param {string} param.[queryString] test purposes only
+ * @param {string} param.adapterId
  * @return {{isActive: boolean}}
  */
-export default function useFeature(
+export default function useFeature({
   featureKey,
   attributes,
   queryString,
   adapterId
-) {
+} = {}) {
   const {pde} = useContext(PdeContext)
   if (pde === null)
     throw new Error('[useFeature] sui-pde provider is required to work')

packages/sui-pde/test/common/useFeatureSpec.js

@@ -39,7 +39,11 @@ describe('when pde context is set', () => {
 
     it('should check if a feature is enabled', () => {
       const {result} = renderHook(
-        () => useFeature('featureKey1', {attribute1: 'value'}),
+        () =>
+          useFeature({
+            featureKey: 'featureKey1',
+            attributes: {attribute1: 'value'}
+          }),
         {wrapper}
       )
       expect(result.current.isActive).to.equal(true)
@@ -53,7 +57,11 @@ describe('when pde context is set', () => {
 
     it('should return feature variables', () => {
       const {result} = renderHook(
-        () => useFeature('featureKey2', {attribute1: 'value'}),
+        () =>
+          useFeature({
+            featureKey: 'featureKey2',
+            attributes: {attribute1: 'value'}
+          }),
         {wrapper}
       )
 
@@ -69,7 +77,11 @@ describe('when pde context is set', () => {
     describe.client('and the hook is executed by the browser', () => {
       it('should check that a feature has been forced to be active', () => {
         const {result} = renderHook(
-          () => useFeature('featureKey3', {}, '?suipde_feature1=on'),
+          () =>
+            useFeature({
+              featureKey: 'featureKey3',
+              queryString: '?suipde_feature1=on'
+            }),
           {wrapper}
         )
         expect(result.current.isActive).to.equal(true)
@@ -78,11 +90,10 @@ describe('when pde context is set', () => {
       it('should check that a feature has been forced to be not active', () => {
         const {result} = renderHook(
           () =>
-            useFeature(
-              'featureKey3',
-              {},
-              '?suipde_featureKey3=off&suipde_feature1=on'
-            ),
+            useFeature({
+              featureKey: 'featureKey3',
+              queryString: '?suipde_featureKey3=off&suipde_feature1=on'
+            }),
           {wrapper}
         )
         expect(result.current.isActive).to.equal(false)
@@ -128,7 +139,7 @@ describe('when pde context is set', () => {
         })
 
         it('should send the on state experiment viewed', () => {
-          renderHook(() => useFeature('activeFeatureFlagKey'), {
+          renderHook(() => useFeature({featureKey: 'activeFeatureFlagKey'}), {
             wrapper
           })
           expect(window.analytics.track.args[0][0]).to.equal(
@@ -142,12 +153,18 @@ describe('when pde context is set', () => {
 
         describe.client('when a feature is seen twice', () => {
           it('should only track one experiment viewed event', () => {
-            renderHook(() => useFeature('repeatedFeatureFlagKey'), {
-              wrapper
-            })
-            renderHook(() => useFeature('repeatedFeatureFlagKey'), {
-              wrapper
-            })
+            renderHook(
+              () => useFeature({featureKey: 'repeatedFeatureFlagKey'}),
+              {
+                wrapper
+              }
+            )
+            renderHook(
+              () => useFeature({featureKey: 'repeatedFeatureFlagKey'}),
+              {
+                wrapper
+              }
+            )
             expect(window.analytics.track.args.length).to.equal(1)
           })
         })
@@ -184,9 +201,12 @@ describe('when pde context is set', () => {
         })
 
         it('should send the off state experiment viewed', () => {
-          renderHook(() => useFeature('notActiveFeatureFlagKey'), {
-            wrapper
-          })
+          renderHook(
+            () => useFeature({featureKey: 'notActiveFeatureFlagKey'}),
+            {
+              wrapper
+            }
+          )
           expect(window.analytics.track.args[0][0]).to.equal(
             'Experiment Viewed'
           )
@@ -238,9 +258,16 @@ describe('when pde context is set', () => {
     })
 
     it('should send experiment viewed event for every test asociated and the experiment viewed associated to the feature flag itself', () => {
-      renderHook(() => useFeature('featureKey4', {attribute1: 'value'}), {
-        wrapper
-      })
+      renderHook(
+        () =>
+          useFeature({
+            featureKey: 'featureKey4',
+            attributes: {attribute1: 'value'}
+          }),
+        {
+          wrapper
+        }
+      )
 
       // feature being called
       expect(window.analytics.track.args[0][0]).to.equal('Experiment Viewed')
@@ -315,10 +342,10 @@ describe('when pde context is set', () => {
           stubFactory(isFeatureEnabled)
         })
         it('should send only one experiment viewed event', () => {
-          renderHook(() => useFeature('repeatedFeatureFlagKey'), {
+          renderHook(() => useFeature({featureKey: 'repeatedFeatureFlagKey'}), {
             wrapper
           })
-          renderHook(() => useFeature('repeatedFeatureFlagKey'), {
+          renderHook(() => useFeature({featureKey: 'repeatedFeatureFlagKey'}), {
             wrapper
           })
           expect(window.analytics.track.args.length).to.equal(1)
@@ -338,10 +365,10 @@ describe('when pde context is set', () => {
           stubFactory(isFeatureEnabled)
         })
         it('should send two experiment viewed events', () => {
-          renderHook(() => useFeature('repeatedFeatureFlagKey'), {
+          renderHook(() => useFeature({featureKey: 'repeatedFeatureFlagKey'}), {
             wrapper
           })
-          renderHook(() => useFeature('repeatedFeatureFlagKey'), {
+          renderHook(() => useFeature({featureKey: 'repeatedFeatureFlagKey'}), {
             wrapper
           })
           expect(window.analytics.track.args.length).to.equal(2)