-
Notifications
You must be signed in to change notification settings - Fork 434
fix(routes): handle missing loader data in head functions #2122
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
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughTwo articles are published: an existing article receives the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/content/articles/hyprnote-publishing-stack.mdx(1 hunks)apps/web/content/articles/transcription-software-mac.mdx(1 hunks)apps/web/src/routes/_view/blog/$slug.tsx(1 hunks)apps/web/src/routes/_view/company-handbook/$.tsx(1 hunks)apps/web/src/routes/_view/docs/$.tsx(1 hunks)apps/web/src/routes/_view/integrations/$category.$slug.tsx(1 hunks)apps/web/src/routes/_view/legal/$slug.tsx(1 hunks)apps/web/src/routes/_view/roadmap/$slug.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/$slug.tsx(1 hunks)apps/web/src/routes/_view/templates/$slug.tsx(1 hunks)apps/web/src/routes/_view/vs/$slug.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/integrations/$category.$slug.tsxapps/web/src/routes/_view/vs/$slug.tsxapps/web/src/routes/_view/roadmap/$slug.tsxapps/web/src/routes/_view/legal/$slug.tsxapps/web/src/routes/_view/docs/$.tsxapps/web/src/routes/_view/shortcuts/$slug.tsxapps/web/src/routes/_view/templates/$slug.tsxapps/web/src/routes/_view/blog/$slug.tsxapps/web/src/routes/_view/company-handbook/$.tsx
🪛 LanguageTool
apps/web/content/articles/transcription-software-mac.mdx
[grammar] ~20-~20: Ensure spelling is correct
Context: ...cast/video editing | Cloud | No | Free (1hr/mo) | No | | Otter.ai | Meeting automat...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~38-~38: Ensure spelling is correct
Context: ...tware for Mac: Detailed Reviews ### 1. Hyprnote: Best for Privacy and Unlimited Local Tr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: ci
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (10)
apps/web/src/routes/_view/vs/$slug.tsx (1)
28-32: LGTM! Defensive guard properly handles missing loader data.The defensive check prevents runtime errors when
loaderData.docis absent, which can occur during SSR/SSG edge cases. Returning an empty meta array is the appropriate fallback, and moving the destructuring after the guard is correct.apps/web/src/routes/_view/templates/$slug.tsx (1)
23-27: LGTM! Consistent defensive guard implementation.The guard correctly handles missing
loaderData.templatewith the same pattern used across other routes in this PR.apps/web/src/routes/_view/shortcuts/$slug.tsx (1)
23-27: LGTM! Defensive guard properly implemented.Consistent with the pattern applied across all route files in this PR.
apps/web/content/articles/hyprnote-publishing-stack.mdx (1)
7-7: LGTM! Article marked as published.The front matter flag correctly indicates this article is ready for publication.
apps/web/src/routes/_view/roadmap/$slug.tsx (1)
21-25: LGTM! Defensive guard properly handles missing item data.The guard correctly checks for
loaderData.itemand prevents runtime errors with an appropriate fallback.apps/web/src/routes/_view/blog/$slug.tsx (1)
48-52: LGTM! Defensive guard correctly implemented.The guard handles missing
loaderData.articleproperly, preventing runtime errors during meta generation. This is particularly important given the additional published-state logic in the loader.apps/web/src/routes/_view/integrations/$category.$slug.tsx (1)
30-34: LGTM! Defensive guard properly applied.Consistent implementation of the defensive pattern for handling missing
loaderData.doc.apps/web/src/routes/_view/legal/$slug.tsx (1)
18-22: LGTM! Defensive guard completes the consistent pattern.This is the final route file in the PR with the defensive guard applied. All route files now consistently handle missing loader data, improving robustness across the application.
apps/web/src/routes/_view/docs/$.tsx (1)
47-51: Defensive guard prevents meta generation errors.The null-check correctly prevents errors when constructing meta tags. However, the same issue exists here as in the company-handbook route: the loader (line 44) asserts doc is non-null even though beforeLoad can proceed without finding a doc (lines 26-28).
apps/web/content/articles/transcription-software-mac.mdx (1)
1-270: Well-structured and comprehensive article.The front matter is properly configured with
published: true, and the article provides thorough, well-organized content comparing Mac transcription tools. The comparison table, detailed reviews, and guided recommendations create a useful resource.The static analysis hints about "1hr/mo" and "Hyprnote" are false positives—these are acceptable abbreviations and product names respectively.
| doc = allHandbooks.find((doc) => doc.slug === `${normalizedSplat}/index`); | ||
| } | ||
|
|
||
| return { doc: doc! }; |
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.
Unsafe non-null assertion can cause runtime crashes.
Line 44 asserts doc! is non-null, but when beforeLoad returns without throwing a redirect (line 27 for "about/what-hyprnote-is"), doc remains undefined. The Component (line 68) will then crash when attempting to destructure doc.
Apply this diff to handle the missing doc safely:
loader: async ({ params }) => {
const splat = params._splat || "";
const normalizedSplat = splat.replace(/\/$/, "");
let doc = allHandbooks.find((doc) => doc.slug === normalizedSplat);
if (!doc) {
doc = allHandbooks.find((doc) => doc.slug === `${normalizedSplat}/index`);
}
- return { doc: doc! };
+ return { doc: doc ?? null };
},And guard the Component:
function Component() {
const { doc } = Route.useLoaderData();
+
+ if (!doc) {
+ return <div>Document not found</div>;
+ }
return <HandbookLayout doc={doc} showSectionTitle={true} />;
}Also applies to: 68-68
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/company-handbook/$.tsx around lines 44 and 68,
avoid the unsafe non-null assertion on doc: ensure the loader returns a safe
shape when doc may be undefined (e.g., return { doc: doc ?? null } or include an
explicit error/redirect) instead of using doc!; then update the Component to
guard against a missing doc before destructuring or rendering (render a fallback
message, throw a redirect, or return null) so the component never attempts to
destructure an undefined value.
| if (!loaderData?.doc) { | ||
| return { meta: [] }; | ||
| } | ||
|
|
||
| const { doc } = loaderData; |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/routes/_view/company-handbook/$.tsx | head -80Repository: fastrepl/hyprnote
Length of output: 2710
🏁 Script executed:
#!/bin/bash
# Search for unsafe non-null assertions with doc in route files
rg -n 'doc!' apps/web/src/routes --type=ts -A2 -B2
# Search for Component functions that destructure loaderData
rg -n 'const.*=.*Route\.useLoaderData' apps/web/src/routes --type=ts -A5Repository: fastrepl/hyprnote
Length of output: 8232
Remove the non-null assertion and properly handle missing doc cases.
Line 44 uses doc! despite beforeLoad potentially returning void without finding a document (lines 25–28). This creates a contradiction: if beforeLoad succeeds without finding a doc, the loader's assertion becomes unsafe. Additionally, the Component function (line 68) directly destructures doc from useLoaderData() without any guard, unlike the head function's defensive check.
Either ensure the loader always returns a doc (by throwing redirect unconditionally when doc is not found), or remove the assertion and handle the undefined case in both the head and Component functions.
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/company-handbook/$.tsx around lines 47–51, the code
uses a non-null assertion on loaderData.doc while beforeLoad can return without
a doc; remove the unsafe `!` and either (A) make the loader always
redirect/throw when doc is missing so the route never receives undefined, or (B)
stop asserting and add defensive guards in both the head function and the
Component: check for undefined `doc` from useLoaderData(), return appropriate
meta/fallback UI or trigger a redirect/404, and ensure all accesses to doc are
conditioned on its presence.
| doc = allDocs.find((doc) => doc.slug === `${normalizedSplat}/index`); | ||
| } | ||
|
|
||
| return { doc: doc! }; |
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.
Unsafe non-null assertion can cause runtime crashes.
Same issue as the company-handbook route: line 44 asserts doc! is non-null, but beforeLoad can return without throwing (line 27 for "about/hello-world"), leaving doc undefined. The Component (line 80) will crash.
Apply this diff to handle missing doc safely:
loader: async ({ params }) => {
const splat = params._splat || "";
const normalizedSplat = splat.replace(/\/$/, "");
let doc = allDocs.find((doc) => doc.slug === normalizedSplat);
if (!doc) {
doc = allDocs.find((doc) => doc.slug === `${normalizedSplat}/index`);
}
- return { doc: doc! };
+ return { doc: doc ?? null };
},And guard the Component:
function Component() {
const { doc } = Route.useLoaderData();
+
+ if (!doc) {
+ return <div>Document not found</div>;
+ }
return <DocLayout doc={doc} showSectionTitle={true} />;
}Also applies to: 80-80
No description provided.