fix(lld): fix analytics useEffect in default #6871
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Ignored Deployments
|
| const dispatch = useDispatch(); | ||
|
|
||
| useEffect(() => { | ||
| if (isLocked) return; |
There was a problem hiding this comment.
I think it is safer to wrap this:
| if (isLocked) return; | |
| if (!isLocked) { |
In previous use cases with useEffect, having a return like this does not necessarily exit the function.
LucasWerey
force-pushed
the
fix/lld_locked_analytics
branch
from
54a3c64
to
af30868
Compare
| if (!isLocked) { | ||
| if (!analyticsFF?.enabled || hasSeenAnalyticsOptInPrompt) return; |
There was a problem hiding this comment.
| if (!isLocked) { | |
| if (!analyticsFF?.enabled || hasSeenAnalyticsOptInPrompt) return; | |
| if (!isLocked && (analyticsFF?.enabled || !hasSeenAnalyticsOptInPrompt)) { |
LucasWerey
force-pushed
the
fix/lld_locked_analytics
branch
from
af30868
to
7857c4e
Compare
| dispatch(setShareAnalytics(false)); | ||
| dispatch(setSharePersonalizedRecommendations(false)); | ||
| }); | ||
| if (!isLocked && analyticsFF?.enabled && !hasSeenAnalyticsOptInPrompt) { |
There was a problem hiding this comment.
why are we changing the logic? previously it was an or case?
There was a problem hiding this comment.
It does the same as before because now we execute the logic instead of returning when the if is true
There was a problem hiding this comment.
Yes it was an or case but we want to trigger the dispatches only if those 3 conditions are met:
- App unlocked
- feature flag is enabled
- the prompt has not been seen by the user
The OR case was a return if the user didn't see the prompt or if the FF is disabled
So we have the same thing here as before
✅ Checklist
npx changesetwas attached.📝 Description
Fix analytics useEffect previously introduced that was overriding app.json
❓ Context
🧐 Checklist for the PR Reviewers