-
Notifications
You must be signed in to change notification settings - Fork 4
Set rewards page as application home #1118
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?
Set rewards page as application home #1118
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes simplify the SaaS app's layout and dashboard by removing multiple navigation and banner components, introducing a new migration banner, and updating routing logic. The root page and certain redirects now point directly to the rewards section. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RootPage
participant NextRouter
User->>RootPage: Access "/"
RootPage->>NextRouter: redirect("/my-dashboard/rewards")
NextRouter-->>User: Navigates to "/my-dashboard/rewards"
sequenceDiagram
participant User
participant SaasLayout
participant AppMigrationBanner
User->>SaasLayout: Load dashboard layout
SaasLayout->>AppMigrationBanner: Render migration alert
AppMigrationBanner-->>User: Show migration info and new app link
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 0
🧹 Nitpick comments (1)
app/(saas)/my-dashboard/layout.tsx (1)
19-19: Remove unused hook call or utilize its return value.The
useMyRewardsTransactionsSidepanel()hook is called but its return value is not used. According to the hook implementation, it returns methods likeopento control the sidepanel. If the sidepanel functionality is no longer needed, this hook call can be removed.If the hook call is no longer needed, apply this diff:
- useMyRewardsTransactionsSidepanel();Alternatively, if the sidepanel functionality might be needed later, consider adding a comment explaining why the hook is called without using the return value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/(saas)/layout.tsx(1 hunks)app/(saas)/my-dashboard/layout.tsx(1 hunks)app/page.tsx(1 hunks)next.config.mjs(2 hunks)shared/constants/router.ts(1 hunks)shared/features/app-migration-banner/app-migration-banner.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/(saas)/my-dashboard/layout.tsx (3)
shared/panels/my-rewards-transactions-sidepanel/my-rewards-transactions-sidepanel.hooks.ts (1)
useMyRewardsTransactionsSidepanel(3-5)shared/features/page-content/page-content.tsx (1)
PageContent(8-23)app/(saas)/my-dashboard/_features/request-payment/request-payment.tsx (1)
RequestPayment(13-63)
🔇 Additional comments (8)
next.config.mjs (1)
11-11: LGTM! Routing changes align with PR objectives.The redirect destinations have been correctly updated to point to
/my-dashboard/rewards, which aligns with the PR objective to set the rewards page as the application home. The use ofpermanent: trueis appropriate for these redirects.Also applies to: 35-35
shared/constants/router.ts (1)
4-4:Details
✅ Verification successful
Verify consistent usage of home route constant across the codebase.
The change correctly aligns the home route constant with the new redirect destinations. However, ensure that all references to
NEXT_ROUTER.home.rootthroughout the codebase are still valid with this new path.
🏁 Script executed:
#!/bin/bash # Description: Find all usages of NEXT_ROUTER.home.root to ensure they work with the new path # Search for usages of the home route constant rg -A 3 -B 3 "NEXT_ROUTER\.home\.root|home\.root"Length of output: 2723
All home route references verified
We found and reviewed all instances ofNEXT_ROUTER.home.rootin the codebase—including:
- shared/providers/auth-provider.tsx
- app/not-found.tsx
- app/error.tsx
- app/(splash)/github-callback/page.tsx
- app/impersonate/[userId]/page.tsx
Each now correctly points to
/my-dashboard/rewardswith no remaining hard-coded/. No further changes are needed.shared/features/app-migration-banner/app-migration-banner.tsx (1)
1-22:Details
✅ Verification successful
LGTM! Well-structured migration banner component.
The component is well-implemented with clear messaging about the app migration. The use of the design system components (
AlertandBaseLink) ensures consistency with the application's UI patterns.Verify that the external URL is accessible and correct:
🏁 Script executed:
#!/bin/bash # Description: Verify the external URL in the migration banner is accessible # Check if the URL is accessible curl -I -s https://contribute.onlydust.com | head -1Length of output: 66
External Link Verified – Ready to Merge
The external URL (
https://contribute.onlydust.com) returns HTTP 200 and is accessible. No further changes are needed; this banner component is good to merge.app/page.tsx (1)
1-5:Details
✅ Verification successful
Verify this is the correct Next.js pattern for root page redirects.
The implementation looks clean, but using
redirect()directly in a component function might not be the recommended Next.js pattern. Consider if this should be implemented differently.
🌐 Web query:
What is the recommended way to implement immediate redirects in Next.js App Router root pages?💡 Result:
Recommended Way to Implement Immediate Redirects in Next.js App Router Root Pages
To implement immediate (server-side) redirects on root pages using Next.js with the App Router, the recommended approach is to use the
redirectfunction fromnext/navigationdirectly within your Server Component (such asapp/page.jsorapp/page.tsx). This method ensures the redirect happens before any content is rendered and issues the correct HTTP status code for search engines and clients[3][4][5].Example: Immediate Redirect in a Root Page
// app/page.js or app/page.tsx import { redirect } from 'next/navigation'; export default function Page() { redirect('/target-path'); // Immediately redirects to /target-path // No content is ever rendered return null; }Key Points:
- Using
redirect()in a Server Component (i.e., in your root page file) causes an instant 307 (Temporary Redirect) by default[4][5].- No visual content is rendered to the user; the redirect happens as soon as the request is received.
- This method is best for unconditional, immediate redirects (e.g., moving the homepage to another route)[3][5].
- Do not wrap
redirect()in atry/catchblock, as it throws internally to trigger the redirect flow[3].
Alternative Approaches
Using
next.config.jsRedirectsFor static path-based redirects (e.g., always redirecting
/to/new-home), you can configure redirects innext.config.js:// next.config.js module.exports = { async redirects() { return [ { source: '/', destination: '/new-home', permanent: true, // or false for temporary }, ]; }, };
- This method is ideal for handling simple, unconditional redirects at the routing level, ensuring the redirect happens even before the application code runs[6][7][8].
Middleware
You can also use Middleware (
middleware.js) for programmatic, conditional, or authentication-based redirects. Middleware runs before a request reaches your application and can redirect based on headers, cookies, or other request data[2][10].
Summary Table
Method Where Used When to Use Example Use Case redirectin Server ComponentRoot/App page (app/page.js) Immediate, unconditional redirects Changing root page redirectsin next.config.jsProject root Static, path-based redirects Deprecated URLs, SEO moves Middleware (NextResponse) middleware.js Conditional or request-based logic Auth or A/B testing
Best Practice
- For immediate, unconditional redirects on root pages (such as redirecting
/to/dashboard), use theredirectfunction fromnext/navigationin your Server Component. This is the most direct and idiomatic approach in Next.js App Router[3][4][5].- For static, path-based redirects (e.g., for SEO or legacy support), add rules to
next.config.js[6][7][8].This guarantees optimal performance and proper status codes for both users and search engines.
Citations:
- 1: https://nextjs.org/docs/app/building-your-application/routing/redirecting
- 2: https://www.contentful.com/blog/next-js-redirect/
- 3: https://nextjs.org/docs/app/building-your-application/routing/linking-and-navigating
- 4: https://nextjs.org/docs/app/api-reference/functions/redirect
- 5: https://www.creolestudios.com/nextjs-14-redirect-with-loading-indicator/
- 6: https://nextjs.org/docs/app/api-reference/config/next-config-js/redirects
- 7: https://refine.dev/blog/next-js-redirect/
- 8: https://blog.logrocket.com/redirects-next-js/
- 9: vercel/next.js#67664
- 10: https://focusreactive.com/next-js-redirects/
Immediate Redirect Pattern Confirmed
The use of
redirect()fromnext/navigationdirectly in the root Server Component (app/page.tsx) is the recommended Next.js App Router approach for immediate, unconditional redirects. No additional changes are needed here.app/(saas)/layout.tsx (2)
17-17: Migration banner placement looks appropriate.The
AppMigrationBanneris correctly placed within theSidebarInsetand will be displayed at the top of the layout, which is appropriate for a migration notification.
10-10:Details
✅ Verification successful
Verify the AppMigrationBanner component exists and is properly implemented.
The import looks correct, but let's ensure the component is available and properly implemented.
🏁 Script executed:
#!/bin/bash # Description: Verify AppMigrationBanner component exists and check its implementation # Expected: Component should exist and be properly implemented # Check if the AppMigrationBanner component exists fd "app-migration-banner.tsx" --type f # Check the component implementation cat "shared/features/app-migration-banner/app-migration-banner.tsx" 2>/dev/null || echo "Component file not found"Length of output: 790
AppMigrationBanner component verified
- Found at
shared/features/app-migration-banner/app-migration-banner.tsx- Implementation uses
AlertandBaseLinkcorrectly and matches the intended migration banner design- Import path in
app/(saas)/layout.tsxis valid and no further changes are neededapp/(saas)/my-dashboard/layout.tsx (2)
3-3: Import cleanup looks good.The simplified import of just
PropsWithChildrenaligns with the reduced functionality in this component.
24-26: Simplified layout structure aligns with migration objectives.The new layout focuses only on the
RequestPaymentcomponent, which aligns with the PR objective of redirecting to the rewards page. The right-aligned placement (self-end) provides good visual hierarchy.
Summary
/page redirect to rewardsTesting
npm run lint(fails:nextnot found)npm test(fails:vitestnot found)Summary by CodeRabbit
New Features
UI Changes
Navigation Updates