-
Notifications
You must be signed in to change notification settings - Fork 649
feat: add support for feature flags #4276
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
Conversation
🦋 Changeset detectedLatest commit: 154d575 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
|
Note from UI mob pairing: add support for providing a (sync) function to check if something is enabled |
|
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
|
Great idea! Love to see it. Short feedback on usage: I think we should make the first iteration a little simpler for ourselves. I'm a little hesitant to support nested scopes right off the bat because usage in gh/gh can quickly get really hard to debug. (I'm thinking inconsistencies between pages, untested combination of feature flags, single instances of a component used with feature flag or the opposite) Ideally, I'd like to start with a broad boundary (adjacent to ThemeProvider) and once we have some experience with it (and related tooling), make it more advanced. Note for future: It would be nice if we had a common ThemeProvider across gh/gh and it was called something like PrimerProvider / PrimerContainer / PrimerBoundary (with exceptions for charting etc.) |
|
@siddharthkp would love to do something like And happy to defer on nesting if that would be helpful 👍 I think the only thing that felt critical is that we want a set of "default" feature flags (in this case the global ones) and that this should be merged in with what is given at the |
siddharthkp
left a comment
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.
Looking good!
I know you're going to write tests, approving in advance
Add an exploration for introducing feature flags to
@primer/react. This PR introduces a new component,FeatureFlags, that is responsible for enabling or disabling flags in a specific tree of components. It also introduces hooks,useFeatureFlaganduseFeatureFlags, as ways for maintainers to determine if a feature is flagged in or not.This PR also includes some docs for how to approach this for things like TypeScript, testing, and (in the future) hopefully I way to interact with flags in Storybook.
Changelog
New
FeatureFlagscomponentuseFeatureFlaghookuseFeatureFlagshookChanged
Removed
Rollout strategy
Introduce in
experimentalentrypoint for testing