sui

sui copied to clipboard

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

Description

• Update useFeature hook to accept structured args instead of raw args
• Update unit tests to account for the changes to the useFeature hook
• Update README.md to account for changes to the useFeature hook
• Correct a spelling mistake in CONTRIBUTING.md (versionning -> versioning)

Related Issue

Fix #1418

Example

Example - 1

const MyComponent = () => {
  const {isActive} = useFeature({featureKey: 'myFeatureKey'}) // isActive = true when the feature flag is activated

  return <p>The feature 'myFeatureKey' is {isActive ? 'active' : 'inactive'}</p>
}

Example - 2

 useFeature({
              featureKey: 'featureKey3',
              queryString: '?suipde_featureKey3=off&suipde_feature1=on'
            })

@rmoralp Can you please review this PR? I've fixed this issue since I saw it tagged for hacktoberfest. Please feel free to let me know in case you guys are not looking for external contributors :)

I have approved workflow run from public fork 👍🏽

Looks like something was wrong during the build. Let's see if we can take a closer look of the issue in the next days.

Thanks @arnau-rius, the following mocha test is failing:

1) copyfiles
       should copy files from src to dest and flatten it:
     Error: ENOENT: no such file or directory, copyfile 'fixturesCopy/a.js' -> 'runtime/a.js'

It is trying to copy the a.js from fixturesCopy directory to the runtime directory. This runtime directory seems to be getting generated during the build. It could be a random failure from looking at the logs and since I don't really have any changes here. Could you please re-trigger the build?

Could you make a commit type fix or feat in order to mark it as a breaking change? It will trigger a major version of this package

BTW code looks great.

Could you make a commit type fix or feat in order to mark it as a breaking change? It will trigger a major version of this package

Alright, sure :)

BTW code looks great.

Thanks @rmoralp!

@rmoralp I have updated the commit type to fix. Could you please re-trigger the workflow build?

As I see, that commit names does not satisfy our conventional commit convention. You should use npm run co to make the commits

@rmoralp, I see. I'll keep that in mind for any future PRs. Thanks. Please feel free to let me know in case you want me to make any further changes to the current PR.

@rmoralp this is a major. The version number must be (manual) updated also b4 merging if no commit message has it declared

@rmoralp @andresin87 I've rebased my branch and fixed up the commits via the npm run co. The major version has been updated now. Please re-trigger the build workflow and feel free to let me know in case there's anything else you need me to do. Thanks.

Hi @theonly1me @andresin87 which is the current status of this PR? In a short period of time we'll had to add a new feature adding a new parameter to the useFeature ( #1601 ) and would be nice to have the structured args already