-
Notifications
You must be signed in to change notification settings - Fork 14
Validate and canonicalize generated toolkit routes #1065
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
base: main
Are you sure you want to change the base?
Changes from all commits
35ecefc
98bf485
1734bc6
60b763c
cec2576
58c188d
baee0a0
7dd396b
bac2dce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a toolkit can’t produce a valid URL, this code throws an error in one place and catches it in another. That works, but it’s harder to follow than needed. Simpler approach: if we can’t build a URL, just skip that toolkit and move on — no throw/catch needed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also: Sidebar and canonical URLs now sanitize categories (so bad values like ../../outside get mapped to others), but integration card links don’t seem to do that yet. Should those use the same category cleanup so cards and pages always match?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done —
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed — cards now use the same client-safe |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /** | ||
| * Shared integration category allow-list and normalization. | ||
| * | ||
| * Kept free of Node/fs imports so client components (integration cards) and | ||
| * server route helpers can share one contract without pulling server-only code | ||
| * into the browser bundle. | ||
| */ | ||
|
|
||
| export const INTEGRATION_CATEGORIES = [ | ||
| "productivity", | ||
| "social", | ||
| "entertainment", | ||
| "development", | ||
| "payments", | ||
| "search", | ||
| "sales", | ||
| "databases", | ||
| "customer-support", | ||
| "others", | ||
| ] as const; | ||
|
|
||
| export type IntegrationCategory = (typeof INTEGRATION_CATEGORIES)[number]; | ||
|
|
||
| export function normalizeCategory( | ||
| value: string | null | undefined | ||
| ): IntegrationCategory { | ||
| if (!value) { | ||
| return "others"; | ||
| } | ||
|
|
||
| return INTEGRATION_CATEGORIES.includes(value as IntegrationCategory) | ||
| ? (value as IntegrationCategory) | ||
| : "others"; | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds a fair amount of validation code. Is that solving a real problem we’ve hit (bad rows in index.json), or is it defensive? If we haven’t seen bad data in practice, we might get most of the benefit from just skipping toolkits with invalid slugs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly defensive + consistency with the earlier request to use Zod here (generator already validates with Zod; we can’t import those schemas into Practical benefit we kept: envelope validates, then filters bad index rows instead of dropping the whole catalog. Happy to slim this further to “skip invalid slugs only” if you’d rather — the slug skip path is now consistent everywhere either way. |
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.
Empty docsLink fails to override
Medium Severity
Explicit empty
docsLinkvalues from toolkit JSON are not correctly applied ingetToolkitsWithDocsLinks. ThedocsLink ? { docsLink } : {}spread condition treats empty strings as falsy, allowing existing design-systemdocsLinks to persist. This can cause card slugs to diverge from generated routes, which correctly interpret an emptydocsLinkas a fallback to the ID slug.Reviewed by Cursor Bugbot for commit baee0a0. Configure here.