open-feature / js-sdk

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

Nest.js SDK #705

Open lukas-reining opened 7 months ago

lukas-reining commented 7 months ago

We should build a Nest.js Module, as Nest.js is a really popular backend framework for JavaScript.

The following is the current idea of what feature could be interesting:

Non-functional requirements:

luizgribeiro commented 7 months ago

That sounds good! I have some questions though.

Dynamic module with forRoot receiving a list of OpenFeature clients Probably providers, no? I think that something like the following structure would do the job:

{
     defaultProvider: Provider();
     namedProviders: [
       {
          name: "Some provider",
          provider: Provider()
       }
     ]
}

This way we can keep it simple for those who only want a default provider but also support different scenarios.

A decorator to inject a feature flag into a method You mean a parameter decorator? Maybe one limitation is the fact that flag evaluation gives a Promise. I'm not shure if this will work

I'd also say that a Method/Controller/Handler decorator can be useful. Enabling or disabling an enpoint from a decorator might be useful

beeme1mr commented 7 months ago

It would also be great if it were possible to configure the injected client to contain request-scoped context properties. That would allow you to easily set context like targetingKey, email, or ip that would apply to all flag evaluations made by the client.

By the way, the OpenFeature playground app uses Nest. We should keep that in mind as we build out this SDK.

lukas-reining commented 7 months ago

It would also be great if it were possible to configure the injected client to contain request-scoped context properties. That would allow you to easily set context like targetingKey, email, or ip that would apply to all flag evaluations made by the client.

Ya totally, changed the requirements.

lukas-reining commented 7 months ago

Probably providers, no? I think that something like the following structure would do the job:

Yes this could make sense. But I would be fine with any implementation that fits the provider/client implementation that we have right now.

lukas-reining commented 7 months ago

I'd also say that a Method/Controller/Handler decorator can be useful. Enabling or disabling an enpoint from a decorator might be useful.

This sounds good! The question here will be if the ergonomics of this feature would be good and looking what disabling of an endpoint means.

lukas-reining commented 7 months ago

I will start to bring some of the existing code into a draft PR and then we can continue to work on this.

luizgribeiro commented 7 months ago

The question here will be if the ergonomics of this feature would be good and looking what disabling of an endpoint means.

I think that returning 404 in this case would mean that it's disabled.

I see that we're coming up with multiple ideas and it will probably make it harder to implement all at once. What do you think about defining deliverable scopes for this and make it incremental?

luizgribeiro commented 7 months ago

A decorator to inject a feature flag into a method (if good to implement)

Do you have any idea on how to implement this? I did a few tests with async param decorators but didn't have any success :disappointed:

lukas-reining commented 7 months ago

Do you have any idea on how to implement this? I did a few tests with async param decorators but didn't have any success 😞

Yes but I am not sure about the ergonomics. The following works, and instead of returning a Promise<EvaluationDetails> we could also wrap it in an observable. In the end we will have to decide if it is "usable" enough.

export interface OpenFeatureStringFlagProps {
  clientName?: string;
  flagKey: string;
  defaultValue: string;
  context?: EvaluationContext;
}

export const OpenFeatureStringFlag= createParamDecorator(
  ({ clientName, flagKey, defaultValue, context }: OpenFeatureFeatureProps) => {
    const client = clientName ? OpenFeature.getClient(clientName) : OpenFeature.getClient();
    return client.getStringDetails(flagKey, defaultValue, context);
  },
);
import { Controller, Get } from '@nestjs/common';
import { EvaluationDetails } from '@openfeature/server-sdk';
import { OpenFeatureFeature } from './feature.decorator';

@Controller()
export class ExampleController {
  @Get('/tenant')
  public async handleRequest(
    @OpenFeatureStringFlag({ clientName: 'myCLient', flagKey: 'tenantName', defaultValue: 'BigCorp' })
    feature: Promise<EvaluationDetails<string>>,
  ) {
    const details = await feature;
    return details.value;
  }
}
lukas-reining commented 7 months ago

I think that returning 404 in this case would mean that it's disabled.

Yes, it would only be important, that this behaves exactly like the endpoint not existing, I would say.

luizgribeiro commented 7 months ago

I've started sketching a module here but I'm not 100% confident about this. I'll probably study libs such as nest/mongoose before coming up with anything barely usable. I haven't done much Dynamic modules stuff from ground. My main concern about this is how to test it properly.

lukas-reining commented 7 months ago

I've started sketching a module here but I'm not 100% confident about this. I'll probably study libs such as nest/mongoose before coming up with anything barely usable. I haven't done much Dynamic modules stuff from ground. My main concern about this is how to test it properly.

As promised, I started to integrate this too. Here is the first draft of the most basic functions: https://github.com/open-feature/js-sdk/pull/718 I chose a testing strategy, maybe you can give some feedback. From this PR we could start and add features to the SDK, or change the API completely. Would be happy to do this together @luizgribeiro.

@luizgribeiro I also wanted to add you as reviewer, but you are not in the org. Would you like to joint the org? Then we could add you.

lukas-reining commented 6 months ago

We are discussing context propagation here: #736