-
Notifications
You must be signed in to change notification settings - Fork 49
feat: provide a local singleton of OpenFeatureAPI #1303
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?
Conversation
Signed-off-by: MattIPv4 <[email protected]>
48eff17 to
618f9ea
Compare
Signed-off-by: MattIPv4 <[email protected]>
618f9ea to
98c3bc0
Compare
|
Hey @MattIPv4, thanks for the proposal. I see the following things to consider here: 1. Higher Level SDKs and integrations 2. Provider lifecycle 3. API clarity I think allow more than one API makes sense. And I think the way you drafted it makes sense. To me concern 2 and 3 should be considered though. What you you think @MattIPv4? |
|
Hey!
To avoid this being a breaking change, I don't want to switch the higher-level SDKs over to using the isolated instance by default. I can see there being a world in which users are wanting the global singleton behaviour there, even in a micro-frontend -- you might want the provider set by the core of the frontend to be accessible by micro-frontends using the higher-level SDKs. I think my suggestion would be to add an
Absolutely agree with this, adding a locking mechanism so that a provider can't be reused makes sense. Though, unless I've missed something, this is already a problem today, as you can add a provider to multiple domains? Do we want to continue allowing this (in which case we'll want to track a ref to the API instance), or prevent that as part of the new locking mechanism (in which case we can just track a lock bool)?
I'm not sure I follow why you wouldn't want a package-local singleton? If you import this in multiple files, you'd want the same instance to interact with? Or are you suggesting you'd prefer folks to manage this fully themselves and export/import a single instance around their app on their own? I don't really see any benefit to the latter; it seems like a worse DX for no benefit (once you've got package-level isolation to solve the SDK versioning issues, domains give you any additional isolation you'd need, I think)? |
That's a valid point. I think this would work |
Binding to multiple domains is not a problem. The SDK tracks if a provider is still used in another domain or the default domain. Only if a provider is not registered for any domain anymore, it is There really must be two API instances for this to be a problem.
Good question, I think we can solve this in another issue. I will open one up for this explicitly. |
Yes I do. But I am not 100% sure if we want it or if the package-local singleton solves enough of an issue while we may not want to handler others for now. Thinking about it, I might be leaning towards agreeing with the package-local singleton and domains beeing sufficient, |
Ah good to know, I hadn't poked around closely enough at how this works today!
That sounds good to me -- leave it as a known gap in this implementation for now (given this is very explicitly opt-in as a whole, I'd hope folks aren't going to mix and match both), and then address it separately down the road 👍 |
This PR
Allows for folks who do not wish to interact with the global singleton instance of the OpenFeatureAPI in the web-sdk to instead use a package-local singleton instance of the OpenFeatureAPI. This should be useful for micro-frontend environments where there is a risk for different versions of the SDK being loaded by micro-frontends and colliding in
windowdue to the global singleton mechanism.Related Issues
https://cloud-native.slack.com/archives/C06E4DE6S07/p1764115826255009
Notes
N/A
Follow-up Tasks
N/A
How to test
tbd.