-
Couldn't load subscription status.
- Fork 29
chore(deps): Update ConfigCat providers to the latest version #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Simon <[email protected]>
Summary of ChangesHello @adams85, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request upgrades the ConfigCat feature flag providers to their latest stable versions, specifically updating Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This PR updates the ConfigCat providers for OpenFeature to their latest versions. While the dependency updates in package.json and the import path change in provider.service.ts are correct, this introduces critical breaking changes that are not addressed.
My review points out that the new versions of both the server and web ConfigCat providers have removed the static create method, which is still being used in the codebase. This will lead to runtime errors. Additionally, there's a peer dependency conflict with @openfeature/server-sdk caused by an outdated overrides entry in package.json. I've provided detailed comments on how to resolve these critical issues.
| "@openfeature/config-cat-web-provider": "^0.1.3", | ||
| "@openfeature/config-cat-provider": "^0.7.2", | ||
| "@openfeature/config-cat-web-provider": "^0.2.0", | ||
| "@openfeature/config-cat-provider": "^0.8.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version bump to @openfeature/[email protected] introduces two critical issues that will likely break the application at runtime.
-
Breaking API Change: The static
ConfigCatProvider.create()method has been removed. The provider instantiation inpackages/provider/src/lib/provider.service.tsat line 183 must be updated to use thenew ConfigCatProvider(...)constructor.// Correct new usage return new ConfigCatProvider({ sdkKey, options: { pollingMode: PollingMode.AutoPoll, pollIntervalSeconds: 5 } });
-
Missing and Conflicting Peer Dependency:
@openfeature/[email protected]has a peer dependency on@openfeature/server-sdk@^1.13.5.@openfeature/server-sdkis not listed in thedependenciesof thispackage.json, which can lead to runtime errors. It should be added.- The
overridessection (lines 148-150) forces@openfeature/server-sdkto version1.13.4, which conflicts with the required^1.13.5.
To fix this, you should add @openfeature/server-sdk to the dependencies and update the overrides section accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static ConfigCatProvider.create() method has been removed.
This is false, it hasn't been removed.
@openfeature/server-sdk is not listed in the dependencies of this package.json, which can lead to runtime errors. It should be added
No need, NPM installs peer dependencies automatically.
The overrides section (lines 148-150) forces @openfeature/server-sdk to version 1.13.4, which conflicts with the required ^1.13.5.
Corrected.
| import { FliptProvider } from '@openfeature/flipt-provider'; | ||
| import { ConfigCatProvider } from '@openfeature/config-cat-provider'; | ||
| import { PollingMode } from 'configcat-node'; | ||
| import { PollingMode } from '@configcat/sdk'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import change is correct, but the update to @openfeature/[email protected] introduces a breaking change that is not addressed in this PR. The static ConfigCatProvider.create() method has been removed and will cause a runtime error.
The provider instantiation at line 183 needs to be updated to use the constructor new ConfigCatProvider(...).
// packages/provider/src/lib/provider.service.ts:183
// Incorrect old usage
return ConfigCatProvider.create(sdkKey, PollingMode.AutoPoll, { pollIntervalSeconds: 5 });
// Correct new usage
return new ConfigCatProvider({ sdkKey, options: { pollingMode: PollingMode.AutoPoll, pollIntervalSeconds: 5 } });Please apply this change to avoid breaking the ConfigCat provider functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is false, ConfigCatProvider.create() hasn't been removed.
| "@nestjs/serve-static": "^4.0.2", | ||
| "@openfeature/config-cat-web-provider": "^0.1.3", | ||
| "@openfeature/config-cat-provider": "^0.7.2", | ||
| "@openfeature/config-cat-web-provider": "^0.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update to @openfeature/[email protected] also introduces a breaking change. The static create method has been removed, similar to the server provider.
Please ensure that any instantiation of ConfigCatWebProvider in the frontend code is updated to use new ConfigCatWebProvider(...) to avoid runtime errors. I cannot check the frontend code from here, but this is a likely required change based on the library's release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is false, ConfigCatWebProvider.create() hasn't been removed.
Signed-off-by: Adam Simon <[email protected]>
This PR
Updates ConfigCat providers to the latest version. See also: