open-feature / js-sdk-contrib

OpenFeature Providers and Hooks for JavaScript
https://openfeature.dev
Apache License 2.0
35 stars 37 forks source link

feat: ConfigCat web provider #816

Open lukas-reining opened 6 months ago

lukas-reining commented 6 months ago

We should add a ConfigCat web provider. The discussion started in a PR of @adams85.

Thanks for the release, @lukas-reining. I did a quick test and found that the new version (v0.6.0) works fine.

Maybe we should also consider developing a client-sdk provider, as this is a server sdk provider. Which makes you change even more important.

Since our SSR SDK can handle both targets, I guess the client provider would be more or less just a duplicate of the server one (at least, in our case). I'd avoid maintaining another package if possible.

BTW, NPM packages kind of support multitargeting via the browser field. AFAIK, most of the bundlers support it nowadays. I have plans to look into the viability of this approach because it would be a godsend to us if we could merge our various JS SDK packages (or at least some) into one.

Originally posted by @adams85 in https://github.com/open-feature/js-sdk-contrib/issues/796#issuecomment-2010312985

@adams85 I am fully with you trying to not have a second provider codebase for the web that is just a copy of the server SDK, but in my opinion this is not the case for OpenFeature.

Problem

The provider interface of the @openfeature/web-sdk is different from the one in the @openfeature/server-sdk. Mainly the difference from the API side is that the web SDK evaluates synchronous while the server SDK evaluates async. This is due to the web SDK using the static context paradigm while the web SDK uses the dynamic context paradigm. The reasoning for this can be seen at Petes article in our blog https://openfeature.dev/blog/catering-to-the-client-side.

Solution

To reduce the amount of duplicate code we should move context-tranformer.ts and utilities like toResolutionDetails from config-cat-provider.ts to a shared folder. For the server there would be noting more left to do.

For the client I think we could use snapshots for the synchronous flag evaluation. The snapshot could be made in the initialize method of the provider as described in your docs and then be auto-polled. The rest of the logic should be same besides being sync then.

lukas-reining commented 6 months ago

What do you think @adams85 and @laliconfigcat? If you think this solutions is fine I could add the web-provider, if you do not want to do it.

adams85 commented 6 months ago

Hi @lukas-reining,

We discussed this with @laliconfigcat and have nothing against a seperate provider for browsers. We don't have plans to actively involved in the implementation, will however gladly answer your questions that may arise w.r.t. the ConfigCat SDK and do a code review if you come up with a solution. So, please go ahead.

For the client I think we could use snapshots for the synchronous flag evaluation. The snapshot could be made in the initialize method of the provider as described in your docs and then be auto-polled. The rest of the logic should be same besides being sync then.

Considering the architecture of the @openfeature/web-sdk, I also think that snapshots with auto polling is the way to go.

Just keep in mind please that there may be an initialization period when the data needed for evaluation is not present in the ConfigCat SDK's internal cache yet. (In the case of configcat-js this data is stored in the browser's local storage, so it may be available right away or may be stale/missing, thus, may be necessary to be downloaded from the remote server.)

During this initialization period, snapshots won't give you any meaningful results, but merely return the default value passed to the getValue method. (You can detect this state by checking the cacheState property of the snapshot.)

If the architecture of the @openfeature/web-sdk allows it, the best way to deal with this problem is to call the waitForReady method of the ConfigCat client and wait for the returned Promise to complete. After that point it is guaranteed that the evaluation data is present in the local cache - unless initialization has timed out, see also the maxInitWaitTimeSeconds option.

lukas-reining commented 6 months ago

Hey @adams85,

this sounds really good! Sorry for the late answer, I have been traveling and there were several other things to do.

We discussed this with @laliconfigcat and have nothing against a seperate provider for browsers. We don't have plans to actively involved in the implementation, will however gladly answer your questions that may arise w.r.t. the ConfigCat SDK and do a code review if you come up with a solution. So, please go ahead.

That is great and I will do this in the next weeks then. I guess it will not be much but will be great for ConfigCat support in OF.

If the architecture of the @openfeature/web-sdk allows it, the best way to deal with this problem is to call the waitForReady method of the ConfigCat client and wait for the returned Promise to complete. After that point it is guaranteed that the evaluation data is present in the local cache - unless initialization has timed out, see also the maxInitWaitTimeSeconds option.

An OF provider can implement an initialize function so this can be completely done by OF waiting for waitForReady in the initialize. This is idiomatic in OF and seems to be the was you described.

Thanks for your proposals and I am looking forward to implement it and see what you think!