-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove hardcoded dynamic routes #1145
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
WalkthroughThis PR removes various Next.js route/page configuration exports (revalidate, dynamic, dynamicParams) across multiple app and API routes and updates the Caps dashboard page to use PageProps<"/dashboard/caps"> with minor prop handling reordering. No functional logic changes are introduced in handlers or components. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 1
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/page.tsx (1)
109-110: Consider keeping the original ordering.The
userIdassignment was moved from before thepage/limitcomputation to after. While this change is functionally safe (no dependency exists between them), the reordering appears arbitrary and doesn't improve readability or performance.Consider reverting to the original ordering for consistency, or if there's a specific reason for this change, it would be helpful to understand the rationale.
const page = Number(searchParams.page) || 1; const limit = Number(searchParams.limit) || 15; - - const userId = user.id; const offset = (page - 1) * limit; + const userId = user.id;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/(org)/dashboard/caps/page.tsx(1 hunks)apps/web/app/(org)/layout.tsx(0 hunks)apps/web/app/(site)/download/[platform]/route.ts(0 hunks)apps/web/app/api/changelog/status/route.ts(0 hunks)apps/web/app/api/download/route.ts(0 hunks)apps/web/app/api/releases/tauri/[version]/[target]/[arch]/route.ts(0 hunks)apps/web/app/api/status/route.ts(0 hunks)apps/web/app/api/thumbnail/route.ts(0 hunks)apps/web/app/embed/[videoId]/page.tsx(0 hunks)apps/web/app/robots.ts(0 hunks)apps/web/app/s/[videoId]/page.tsx(0 hunks)
💤 Files with no reviewable changes (10)
- apps/web/app/api/download/route.ts
- apps/web/app/s/[videoId]/page.tsx
- apps/web/app/api/changelog/status/route.ts
- apps/web/app/(site)/download/[platform]/route.ts
- apps/web/app/embed/[videoId]/page.tsx
- apps/web/app/robots.ts
- apps/web/app/api/thumbnail/route.ts
- apps/web/app/api/status/route.ts
- apps/web/app/(org)/layout.tsx
- apps/web/app/api/releases/tauri/[version]/[target]/[arch]/route.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/caps/page.tsx (1)
packages/web-backend/src/Auth.ts (1)
getCurrentUser(10-31)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
| export default async function CapsPage(props: { | ||
| searchParams: Promise<{ [key: string]: string | string[] | undefined }>; | ||
| }) { | ||
| export default async function CapsPage(props: PageProps<"/dashboard/caps">) { |
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
Verify PageProps type is available.
The function signature now uses PageProps<"/dashboard/caps">, but this type is not imported in the file. Confirm that PageProps is either:
- A global type provided by Next.js
- Defined in a custom types file that's globally available via
tsconfig.json - Should be imported explicitly
Run the following script to verify the PageProps type definition:
🏁 Script executed:
#!/bin/bash
# Search for PageProps type definition in the codebase
rg -n --type=ts 'type PageProps' -A 5
# Search for PageProps imports
rg -n --type=ts "import.*PageProps" -C 2
# Check if it's used elsewhere in the codebase
rg -n --type=ts 'PageProps<' -C 2Length of output: 1773
Import PageProps in CapsPage
Add the missing type import so PageProps<"/dashboard/caps"> is resolved. For example:
import type { PageProps } from 'next/app';at the top of apps/web/app/(org)/dashboard/caps/page.tsx.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/page.tsx around line 99, the generic
PageProps<"/dashboard/caps"> is used but PageProps is not imported; add an
import type for PageProps from 'next/app' at the top of the file (e.g., import
type { PageProps } from 'next/app';) so the type resolves.
Next is able to detect which routes are dynamic and static, these annotations aren't needed
Summary by CodeRabbit
New Behavior
Refactor
Chores