open-feature / js-sdk

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

Make OpenFeature class implement EvaluationLifeCycle interface #13

Closed toddbaert closed 2 years ago

toddbaert commented 2 years ago

The OpenFeature API object (class) implements the methods defined on EvaluationLifeCycle, but doesn't implement the interface, because static members cannot implement an interface (at least trivially). This means there could be drift between the hook methods on the client, and the API.

We should find a solution for this, either with some typing magic, or by perhaps exporting an instance directly instead of using static methods.

anudeepreddy commented 2 years ago

Hi,

Could we do something like below:

export const OpenFeature: EvaluationLifeCycle = _OpenFeature;
                //↑____ Error is thrown here when _OpenFeature doesn't implement the methods and properties from EvaluationLifeCycle

This approach does the job of enforcing the contract but I am not sure if this is the right way to achieve it.

toddbaert commented 2 years ago

I actually think the right approach might be to NOT export a class with static methods, but export an instance itself. That instance can be a class that we don't allow to be instantiated outside the API code.

anudeepreddy commented 2 years ago

I have worked on this. I have made the following changes:

Should I go ahead and raise a PR?

toddbaert commented 2 years ago

@anudeepreddy go ahead and open a PR. The only thing that sounds like it could be a problem is: made the getInstance static method public.

I think instead I would just create an instance and make that the export. Then you are basically exporting a singleton, which should work I think.

toddbaert commented 2 years ago

Closed with https://github.com/open-feature/node-sdk/pull/44