fix: don't break personal dashboard charts if the flag is called .
#8807
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
| @@ -97,7 +97,7 @@ const EmptyFlagMetricsChart = () => { | |||
| const useMetricsEnvironments = (project: string, flagName: string) => { | |||
| const [environment, setEnvironment] = useState<string | null>(null); | |||
| const { feature } = useFeature(project, flagName); | |||
| const activeEnvironments = feature.environments.map((env) => ({ | |||
| const activeEnvironments = (feature?.environments ?? []).map((env) => ({ | |||
There was a problem hiding this comment.
Is the return type of useFeature lying to us? It would indicate the defensive technique is not needed looking at the useFeature return type
There was a problem hiding this comment.
Yeah, good point. It's both lying and we're abusing it.
First, it's wrapping an http request. This means we might not have the data immediately, but we don't wait. Also, there may not be any data there (we might get an error), but we don't handle that (did someone say union types?).
But second, the useFeature hook returns the deprecated IFeatureToggle interface. If you check that interface, you'll find that environments is always present. But if you compare it to the FeatureSchema which supersedes it, you'll see that environments is optional (yikes!).
There was a problem hiding this comment.
Ideally we should fix the source of truth (the useFeature hook type) so that we don't have to catch those mistakes at runtime
There was a problem hiding this comment.
Aye. I'll make a follow-up PR to gauge how much work that'd be.
thomasheartman
deleted the
1-3132/don't-break-charts-if-flag-can't-be-accessed
branch
This PR fixes an issue where the personal dashboard would fail to render if the flag was called
.(Curiously, it was not an issue with..; probably because they end up accessing different URLs).I've taken the very pragmatic approach here of saying "right, we know that
.and..cause issues, let's just not even try to fetch data for them".The option, of course, is to bake in more error handling in the components, but due to how we've got hooks depending on each other, it's a bit of a rabbit hole to go down. I think this is a good compromise for now.
So now, you'll get this instead:
I've also gone and updated the text for when we get a metrics fetching error, because this probably isn't due to the flag name anymore. If it is, we want to know.