-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: Hide timezones behind FF #26478
base: master
Are you sure you want to change the base?
Conversation
We're seeing incosistent values in production, let's hide it behind a FF for now
WebStatsBreakdown.Timezone | ||
) | ||
: null, | ||
].filter((x) => x !== null), |
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.
Yes, it's complex, TS wouldn't let me otherwise
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.
It's temporary, so I'm fine with how bad this looks
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.
We have isNotNil
as a type guard, which can work well for this kind of thing. You might be able to add .filter(isNotNil)
to the outer array, and let this be null without needing to create an inner array
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.
That didn't work on its own, but I changed the code slightly, it looks nicer now
This is simpler than what we had before
Size Change: +56 B (0%) Total Size: 1.16 MB ℹ️ View Unchanged
|
We're seeing inconsistent values in production, let's hide it behind a FF for now
See #26475