feat(generators): expose underlying OpenFeature client on generated clients#236
feat(generators): expose underlying OpenFeature client on generated clients#236kriscoleman wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
kriscoleman
force-pushed
the
feat/expose-underlying-client
branch
from
ad09654 to
87b6d22
Compare
|
done. |
kriscoleman
force-pushed
the
feat/expose-underlying-client
branch
from
87b6d22 to
cac1f24
Compare
…lients Allow users to access the underlying OpenFeature SDK client for ad-hoc flag evaluations beyond what is defined in the manifest. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
…ion warning Flag keys that transform to a reserved generator symbol (Client in Go, client in Node.js) are excluded from the generated output and a warning is emitted at generation time. Documents the reserved symbols per generator in the generators README. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
kriscoleman
force-pushed
the
feat/expose-underlying-client
branch
from
cac1f24 to
879a742
Compare
📝 WalkthroughWalkthroughAll language generators (Angular, C#, Go, Java, Node.js, React) are updated to expose the underlying OpenFeature client for ad-hoc flag evaluations alongside the manifest-defined typed methods. Go, Node.js, and React generators add reserved-name collision detection that filters and warns on flags whose transformed keys collide with the newly reserved exported symbol names. A shared ChangesExpose Underlying OpenFeature Client Across All Generators
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)internal/generators/README.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/generators/react/react.tmpl`:
- Around line 9-11: The Generate() method in the React generator needs to
implement reserved-name filtering to prevent collisions with the re-exported
useOpenFeatureClient symbol. Create a reservedNames map that includes at minimum
"useOpenFeatureClient", then call FilterFlagset() to filter the flagset before
passing it to the template rendering, following the same pattern implemented in
the golang.go and nodejs.go generators.
In `@internal/generators/README.md`:
- Around line 71-73: The fenced code block containing the warning message about
the "client" flag is missing a language identifier, which triggers the
markdownlint MD040 rule. Add the language identifier "text" after the opening
triple backticks on the line before "Flag "client" transforms to "Client"" to
properly specify the code block language and resolve the linting issue.
In `@test/go-integration/test.go`:
- Around line 108-110: The current assertion on line 109 only verifies that
generated.Client exists at compile-time by assigning it to the blank identifier.
Replace the blank identifier assignment with an explicit nil check using a
conditional or assertion statement that will cause the test to fail if
generated.Client is nil at runtime, ensuring the underlying client is actually
exposed and accessible during test execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 982d8ac5-af8f-4798-b230-9e644bc5d9c5
📒 Files selected for processing (18)
internal/cmd/testdata/success_angular.goldeninternal/cmd/testdata/success_csharp.goldeninternal/cmd/testdata/success_go.goldeninternal/cmd/testdata/success_java.goldeninternal/cmd/testdata/success_nodejs.goldeninternal/cmd/testdata/success_react.goldeninternal/generators/README.mdinternal/generators/angular/angular.tmplinternal/generators/csharp/csharp.tmplinternal/generators/golang/golang.gointernal/generators/golang/golang.tmplinternal/generators/java/java.tmplinternal/generators/nodejs/nodejs.gointernal/generators/nodejs/nodejs.tmplinternal/generators/react/react.tmpltest/csharp-integration/Program.cstest/go-integration/test.gotest/nodejs-integration/test.ts
The React generator re-exports the useOpenFeatureClient hook but did not guard against a flag key transforming into that same name. A flag key such as "openFeatureClient" would generate `export const useOpenFeatureClient`, duplicating the re-exported symbol and breaking the generated TypeScript. React now filters reserved symbols like the Go and Node.js generators do. The duplicated inline filter loops are consolidated into a shared generators.FilterReservedFlags helper covered by a unit test. C# and Java intentionally do not reserve a name: their exposed client symbols (a `Client` property and a no-arg `getOpenFeatureClient()`) cannot collide with the `<Pascal>Async`/`<camel>(ctx)` members generated from flag keys, so a guard there would falsely exclude valid flags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/generators/react/react.go (1)
47-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix comment to match implementation.
The comment on line 48 states "use + ToPascal" but the implementation on line 56 uses
strcase.ToCamel. WhileToPascalandToCamelare functionally equivalent (both produce PascalCase), the comment should match the code for clarity.📝 Suggested fix
// reservedNames are symbols exported by the React generator itself. Flag keys -// whose generated hook name (use + ToPascal) matches one of these will be +// whose generated hook name (use + ToCamel) matches one of these will be // excluded from the generated output and a warning will be emitted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/generators/react/react.go` around lines 47 - 52, The comment above the reservedNames map states that the generated hook name uses "use + ToPascal" but the actual implementation uses strcase.ToCamel. Update the comment on line 48 to replace "ToPascal" with "ToCamel" to accurately reflect the implementation of the hook name generation logic.internal/generators/reserved_test.go (1)
18-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider additional test cases for edge scenarios.
The current test suite covers basic collision detection but misses some edge cases:
- Empty input flagset
- All flags colliding (should return empty flagset)
- Preservation of flag fields beyond
KeyandType(if any exist)Additionally, line 75 uses
t.Fatalfinside a loop, which stops at the first mismatch. Considert.Errorfto report all differences.📝 Suggested additional test cases
+ { + name: "empty flagset", + generator: "Go", + reserved: map[string]bool{"Client": true}, + transform: strcase.ToCamel, + input: []string{}, + want: []string{}, + }, + { + name: "all flags collide", + generator: "Go", + reserved: map[string]bool{"Client": true, "Config": true}, + transform: strcase.ToCamel, + input: []string{"client", "config"}, + want: []string{}, + },And improve the assertion:
for i := range got { if got[i] != tt.want[i] { - t.Fatalf("got %v, want %v", got, tt.want) + t.Errorf("index %d: got %v, want %v", i, got, tt.want) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/generators/reserved_test.go` around lines 18 - 80, The TestFilterReservedFlags function needs additional test coverage for edge cases and improved assertion logic. Add new test cases to cover empty input flagset, scenario where all flags collide with reserved names and should return empty result, and preservation of flag fields beyond Key and Type. Additionally, in the test loop where the output is compared against expected values, replace t.Fatalf with t.Errorf so that all mismatches are reported rather than stopping at the first difference, allowing better visibility into all failures in a single test run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/generators/react/react.go`:
- Around line 47-52: The comment above the reservedNames map states that the
generated hook name uses "use + ToPascal" but the actual implementation uses
strcase.ToCamel. Update the comment on line 48 to replace "ToPascal" with
"ToCamel" to accurately reflect the implementation of the hook name generation
logic.
In `@internal/generators/reserved_test.go`:
- Around line 18-80: The TestFilterReservedFlags function needs additional test
coverage for edge cases and improved assertion logic. Add new test cases to
cover empty input flagset, scenario where all flags collide with reserved names
and should return empty result, and preservation of flag fields beyond Key and
Type. Additionally, in the test loop where the output is compared against
expected values, replace t.Fatalf with t.Errorf so that all mismatches are
reported rather than stopping at the first difference, allowing better
visibility into all failures in a single test run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b055423f-b9f5-4827-9e3a-b10923c29a0b
📒 Files selected for processing (6)
internal/generators/README.mdinternal/generators/golang/golang.gointernal/generators/nodejs/nodejs.gointernal/generators/react/react.gointernal/generators/reserved.gointernal/generators/reserved_test.go
✅ Files skipped from review due to trivial changes (1)
- internal/generators/README.md
1 participant
Summary
Clientpackage-level variablereadonly client: Clientproperty onGeneratedClientinterfacegetOpenFeatureClient()method onGeneratedClientinterfacepublic IFeatureClient Clientproperty onGeneratedClientuseOpenFeatureClienthook from@openfeature/react-sdkclientgetter onGeneratedFeatureFlagServicereturning the underlyingFeatureFlagServiceself.client(no change needed)clientproperty from Node.jsGeneratedClient(no change needed)Test plan
generated.Clientaccessclient.clientfor ad-hoc evaluationsgetOpenFeatureClient()methodClientproperty accessuseOpenFeatureClientworks in a React appclientgetter works in an Angular appself.clientcontinues to work as expected