-
-
Notifications
You must be signed in to change notification settings - Fork 2
Prefetch project data #176
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces a new client hook to prefetch project data and integrates it into the project layout using a numeric projectId. Simplifies keyboard shortcut handlers for creating tasks/events. Minor dependency bump and formatting-only change in a TRPC router. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Layout as Layout (projects/[projectId]/layout)
participant Hook as useProjectPrefetch
participant RQ as React Query Client
participant TRPC as TRPC Routers
participant Events as events.getByMonth
participant Projects as projects.getProjectById
participant Tasks as tasks.getTaskLists
participant TaskList as tasks.getListById
User->>Layout: Navigate to project dashboard
Layout->>Hook: useProjectPrefetch(projectId)
rect rgb(240, 248, 255)
note over Hook: On mount or projectId change
Hook->>RQ: prefetchQuery(events by month, projectId, now)
Hook->>RQ: prefetchQuery(project by id, projectId)
Hook->>RQ: prefetchQuery(task lists, projectId)
RQ->>TRPC: fetch events/projects/task lists
TRPC->>Events: Resolve monthly events
TRPC->>Projects: Resolve project details
TRPC->>Tasks: Resolve task lists
RQ-->>Hook: Cache populated
Hook->>RQ: get cached task lists
alt task lists present
loop For each task list
Hook->>RQ: prefetchQuery(task list by id)
RQ->>TRPC: fetch task list
TRPC->>TaskList: Resolve list
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
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 (7)
trpc/routers/tasks.ts (1)
170-179: Standardize error types across the router (TRPCError vs plain Error).Elsewhere in this router we throw plain Error strings (e.g., “Task list not found”, “permission…”). Consider switching these to TRPCError for consistent error handling and proper status codes over the wire.
Example patch (apply similarly in other resolvers):
- if (!oldTaskList) { - throw new Error("Task list not found"); - } + if (!oldTaskList) { + throw new TRPCError({ code: "NOT_FOUND", message: "Task list not found" }); + }package.json (1)
53-53: caniuse-lite bump is fine; ensure lockfile and Browserslist DB are consistent.After changing this dependency, please:
- Update the lockfile and install: pnpm i / yarn install.
- Optionally refresh Browserslist DB to avoid warnings in CI: npx update-browserslist-db@latest.
components/project/events/full-calendar.tsx (1)
77-77: Use a stable callback for the keyboard shortcut to avoid re-subscribing on every render.If useKeyboardShortcut depends on the handler reference, passing a new inline function each render can cause unnecessary add/remove listeners. Prefer a memoized callback.
- useKeyboardShortcut("n", () => setCreate(true)); + const openCreate = useCallback(() => setCreate(true), [setCreate]); + useKeyboardShortcut("n", openCreate);Also verify that useKeyboardShortcut ignores key events originating from inputs/textareas/contenteditable to prevent “n” from triggering while the user is typing in a field within the calendar.
app/(dashboard)/[tenant]/projects/[projectId]/layout.tsx (1)
8-15: Harden projectId parsing and guard against NaN.useParams can yield string|string[]; coercing with unary + and non-null assertion may pass NaN downstream. Guard the value before calling hooks/providers.
- const params = useParams(); - const projectId = +params.projectId!; - - useProjectPrefetch(projectId); - return ( - <TaskListsProvider projectId={projectId}>{children}</TaskListsProvider> - ); + const params = useParams(); + const idParam = Array.isArray(params.projectId) + ? params.projectId[0] + : params.projectId; + const projectId = Number(idParam); + + if (!Number.isFinite(projectId)) { + // Optionally render a fallback or just children without provider. + return <>{children}</>; + } + + useProjectPrefetch(projectId); + return ( + <TaskListsProvider projectId={projectId}>{children}</TaskListsProvider> + );Confirm TaskListsProvider behavior when given NaN; if it propagates into queries, add a runtime invariant or throw early.
components/form/task.tsx (1)
21-21: Stabilize the “n” shortcut handler and optionally gate while creating.Inline handler recreates per render; also consider suppressing while the dialog is open to avoid redundant triggers.
- useKeyboardShortcut("n", () => setIsCreating(true)); + const openCreate = useCallback(() => { + if (!isCreating) setIsCreating(true); + }, [isCreating]); + useKeyboardShortcut("n", openCreate);Please verify that useKeyboardShortcut ignores events when focus is inside inputs/textareas to prevent stealing “n” while typing in the form.
hooks/use-project-prefetch.ts (2)
12-12: Consider using explicit type checking for better robustness.Since
projectIdis typed asnumber, the truthy checkif (!projectId)will prevent prefetching whenprojectIdis0, which might be a valid ID in some systems.Apply this diff to use explicit type checking:
- if (!projectId) return; + if (projectId === null || projectId === undefined) return;Or if you want to be more explicit about invalid numeric IDs:
- if (!projectId) return; + if (!Number.isInteger(projectId) || projectId < 1) return;
60-60: Remove trailing empty line.There's an unnecessary empty line at the end of the file.
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/(dashboard)/[tenant]/projects/[projectId]/layout.tsx(1 hunks)components/form/task.tsx(1 hunks)components/project/events/full-calendar.tsx(1 hunks)hooks/use-project-prefetch.ts(1 hunks)package.json(1 hunks)trpc/routers/tasks.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-17T02:06:01.635Z
Learnt from: CR
PR: techulus/manage#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T02:06:01.635Z
Learning: Applies to trpc/routers/*.ts : Place tRPC routers (user, settings, projects, tasks, events, search) under trpc/routers with one file per router
Applied to files:
trpc/routers/tasks.ts
🔇 Additional comments (3)
trpc/routers/tasks.ts (1)
34-36: Formatting-only change looks good.No behavior change; the TRPCError construction is identical. Safe to merge.
hooks/use-project-prefetch.ts (2)
7-58: Well-structured prefetching hook with efficient query orchestration.The hook effectively prefetches project-related data in a logical sequence, first loading base project data and task lists, then drilling down into individual task list details. The use of
Promise.allSettledensures that individual query failures don't block the entire prefetch process, and the conditional prefetch of individual task lists based on the availability of task lists data is a smart optimization.
43-53: Script to inspect thegetTaskListsimplementation has been submitted.
Summary by CodeRabbit