-
Notifications
You must be signed in to change notification settings - Fork 37
feat(react): add FeatureFlag component #1164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
import React from 'react'; | ||
import { useFlag } from '../evaluation'; | ||
import type { FlagQuery } from '../query'; | ||
|
||
/** | ||
* Props for the Feature component that conditionally renders content based on feature flag state. | ||
* @interface FeatureProps | ||
*/ | ||
interface FeatureProps { | ||
/** | ||
* The key of the feature flag to evaluate. | ||
*/ | ||
featureKey: string; | ||
|
||
/** | ||
* Optional value to match against the feature flag value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please specify here exactly what comparison is used\ ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we do the default mentioned below, we can just describe it as |
||
* If provided, the component will only render children when the flag value matches this value. | ||
* If a boolean, it will check if the flag is enabled (true) or disabled (false). | ||
* If a string, it will check if the flag variant equals this string. | ||
*/ | ||
match?: string | boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be really cool to add support for a predicate function that accepts the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
+1 Also with this, you could make this function work for all the types. function equals<T>(expected: T, actual: EvaluationDetails<T>){
return expected === actual.value
} And for objects we could actually make this required if we wanted as this default does not make much sense here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually ya, I think that's a great idea. |
||
|
||
/** | ||
* Default value to use when the feature flag is not found. | ||
*/ | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
defaultValue: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the predicate from above this could be generic. |
||
|
||
/** | ||
* Content to render when the feature flag condition is met. | ||
* Can be a React node or a function that receives flag query details and returns a React node. | ||
*/ | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
children: React.ReactNode | ((details: FlagQuery<any>) => React.ReactNode); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
/** | ||
* Optional content to render when the feature flag condition is not met. | ||
*/ | ||
fallback?: React.ReactNode; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we are going to only have one alternative child (fallback) we definitely need to do this, so that users can have control over to render this in very specific situations (evaluation errors, reasons, etc). The alternative (which I don't like as muich) would be to also specify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think for it feels a little more idiomatic to have the matcher that is also able to match for error, as you said @toddbaert. |
||
|
||
/** | ||
* If true, inverts the condition logic (renders children when condition is NOT met). | ||
*/ | ||
negate?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this is needed if we do this: https://github.com/open-feature/js-sdk/pull/1164/files#r2036805701 I would recommend getting rid of this, then users can white their own invert. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
} | ||
|
||
/** | ||
* FeatureFlag component that conditionally renders its children based on the evaluation of a feature flag. | ||
Check warning on line 48 in packages/react/src/declarative/FeatureFlag.tsx
|
||
* | ||
* @param {FeatureProps} props The properties for the FeatureFlag component. | ||
* @returns {React.ReactElement | null} The rendered component or null if the feature is not enabled. | ||
*/ | ||
export function FeatureFlag({ | ||
featureKey, | ||
match, | ||
negate = false, | ||
defaultValue = true, | ||
children, | ||
fallback = null, | ||
}: FeatureProps): React.ReactElement | null { | ||
const details = useFlag(featureKey, defaultValue, { | ||
updateOnContextChanged: true, | ||
}); | ||
|
||
// If the flag evaluation failed, we render the fallback | ||
if (details.reason === 'ERROR') { | ||
return <>{fallback}</>; | ||
} | ||
|
||
let isMatch = false; | ||
if (typeof match === 'string') { | ||
isMatch = details.variant === match; | ||
} else if (typeof match !== 'undefined') { | ||
isMatch = details.value === match; | ||
} | ||
|
||
// If match is undefined, we assume the flag is enabled | ||
if (match === void 0) { | ||
isMatch = true; | ||
} | ||
|
||
const shouldRender = negate ? !isMatch : isMatch; | ||
|
||
if (shouldRender) { | ||
console.log('chop chop'); | ||
const childNode: React.ReactNode = typeof children === 'function' ? children(details) : children; | ||
return <>{childNode}</>; | ||
} | ||
|
||
return <>{fallback}</>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './FeatureFlag'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import React from 'react'; | ||
import '@testing-library/jest-dom'; // see: https://testing-library.com/docs/react-testing-library/setup | ||
import { render, screen } from '@testing-library/react'; | ||
import { FeatureFlag } from '../src/declarative/FeatureFlag'; // Assuming Feature.tsx is in the same directory or adjust path | ||
import { InMemoryProvider, OpenFeature, OpenFeatureProvider } from '../src'; | ||
|
||
describe('Feature Component', () => { | ||
const EVALUATION = 'evaluation'; | ||
const MISSING_FLAG_KEY = 'missing-flag'; | ||
const BOOL_FLAG_KEY = 'boolean-flag'; | ||
const BOOL_FLAG_NEGATE_KEY = 'boolean-flag-negate'; | ||
const BOOL_FLAG_VARIANT = 'on'; | ||
const BOOL_FLAG_VALUE = true; | ||
const STRING_FLAG_KEY = 'string-flag'; | ||
const STRING_FLAG_VARIANT = 'greeting'; | ||
const STRING_FLAG_VALUE = 'hi'; | ||
|
||
const FLAG_CONFIG: ConstructorParameters<typeof InMemoryProvider>[0] = { | ||
[BOOL_FLAG_KEY]: { | ||
disabled: false, | ||
variants: { | ||
[BOOL_FLAG_VARIANT]: BOOL_FLAG_VALUE, | ||
off: false, | ||
}, | ||
defaultVariant: BOOL_FLAG_VARIANT, | ||
}, | ||
[BOOL_FLAG_NEGATE_KEY]: { | ||
disabled: false, | ||
variants: { | ||
[BOOL_FLAG_VARIANT]: BOOL_FLAG_VALUE, | ||
off: false, | ||
}, | ||
defaultVariant: 'off', | ||
}, | ||
[STRING_FLAG_KEY]: { | ||
disabled: false, | ||
variants: { | ||
[STRING_FLAG_VARIANT]: STRING_FLAG_VALUE, | ||
parting: 'bye', | ||
}, | ||
defaultVariant: STRING_FLAG_VARIANT, | ||
} | ||
}; | ||
|
||
const makeProvider = () => { | ||
return new InMemoryProvider(FLAG_CONFIG); | ||
}; | ||
|
||
OpenFeature.setProvider(EVALUATION, makeProvider()); | ||
|
||
const childText = 'Feature is active'; | ||
const ChildComponent = () => <div>{childText}</div>; | ||
|
||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
describe('<FeatureFlag />', () => { | ||
it('should not show the feature component if the flag is not enabled', () => { | ||
render( | ||
<OpenFeatureProvider domain={EVALUATION}> | ||
<FeatureFlag featureKey={BOOL_FLAG_KEY} defaultValue={false}> | ||
<ChildComponent /> | ||
</FeatureFlag> | ||
</OpenFeatureProvider>, | ||
); | ||
|
||
expect(screen.queryByText(childText)).toBeInTheDocument(); | ||
}); | ||
|
||
it('should fallback when provided', () => { | ||
render( | ||
<OpenFeatureProvider domain={EVALUATION}> | ||
<FeatureFlag featureKey={MISSING_FLAG_KEY} defaultValue={false} fallback={<div>Fallback</div>}> | ||
<ChildComponent /> | ||
</FeatureFlag> | ||
</OpenFeatureProvider>, | ||
); | ||
|
||
expect(screen.queryByText('Fallback')).toBeInTheDocument(); | ||
|
||
screen.debug(); | ||
}); | ||
|
||
it('should handle showing multivariate flags with bool match', () => { | ||
render( | ||
<OpenFeatureProvider domain={EVALUATION}> | ||
<FeatureFlag featureKey={STRING_FLAG_KEY} match={'greeting'} defaultValue={'default'}> | ||
<ChildComponent /> | ||
</FeatureFlag> | ||
</OpenFeatureProvider>, | ||
); | ||
|
||
expect(screen.queryByText(childText)).toBeInTheDocument(); | ||
}); | ||
|
||
it('should show the feature component if the flag is not enabled but negate is true', () => { | ||
render( | ||
<OpenFeatureProvider domain={EVALUATION}> | ||
<FeatureFlag featureKey={BOOL_FLAG_KEY} defaultValue={false}> | ||
<ChildComponent /> | ||
</FeatureFlag> | ||
</OpenFeatureProvider>, | ||
); | ||
|
||
expect(screen.queryByText(childText)).toBeInTheDocument(); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be called
key
... or possiblyflagKey
, but I think that's redundant given the name of the component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key
is reserved in React: https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-keyso I would vote for
flagKey