Add granular applications list component #27908
Conversation
…roduct availability component and add parentheses handling, remove partial for granular controls list
cdrubin
requested review from
a team,
kodster28,
maxvp and
pedrosousa
as code owners
github-actions
bot
added
product:cloudflare-one
product:rules
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
| display_id: display_id, | ||
| applications: item.applications, | ||
| }; | ||
| } |
There was a problem hiding this comment.
What you're looking for here is a reduce
const lookup = data.reduce(
(acc, item) => {
// do stuff
return {...acc, newEntry}
},
{} as Record<string, GranularControlApplication>
)
There was a problem hiding this comment.
I have recently started to use less of the map and reduce functions than previously. I don't find them as easy to read as the explicit actions in a loop and for the work being done here readability is a stronger consideration I think.
| > = { | ||
| loader: middlecacheLoader("v1/application-controls/applications.json", { | ||
| parser: (fileContent: string) => { | ||
| const data = JSON.parse(fileContent); |
There was a problem hiding this comment.
Can you confirm that this fileContent string will match the zod schema in this unconventional loader pattern? Will you need to use zod to validate this string? I think Astro will apply the schema against the the lookup value you return, but are you guaranteed that this data is a valid of array of X?
(I say X because I think it wouldn't match the schema you gave Astro but some other schema. And now that I type that out I'm guessing that Astro is not validating this string since you would need to provide two schemas, right?)
There was a problem hiding this comment.
Astro is validating the data in fileContent against schema provided and, if there are issues throws a [InvalidContentEntryDataError] product-availability → XXX data does not match collection schema for that array item
| for (const item of data) { | ||
| const display_id = item.category | ||
| .split("-") | ||
| .map((w: string) => w[0].toUpperCase() + w.substring(1).toLowerCase()) |
There was a problem hiding this comment.
You had to manually specify string here? It wasn't able to infer that?
There was a problem hiding this comment.
Oh, I think that's because you need a second schema. So it makes sense that at this point in the flow it doesn't know that category is a string.
Some of that is semantic "make TS happy" but the reason TS isn't happy is because you have a potential bug here. If the thing you write to the file isn't in the right shape, then this code will fail. There's no check that category exists and is a string. There's also no check that item.applications exists, but I think when Astro checks the resulting lookup that would effectively get checked.
(There's also no check that w has a length > 2, but TS wouldn't save you there.)
My original recommendation of using a standard pattern for object loader still applies. But if we're too far down this path, I think you'll want more validation.
…gory display name generation function for granular application control applications
* Add granular applications list component, improve error handling of product availability component and add parentheses handling, remove partial for granular controls list * Improve compliance with possible return values from Astro loader custom parser * Improved use of Typescript and zod for typing and more resilient category display name generation function for granular application control applications
5 participants
Uh oh!
There was an error while loading. Please reload this page.