-
Notifications
You must be signed in to change notification settings - Fork 33
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
[FEATURE] React SDK: Simplify bootstrapping of the OpenFeatureProvider #1047
Comments
The default provider is always the same since we're using a global singleton. For that reason, I think it might be misleading to explicitly register an OpenFeature provider in the React provider configuration. However, we could potentially create the behavior you're proposing by leveraging a domain behind the scenes. This is just spitballing, but we might be able to do something like this: function App() {
return (
// Automagically domain scopes this provider.
<OpenFeatureProvider provider={provider}>
<Page></Page>
</OpenFeatureProvider>
);
} The main advantage would be that all providers are scoped appropriately. However, it may introduce unnecessary complexity for both us, the SDK maintainer and the end users. What do you think @toddbaert @wichopy @lukas-reining @thomaspoignant . |
Yes for the default provider this would lead to strange behavior as @beeme1mr says.
This could be an idea. This feels like a OpenFeature internal use of domains. |
It would be nice to have something in the React Provider that reminds users to configure a provider. Perhaps we could define a pattern that doesn't imply any provider scoping. |
On the other hand one could argue that what @wichopy proposed fits perfectly to what the context mutator does. |
My reasoning is since we are trying to Reactify things, developers shouldn't need to interact with the OpenFeature singleton directly. A React developer should come in and use the openfeature lib like they would any other lib, which is through hooks and components. I'm still familiarizing myself with how the web sdk works, but how is having the react provider set the openfeature provider an issue? I imagined just moving the So something like this: export function OpenFeatureProvider({ client, domain, provider, children, ...options }: ProviderProps) {
// some assertions that provider is not already set and that the provider is valid
// In the readme we state that the developer needs to be careful not to set the provider more than once, why cant we do that for them?
if (validateArgs(provider) {
// throw errors
}
// if (!client) {
// client = OpenFeature.getClient(domain);
// }
OpenFeature.setProvider(domain, provider);
const client = OpenFeature.getClient(domain);
return <Context.Provider value={{ client, options, domain }}>{children}</Context.Provider>;
} |
Hey @wichopy, I agree that it isn't a technical problem. I'm more concerned about implying scoping behavior that doesn't exist. It could also lead to odd configurations like: <OpenFeatureProvider provider={Provider1} >
<Header />
<OpenFeatureProvider provider={Provider2} >
<AnotherComponent />
</OpenFeatureProvider>
</OpenFeatureProvider> This would lead to unexpected behavior. However, I can see the value of something like this, and I agree that we should make this React-friendly. @lukas-reining @toddbaert, what do you think? |
Requirements
This issue has similarities to #968
I am used to seeing the initialization of React providers being very minimal. Some examples are react router and react-query. They require passing in a config and you're done. Internally they could be setting some global state in some core library, but the developer doesn't have to think about that.
With the open-feature react SDK, you need to set the openfeature global object prior to using the provider.
Proposed interface change to the README:
If we were to do go forward with a change like this, it would probably need to be in the next major release?
The text was updated successfully, but these errors were encountered: