⚡ Bolt: Derive project lists in-memory to reduce redundant API requests#152
⚡ Bolt: Derive project lists in-memory to reduce redundant API requests#152aicoder2009 wants to merge 1 commit into
Conversation
When fetching a project, the detail page was fetching both the project's lists (`/api/projects/${projectId}/lists`) and all of the user's lists (`/api/lists`) simultaneously. This optimization removes the redundant API call and derives the project's lists in-memory by filtering the globally fetched lists.
This prevents duplicate backend database queries, saving a round-trip and significantly improving Time to First Byte (TTFB).
Co-authored-by: aicoder2009 <127642633+aicoder2009@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements an in-memory subset filtering pattern: the project page now fetches all lists globally and filters them client-side to match the current project, eliminating a dependent API request. The change is documented in new guidance and applied to the project page's data-fetching logic. ChangesIn-Memory List Filtering Pattern
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/projects/[id]/page.tsx (2)
209-211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate
allListsto reflect the new projectId assignment.When an existing list is added to the project, the server updates its
projectId, butallListsis not synchronized. This creates stale state that will cause inconsistencies if the list is later removed or if the user navigates away and returns.🔧 Proposed fix to sync allLists state
if (result.success) { const addedList = allLists.find((l) => l.id === listId); if (addedList) { setLists((prev) => [...prev, { ...addedList, projectId }]); + setAllLists((prev) => + prev.map((l) => (l.id === listId ? { ...l, projectId } : l)) + ); } posthog.capture("list_added_to_project", { project_id: projectId });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/projects/`[id]/page.tsx around lines 209 - 211, The code pushes an existing list into lists but leaves allLists stale; update the corresponding entry in allLists when assigning projectId. In the block that finds addedList (symbol: addedList) and calls setLists, also call setAllLists and immutably map over previous allLists replacing the item where l.id === listId with a copy that has projectId set (use the same projectId variable), so the global list state reflects the server-side assignment and avoids later inconsistencies.
187-187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate
allListsto reflect the removed projectId assignment.When a list is removed from the project, the server sets its
projectIdtonull, butallListsretains the staleprojectId. This breaks theavailableListscomputation (lines 231-234), preventing the user from re-adding the list without refreshing the page.🔧 Proposed fix to sync allLists state
if (result.success) { setLists((prev) => prev.filter((l) => l.id !== listId)); + setAllLists((prev) => + prev.map((l) => (l.id === listId ? { ...l, projectId: null } : l)) + ); posthog.capture("list_removed_from_project", { project_id: projectId });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/projects/`[id]/page.tsx at line 187, The current removal only updates local project-specific lists via setLists((prev) => prev.filter((l) => l.id !== listId)) but leaves allLists with the stale projectId, breaking availableLists; update state so the removed list's projectId is set to null in allLists as well. After performing the existing setLists update, call setAllLists(prev => prev.map(l => l.id === listId ? { ...l, projectId: null } : l)) (or merge this into a single state updater) so availableLists calculation sees the list as unassigned and can be re-added; reference setLists, setAllLists, allLists, availableLists, projectId, and listId when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/projects/`[id]/page.tsx:
- Around line 209-211: The code pushes an existing list into lists but leaves
allLists stale; update the corresponding entry in allLists when assigning
projectId. In the block that finds addedList (symbol: addedList) and calls
setLists, also call setAllLists and immutably map over previous allLists
replacing the item where l.id === listId with a copy that has projectId set (use
the same projectId variable), so the global list state reflects the server-side
assignment and avoids later inconsistencies.
- Line 187: The current removal only updates local project-specific lists via
setLists((prev) => prev.filter((l) => l.id !== listId)) but leaves allLists with
the stale projectId, breaking availableLists; update state so the removed list's
projectId is set to null in allLists as well. After performing the existing
setLists update, call setAllLists(prev => prev.map(l => l.id === listId ? {
...l, projectId: null } : l)) (or merge this into a single state updater) so
availableLists calculation sees the list as unassigned and can be re-added;
reference setLists, setAllLists, allLists, availableLists, projectId, and listId
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cf0e724-f8f9-40b6-8f3a-54269cee8f34
📒 Files selected for processing (2)
.jules/bolt.mdsrc/app/projects/[id]/page.tsx
💡 What: Removed the redundant fetch for
/api/projects/${projectId}/listson the project details page and derived the data in-memory by filtering the result of/api/lists.🎯 Why: When a user visits a project page, the application was making an API call for a specific project's lists and another API call for ALL lists concurrently. This caused duplicate database queries on the backend since the DynamoDB implementation of
getProjectListsalready fetches all user lists and filters them.📊 Impact: Reduces backend database queries by half (for lists) and eliminates one full network round-trip from the client, noticeably improving Time to First Byte (TTFB).
🔬 Measurement: Inspect the Network tab on
/projects/[id]. You will no longer see the/api/projects/[id]/listsrequest firing. The UI should populate identical list data just as fast or faster.PR created automatically by Jules for task 1241041028634973475 started by @aicoder2009
Summary by CodeRabbit
Refactor
Chores