Skip to content

feat(generators): expose underlying OpenFeature client on generated clients#236

Open
kriscoleman wants to merge 1 commit intoopen-feature:mainfrom
kriscoleman:feat/expose-underlying-client
Open

feat(generators): expose underlying OpenFeature client on generated clients#236
kriscoleman wants to merge 1 commit intoopen-feature:mainfrom
kriscoleman:feat/expose-underlying-client

Conversation

@kriscoleman
Copy link
Copy Markdown
Collaborator

Summary

  • Exposes the underlying OpenFeature SDK client on all generated clients, allowing users to perform ad-hoc flag evaluations beyond what is defined in their manifest
  • Each generator exposes the client in an idiomatic way for its language/framework:
    • Go: Exported Client package-level variable
    • Node.js: readonly client: Client property on GeneratedClient interface
    • Java: getOpenFeatureClient() method on GeneratedClient interface
    • C#: public IFeatureClient Client property on GeneratedClient
    • React: Re-exports useOpenFeatureClient hook from @openfeature/react-sdk
    • Angular: client getter on GeneratedFeatureFlagService returning the underlying FeatureFlagService
    • Python: Already publicly accessible via self.client (no change needed)
    • NestJS: Inherits client property from Node.js GeneratedClient (no change needed)

Test plan

  • All existing golden file tests pass
  • Go, Node.js, and C# integration tests updated to verify client accessibility
  • Verify generated Go code compiles with generated.Client access
  • Verify generated Node.js code works with client.client for ad-hoc evaluations
  • Verify generated Java code compiles with getOpenFeatureClient() method
  • Verify generated C# code compiles with Client property access
  • Verify React re-export of useOpenFeatureClient works in a React app
  • Verify Angular client getter works in an Angular app
  • Verify Python self.client continues to work as expected
  • Verify NestJS inherits client property correctly

…lients

Allow users to access the underlying OpenFeature SDK client for ad-hoc
flag evaluations beyond what is defined in the manifest.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the code generation templates for Angular, C#, Go, Java, Node.js, and React to expose the underlying OpenFeature client, enabling ad-hoc flag evaluations. Review feedback identifies potential naming collisions in the Go and Node.js generators where the generic name 'client' might conflict with user-defined flags. It is recommended to use more specific identifiers, such as 'OpenFeatureClient' or 'openFeatureClient', to avoid compilation errors and ensure the generated code remains robust regardless of the flag manifest content.

Comment on lines +26 to +28
// Client is the underlying OpenFeature client used for flag evaluations.
// It can be used directly for ad-hoc flag evaluations beyond what is defined in the manifest.
var Client = openfeature.NewDefaultClient()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using Client as the name for the exported package-level variable may cause a naming collision if a feature flag in the manifest is also named client. When transformed via ToPascal, the flag variable would also be named Client. Consider using a more distinct name like OpenFeatureClient to avoid this risk.

// OpenFeatureClient is the underlying OpenFeature client used for flag evaluations.
// It can be used directly for ad-hoc flag evaluations beyond what is defined in the manifest.
var OpenFeatureClient = openfeature.NewDefaultClient()

Stringer: stringer({{ .Key | Quote }}),
Value: func(ctx context.Context, evalCtx openfeature.EvaluationContext) {{- if eq (.Type | OpenFeatureType) "Object"}}any{{- else}}{{ .Type | TypeString }}{{- end}} {
return client.{{ .Type | OpenFeatureType }}(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType) "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)
return Client.{{ .Type | OpenFeatureType }}(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType) "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update this reference to match the suggested name change for the exported client variable.

	     return OpenFeatureClient.{{ .Type | OpenFeatureType }}(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType)  "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)

},
ValueWithDetails: func(ctx context.Context, evalCtx openfeature.EvaluationContext) (openfeature.GenericEvaluationDetails[{{- if eq (.Type | OpenFeatureType) "Object"}}any{{- else}}{{ .Type | TypeString }}{{- end}}], error){
return client.{{ .Type | OpenFeatureType }}ValueDetails(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType) "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)
return Client.{{ .Type | OpenFeatureType }}ValueDetails(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType) "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update this reference to match the suggested name change for the exported client variable.

	     return OpenFeatureClient.{{ .Type | OpenFeatureType }}ValueDetails(ctx, {{ .Key | Quote }}, {{ if eq (.Type | OpenFeatureType)  "Object" }}{{.DefaultValue | ToMapLiteral }}{{- else }}{{ .DefaultValue | QuoteString }}{{- end}}, evalCtx)

fmt.Printf("themeCustomization: %v\n", themeCustomization)

// Verify the underlying client is accessible for ad-hoc evaluations
_ = generated.Client
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the test to use the suggested OpenFeatureClient name.

Suggested change
_ = generated.Client
_ = generated.OpenFeatureClient

Comment on lines +24 to +25
/** The underlying OpenFeature client for ad-hoc flag evaluations. */
readonly client: Client;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding a client property to the GeneratedClient interface introduces a potential naming collision with any feature flag named client. Since flags are exposed as methods on this interface (e.g., client(...)), having a property and a method with the same name will result in a TypeScript compilation error. Consider using a more specific name such as openFeatureClient.

  /** The underlying OpenFeature client for ad-hoc flag evaluations. */
  readonly openFeatureClient: Client;

const client = domain ? OpenFeature.getClient(domain, context) : OpenFeature.getClient(context)

return {
client,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the property name to match the suggested change in the GeneratedClient interface.

    openFeatureClient: client,

Comment on lines +65 to +67
if (!client.client) {
throw new Error('Underlying OpenFeature client not exposed');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the test to use the suggested openFeatureClient property name.

Suggested change
if (!client.client) {
throw new Error('Underlying OpenFeature client not exposed');
}
if (!client.openFeatureClient) {
throw new Error('Underlying OpenFeature client not exposed');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant