fix: handle page cases not found on middleware#356
Conversation
WalkthroughThis pull request introduces a new dependency to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/middleware.ts (2)
9-10: Consider adding reportId validation constraintsWhile the pattern correctly matches the URL structure, consider adding constraints to the reportId capture group to prevent potential security issues with malformed or overly long IDs.
- const reportCasesPattern = /^\/studio\/reports\/([^\/]+)\/cases$/ + const reportCasesPattern = /^\/studio\/reports\/([a-zA-Z0-9-_]{1,50})\/cases$/
17-20: Consider validating report existence before redirectThe middleware redirects without validating if the target report exists. This could lead to a poor user experience if the report doesn't exist.
Consider implementing one of these approaches:
- Add report validation in the middleware (might impact performance)
- Handle the not-found case in the target page component
- Add a loading state during validation
Would you like me to provide an example implementation for any of these approaches?
src/components/ui/scroll-area.tsx (1)
8-24: Consider adding size constraints and ScrollBar orientation.The ScrollArea component looks good overall, but there are a few suggestions for improvement:
- Add explicit height/width constraints to prevent layout issues:
- className={cn("relative overflow-hidden", className)} + className={cn("relative overflow-hidden min-h-[100px]", className)}
- Consider passing orientation to ScrollBar for explicit control:
- <ScrollBar /> + <ScrollBar orientation="vertical" /> + <ScrollBar orientation="horizontal" />package.json (1)
29-29: Consider lazy loading for optimal bundle size.Since ScrollArea might not be needed on initial page load, consider dynamic imports to reduce the initial bundle size:
// Example implementation in your scroll-area.tsx const ScrollArea = dynamic(() => import('@radix-ui/react-scroll-area'), { ssr: false });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json(1 hunks)src/components/ui/scroll-area.tsx(1 hunks)src/middleware.ts(1 hunks)
🔇 Additional comments (5)
src/middleware.ts (1)
17-20: Verify potential redirect loop edge case
The current implementation might create a redirect loop if the target URL /studio/reports/{id} also matches the pattern. Let's verify this scenario.
✅ Verification successful
No redirect loop risk found
Based on the directory structure and routing patterns, there's no risk of a redirect loop. The middleware redirects to /studio/reports/[reportSlug], and this path is handled by a proper Next.js route at src/app/(studio)/studio/reports/[reportSlug]/. The pattern matching in middleware only matches the source URL pattern, not the destination URL structure, preventing any potential redirect loops.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any routes that could cause redirect loops
# Look for route handlers or pages that might match both URL patterns
# Search for files handling report routes
fd -e ts -e tsx -e js -e jsx | xargs rg -l "reports/\[.*\]"
# Search for potential conflicting route patterns
ast-grep --pattern 'createRouteMatcher($pattern)'
Length of output: 113
Script:
#!/bin/bash
# Let's try a different approach to check for route patterns and middleware logic
# Check the middleware implementation to understand the pattern matching
rg -A 5 -B 5 "pathname.*reports" src/middleware.ts
# Look for report-related routes and handlers
rg -l "reports.*id|reports/\[" app/
# Check for any other middleware or route handlers that might affect reports
rg -l "middleware.*reports|reports.*redirect" src/ app/
Length of output: 484
Script:
#!/bin/bash
# Let's check the project structure and routing patterns
# Find all potential route files and directories
fd -t d
# Look for report-related routes in any directory
rg -l "reports.*id|reports/\[" .
# Check the pattern matching logic in middleware
rg -A 3 "const match = " src/middleware.ts
Length of output: 1199
src/components/ui/scroll-area.tsx (3)
1-7: LGTM! Imports are well-organized.
The imports are properly structured with the "use client" directive correctly placed at the top for client-side rendering.
48-48: LGTM! Clean exports.
The exports are properly structured, making both components available for use in other parts of the application.
26-46: Verify the scrollbar thumb contrast ratio.
The implementation looks solid, but let's verify the visibility of the scrollbar thumb:
Also, consider adding hover states for better user interaction:
<ScrollAreaPrimitive.ScrollAreaThumb
- className="relative flex-1 rounded-full bg-border"
+ className="relative flex-1 rounded-full bg-border hover:bg-border/80 transition-colors"
/>✅ Verification successful
The scrollbar thumb contrast is properly implemented with theme-aware colors
The bg-border class uses HSL values that provide sufficient contrast in both light and dark modes:
- Light mode:
--border: 240 5.9% 90%(light gray) - Dark mode:
--border: 240 3.7% 15.9%(dark gray)
The current implementation is correct as it:
- Automatically adapts to the theme
- Uses system colors that maintain proper contrast ratios
- Follows the design system's color tokens
The suggestion for hover states is still valid for better interactivity, but the base contrast implementation is sound.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the bg-border token definition in the theme configuration
rg -l "bg-border" --type css --type typescript
Length of output: 81
Script:
#!/bin/bash
# Search for bg-border token definition in all files
rg "bg-border"
# Also search for tailwind or theme configuration files
fd -e config.js -e config.ts
Length of output: 388
Script:
#!/bin/bash
# Check the tailwind config for border color definition
cat tailwind.config.ts
# Also check if there's a CSS variables file that might define these colors
fd -e css | xargs cat
Length of output: 4076
package.json (1)
29-29: LGTM! Verify latest stable version.
The addition of @radix-ui/react-scroll-area maintains version consistency with other Radix UI components.
✅ Verification successful
Version 1.2.1 is up-to-date with reasonable bundle size
The package is using the latest stable version (1.2.1) from NPM, and the bundle size is minimal at ~6.4KB gzipped, which is reasonable for a scroll area component with accessibility features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the latest version and bundle size of @radix-ui/react-scroll-area
# Check NPM for latest version
echo "Latest version from NPM:"
npm show @radix-ui/react-scroll-area version
# Check bundle size impact
echo "Bundle size analysis:"
curl -s "https://bundlephobia.com/api/size?package=@radix-ui/react-scroll-area@1.2.1" | jq '{size: .size, gzip: .gzip}'
Length of output: 322
1 participant
This is an automated pull request for branch develop
Summary by CodeRabbit
New Features
ScrollAreaandScrollBarcomponents for enhanced scrollable areas in the UI.Chores
@radix-ui/react-scroll-areadependency to support new scroll components.