refactor(grapher): better handling of query params #4248
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-12-03 16:27:16 UTC |
marcelgerber
force-pushed
the
grapher-query-params
branch
from
430b69c to
2e6bdd6
Compare
marcelgerber
changed the title
| ).queryStr | ||
| tab: "chart", | ||
| country: generateSelectedEntityNamesParam(entities), | ||
| } satisfies GrapherQueryParams).queryStr |
There was a problem hiding this comment.
I had to read up on satisfies (again!). Beautiful to see it in action :)
There was a problem hiding this comment.
It's also my first use of it 🎉
In this case, it's just a way to ensure that the object actually matches that type, and to do that inline. The alternative would be to use as, but that just blindly casts it to that type and only checks the rough shape, not that it actually only specifies known properties.
So, yeah, a nice and useful addition to TS :)
marcelgerber
force-pushed
the
grapher-query-params
branch
from
2e6bdd6 to
549eaec
Compare
The way we were handling grapher query params was subpar at best - including that some of the types were not an accurate reflection of the properties that can actually be set, and so on.
d2da674 's commit message explains the changes made to the
GrapherQueryParamstype, for example.There were also some corners where we had less type-safety than we can have.
grapherConfigToQueryParamsis not used yet, but I'll need it down the road for narrative views. It takes a (partial) chart config and converts it into the query params fitting to achieve this grapher state.