fix: second evaluation would have cache #2882
Conversation
rebelchris
requested review from
DragosIorgulescu,
omBratteng,
capJavert,
kopancek,
sshanzel and
nensidosari
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| if (!shouldEvaluate) { | ||
| return false as WidenPrimitives<T>; | ||
| } |
There was a problem hiding this comment.
What about features that are not bool?
There was a problem hiding this comment.
I can change it to the default value, would that make more sense I guess?
There was a problem hiding this comment.
Yes, just to support other feature flags in this hook.
There was a problem hiding this comment.
@capJavert Although, at some point we might change default to true, and then you get this experiment forever.
But guess once we hit that we can add something like valueWhenNotEvaluated or something.
There was a problem hiding this comment.
Hmm, don't know at top of my head, but sounds complicated and yes something to evaluate for future usage of the hook.
There was a problem hiding this comment.
Basically just returning something that GB does not return feels wrong right? We should hide the added UI in regards to something else and feature value should always be what GB returns, in this case "true" if user is part of experiment.
There was a problem hiding this comment.
Yeah correct, for this flow it's fine, but can imagine there might be something where it won't work ilke this.
There we could potentially use the initial evaluation for rendering determination.
Changes
Describe what this PR does
shouldEvaluatebecomes false, but it would not retrigger unless you did a hard refresh on the page.Events
Did you introduce any new tracking events?
Don't forget to update the Analytics Taxonomy sheet
Log the new/changed events below:
Please make sure existing components are not breaking/affected by this PR
Manual Testing
On those affected packages:
Did you test the modified components media queries?
Did you test on actual mobile devices?
WT-{number} #done
Preview domain
https://fix-second-eval-fix.preview.app.daily.dev