fix: Not Found issue on direct link access #98
Conversation
WalkthroughA new configuration file, Changes
Poem
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- frontend/vercel.json (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
frontend/vercel.json (2)
1-8: LGTM! This configuration will fix the direct link access issue.The rewrite rule correctly forwards all requests to index.html, allowing the client-side router to handle the routing properly.
4-5: Verify the impact on API routes.Ensure that this catch-all rewrite rule doesn't interfere with any API routes if they are hosted on the same domain.
✅ Verification successful
Let me analyze the results and gather more information about the backend API setup to make a conclusive verification.
This will help us understand:
- If there are any other vercel.json configurations
- How the backend API is set up and if it's on a separate domain
Based on the gathered information, I can now provide a final response as we have sufficient evidence to understand the API setup:
The catch-all rewrite rule in vercel.json is safe for API routes
The codebase shows that:
- The backend is a separate Express.js application (as seen in backend/src/index.ts)
- API calls in the frontend code are made to
/api/*endpoints- The backend appears to be deployed separately, not served from the same Vercel deployment as the frontend
The catch-all rewrite to index.html is safe because:
- The backend is a separate service, not affected by frontend's vercel.json
- API requests will be handled by your deployment platform's routing before the rewrite rule takes effect
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential API routes that might be affected rg -g '!node_modules' -g '!build' -g '!dist' "api" --type js --type tsLength of output: 2213
Script:
#!/bin/bash # Check vercel.json configurations and backend server setup fd vercel.json cat backend/src/index.ts | head -n 50Length of output: 1220
| "rewrites": [ | ||
| { | ||
| "source": "/(.*)", | ||
| "destination": "/index.html" | ||
| } | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding additional configurations for better production readiness.
While the current configuration solves the core issue, consider these enhancements for better production deployment:
- Add exceptions for static assets to prevent unnecessary rewrites
- Include security headers
- Configure caching rules
Here's an enhanced configuration:
{
"rewrites": [
{
"source": "/(.*)",
"destination": "/index.html"
}
- ]
+ ],
+ "headers": [
+ {
+ "source": "/(.*)",
+ "headers": [
+ {
+ "key": "X-Content-Type-Options",
+ "value": "nosniff"
+ },
+ {
+ "key": "X-Frame-Options",
+ "value": "DENY"
+ },
+ {
+ "key": "X-XSS-Protection",
+ "value": "1; mode=block"
+ }
+ ]
+ },
+ {
+ "source": "/static/(.*)",
+ "headers": [
+ {
+ "key": "Cache-Control",
+ "value": "public, max-age=31536000, immutable"
+ }
+ ]
+ }
+ ]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "rewrites": [ | |
| { | |
| "source": "/(.*)", | |
| "destination": "/index.html" | |
| } | |
| ] | |
| "rewrites": [ | |
| { | |
| "source": "/(.*)", | |
| "destination": "/index.html" | |
| } | |
| ], | |
| "headers": [ | |
| { | |
| "source": "/(.*)", | |
| "headers": [ | |
| { | |
| "key": "X-Content-Type-Options", | |
| "value": "nosniff" | |
| }, | |
| { | |
| "key": "X-Frame-Options", | |
| "value": "DENY" | |
| }, | |
| { | |
| "key": "X-XSS-Protection", | |
| "value": "1; mode=block" | |
| } | |
| ] | |
| }, | |
| { | |
| "source": "/static/(.*)", | |
| "headers": [ | |
| { | |
| "key": "Cache-Control", | |
| "value": "public, max-age=31536000, immutable" | |
| } | |
| ] | |
| } | |
| ] |
|
We are not using Vercel to host our app but we were "previously" using Vercel. We are completely moving out of Vercel into Cloudflare. This PR is out of the project scope. Thank you. |
This resolves the Not Found issue (#96) on direct page access through links.
Summary by CodeRabbit