-
Notifications
You must be signed in to change notification settings - Fork 212
fix: guard against undefined client in useFeatureFlagEnabled #2655
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Additional Comments (2)
-
packages/react/src/hooks/useFeatureFlagEnabled.ts, line 8 (link)logic: Inconsistent null safety - if
clientcan be undefined on line 19, it can also be undefined here whereclient.isFeatureEnabled()is called without optional chaining -
packages/react/src/hooks/useFeatureFlagEnabled.ts, line 11-13 (link)logic: Inconsistent null safety - if
clientcan be undefined on line 19, it can also be undefined here whereclient.onFeatureFlags()andclient.isFeatureEnabled()are called without optional chaining
1 file reviewed, 4 comments
|
|
||
| // if the client is not loaded yet, check if we have a bootstrapped value and then true/false it | ||
| if (!client.featureFlags.hasLoadedFlags && bootstrap?.featureFlags) { | ||
| if (!client?.featureFlags?.hasLoadedFlags && bootstrap?.featureFlags) { |
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.
style: Other similar hooks (useFeatureFlagVariantKey.ts:17, useFeatureFlagPayload.ts:17, useActiveFeatureFlags.ts:16) don't use optional chaining but this one does - consider whether this change is actually needed or if the issue is elsewhere
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react/src/hooks/useFeatureFlagEnabled.ts
Line: 19:19
Comment:
**style:** Other similar hooks (`useFeatureFlagVariantKey.ts:17`, `useFeatureFlagPayload.ts:17`, `useActiveFeatureFlags.ts:16`) don't use optional chaining but this one does - consider whether this change is actually needed or if the issue is elsewhere
How can I resolve this? If you propose a fix, please make it concise.|
Size Change: +1.74 kB (+0.03%) Total Size: 5.08 MB
ℹ️ View Unchanged
|
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
gustavohstrassburger
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.
Lets ship it and see if it fixes.
I'm assuming you didn't manage to reproduce it, right?
Problem
The check !client.featureFlags.hasLoadedFlags was causing "cannot read properties of undefined (reading hasLoadedFlags) so changed it to !client?.featureFlags?.hasLoadedFlags
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file