open-feature / js-sdk

JavaScript SDK for OpenFeature
https://openfeature.dev
Apache License 2.0
147 stars 32 forks source link

[FEATURE] React SDK: Simplify bootstrapping of the OpenFeatureProvider #1047

Open wichopy opened 3 days ago

wichopy commented 3 days ago

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:


-// Instantiate and set our provider (be sure this only happens once)!
-// Note: there's no need to await its initialization, the React SDK handles re-rendering and suspense for you!
-OpenFeature.setProvider(new InMemoryProvider(flagConfig));

// Enclose your content in the configured provider
function App() {
  return (
-    <OpenFeatureProvider>
+    <OpenFeatureProvider provider={provider}>
      <Page></Page>
    </OpenFeatureProvider>
  );
}

+//For domain scoped providers
+function App() {
+  return (
+    <OpenFeatureProvider provider={provider} domain={domain}>
+      <Page></Page>
+    </OpenFeatureProvider>
+  );
+}

If we were to do go forward with a change like this, it would probably need to be in the next major release?

beeme1mr commented 1 day ago

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 .

lukas-reining commented 1 day ago

The default provider is always the same since we're using a global singleton.

Yes for the default provider this would lead to strange behavior as @beeme1mr says.

However, we could potentially create the behavior you're proposing by leveraging a domain behind the scenes.

This could be an idea. This feels like a OpenFeature internal use of domains.

beeme1mr commented 1 day ago

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.

lukas-reining commented 1 day ago

On the other hand one could argue that what @wichopy proposed fits perfectly to what the context mutator does. There the domain is also implied from the context and transparent to the caller of mutateContext. So for consistency we should keep these in sync maybe. What do you think @toddbaert @beeme1mr ?

wichopy commented 1 day ago

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 OpenFeature.setProvider into the react-sdk.

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>;
}