Skip to content

Conversation

@marcodejongh
Copy link
Owner

  • Create draft-climbs-db.ts for IndexedDB persistence
  • Add DraftsProvider context for managing draft state
  • Modify create-climb-form to auto-save drafts with UUID
  • Add DraftsDrawer component for viewing/managing drafts

@vercel
Copy link

vercel bot commented Dec 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
boardsesh Error Error Dec 30, 2025 9:54pm

@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

Issues Found

  1. Race condition in draft initialization (create-climb-form.tsx:86-129): The hasInitializedRef check runs before async operations complete. If the component remounts quickly, a second draft could be created before the first createDraft resolves.

  2. Missing cleanup for timer on unmount (create-climb-form.tsx:152-168): The auto-save effect returns a cleanup function, but autoSaveDraft could still fire after unmount if the timer was set just before unmount. Consider using an isMounted ref or AbortController.

  3. Unused import (create-climb-form.tsx:14): constructClimbListWithSlugs is imported from @/app/lib/url-utils but the same function is already being used correctly later in the file - this is fine. However, getDraftClimbsForBoard in draft-climbs-db.ts:155 and deleteAllDraftClimbs at line 187 are exported but never used.

  4. Cancel doesn't delete draft (create-climb-form.tsx:312-322): When user clicks cancel, the draft remains in IndexedDB. This creates orphaned drafts that accumulate over time.

  5. Fork frames ignored when resuming draft (create-climb-form.tsx:92-105): When loading an existing draft, forkFrames prop is ignored. If someone navigates to the create page with forkFrames and a draftId, the draft's holds override the fork.

  6. No limit on drafts count: There's no maximum number of drafts stored. Users could accumulate hundreds of drafts in IndexedDB without realizing it.

  7. draftsCount can go negative (drafts-context.tsx:90): setDraftsCount((prev) => prev - 1) has no guard against going below zero if the state gets out of sync.

@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

Issues Found:

  1. Unused import - drafts-drawer.tsx:8 imports constructClimbListWithSlugs but never uses it

  2. Missing draft cleanup on cancel - create-climb-form.tsx:312-322 The handleCancel function navigates away but doesn't delete the draft, leaving orphaned drafts in IndexedDB

  3. Draft created even when forking - create-climb-form.tsx:107-125 When forkFrames is provided (forking an existing climb), a new draft is still created immediately, potentially creating duplicate drafts if the user had already started a draft

  4. Race condition in initialization - create-climb-form.tsx:86-129 The hasInitializedRef guard runs in a useEffect that depends on createDraft, but createDraft changes identity if DraftsProvider re-renders, potentially causing issues

  5. Missing error handling for invalid draftId - create-climb-form.tsx:92-106 When resuming a draft, if getDraftClimb returns null (draft not found), the code silently continues with isLoadingDraft set to false but no feedback to user

  6. Extra blank line - header.tsx:248 has an unnecessary double blank line

@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

packages/web/app/components/create-climb/create-climb-form.tsx

  • Race condition: Lines 86-129 - The hasInitializedRef check runs synchronously, but the async initializeDraft can race with the first render, potentially creating duplicate drafts if the component remounts quickly.
  • Memory leak: Line 47 - autoSaveTimerRef cleanup only happens in the debounce effect (line 163-166), but not if the component unmounts while a timer is pending from a different code path.
  • Cancel doesn't delete draft: Line 312-322 - handleCancel navigates away but leaves an empty draft in IndexedDB, which will accumulate over time.

packages/web/app/lib/draft-climbs-db.ts

  • Stale DB promise on error: Line 32-48 - If openDB fails, dbPromise remains set to the rejected promise, causing all subsequent calls to fail permanently. Should reset dbPromise = null on error.

packages/web/app/components/drafts/drafts-context.tsx

  • Negative count possible: Line 91 - setDraftsCount((prev) => prev - 1) can go negative if deleteDraft is called when count is already 0 (e.g., due to stale state after concurrent operations).

packages/web/app/components/board-page/header.tsx

  • Line 257-258: Extra blank line added (minor).

- Create draft-climbs-db.ts for IndexedDB persistence
- Add DraftsProvider context for managing draft state
- Modify create-climb-form to auto-save drafts with UUID
- Add DraftsDrawer component for viewing/managing drafts
- Add Drafts button with badge count in header
- Create DraftsDrawer component for viewing/managing drafts
- Store board configuration names for proper URL generation
- Fix navigation to draft climbs with slug-based URLs
- Add BoardRenderer thumbnail to each draft item
- Filter drafts by current board configuration
- Update badge count to show only relevant drafts
- Pass boardDetails to DraftsDrawer for rendering
- Create generic ClimbsList and ClimbsListItem components
- Support drag-and-drop reordering
- Support swipe actions (left/right)
- Support customizable menu items
- Update DraftsDrawer to use new ClimbsList component
- Add reorderDrafts function to drafts context

The new ClimbsList component can be reused for queue list in the future.
- Fix race condition in create-climb-form.tsx by tracking mounted state
- Fix memory leak by checking isMountedRef before async callbacks
- Fix cancel not deleting draft by adding delete call to handleCancel
- Fix stale DB promise by clearing it on error in draft-climbs-db.ts
- Fix negative count issue by using Math.max(0, prev - 1) in context
@marcodejongh marcodejongh force-pushed the claude/draft-climb-persistence-H9KWy branch from dc15b95 to f8d3547 Compare December 30, 2025 20:37
@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

packages/web/app/components/create-climb/create-climb-form.tsx

  • Line 149: setLitUpHoldsMap is not a stable reference and missing from the dependency array check - including it could cause unnecessary re-runs of the initialization effect
  • Line 119-139: When creating a new draft, setIsLoadingDraft(false) is never called in the success path, only in the draftIdFromUrl branch
  • Line 188: autoSaveDraft is in the dependency array but also triggers on currentDraft changes, creating potential save loops on draft creation

packages/web/app/components/climbs-list/climbs-list-item.tsx

  • Line 5: DeleteOutlined is imported but never used in this component

packages/web/app/components/drafts/drafts-context.tsx

  • Line 95-102: reorderDrafts only updates local state but doesn't persist the new order to IndexedDB - drafts will revert to original order on refresh

packages/web/app/lib/draft-climbs-db.ts

  • Line 32: Database promise is stored in a module-level variable that could cause issues if accessed from server-side code (IndexedDB is browser-only)

- Fix missing setIsLoadingDraft(false) on new draft creation
- Fix auto-save loops by using ref for draft UUID instead of callback
- Remove unused DeleteOutlined import from climbs-list-item.tsx
- Add SSR guard for IndexedDB module-level variable
- Add reorderDraftClimbs function and persist draft order to IndexedDB
@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

packages/web/app/components/create-climb/create-climb-form.tsx

  • Line 96-150: The hasInitializedRef check prevents re-initialization but doesn't handle the case where the user navigates to a different draftId in the URL without a full remount. Consider using draftIdFromUrl as part of the dependency check.
  • Line 137-139: Using window.history.replaceState bypasses Next.js router, which may cause state synchronization issues. Consider using router.replace() with shallow: true or Next.js's useSearchParams update pattern.

packages/web/app/components/drafts/drafts-context.tsx

  • Line 68: The setDrafts call uses an inline mapped update, but the updatedAt is set to Date.now() locally without waiting for the DB operation to complete. If the DB update fails, the local state will be inconsistent with the persisted state.

packages/web/app/lib/draft-climbs-db.ts

  • Line 207-236: The reorderDraftClimbs function modifies updatedAt timestamps to encode ordering, which conflates two different concerns (modification time vs display order). This could confuse users if they expect "recently edited" to reflect actual edits. Consider adding a dedicated sortOrder field.

packages/web/app/components/climbs-list/climbs-list-item.tsx

  • Lines 55-111: The swipe gesture handling sets state during onSwiping which triggers re-renders on every touch move event. Consider using CSS transforms via refs instead of state for smoother performance on mobile.
  • Line 169: Inline styles with multiple computed properties on each render. Consider memoizing the style object or using CSS classes.

packages/web/app/components/climbs-list/climbs-list.tsx

  • Line 26-48: The monitorForElements effect doesn't check if the source and target are the same, which could cause unnecessary reordering when dropping an item on itself.

- Fix draftId URL change handling by tracking lastInitializedDraftId
- Replace window.history.replaceState with Next.js router.replace
- Add same-item drop check in climbs-list to prevent unnecessary reorders
- Optimize swipe gestures with DOM refs instead of state for smoother UX
- Memoize inline styles in climbs-list-item
- Remove unused swipeOffset state variable
- Prefix unused boardDetails param with underscore
@claude
Copy link

claude bot commented Dec 30, 2025

Claude Review

Issues Found

  1. Race condition in draft initialization (packages/web/app/components/create-climb/create-climb-form.tsx:96-154): The effect depends on searchParams and router which can cause the URL update at line 143 to trigger a re-render, potentially causing the effect to run again despite the ref guard. Consider using a more stable approach or moving the URL update outside the effect.

  2. Missing error handling for non-existent draft (packages/web/app/components/create-climb/create-climb-form.tsx:106-119): When draftIdFromUrl is provided but getDraftClimb returns null/undefined (draft not found), the form loads with empty state but keeps the invalid draftId in URL. Should either redirect or create a new draft.

  3. Auto-save effect has stale closure risk (packages/web/app/components/create-climb/create-climb-form.tsx:162-195): generateFramesString is in the dependency array but the value captured in the timeout may be stale. Consider using a ref for the frames generation or moving the call outside the timeout.

  4. Unused parameter (packages/web/app/components/climbs-list/climbs-list.tsx:14): boardDetails: _boardDetails is destructured but never used in the component.

  5. No limit on draft accumulation: The IndexedDB store has no mechanism to limit the number of stored drafts. Users who frequently visit the create page without publishing could accumulate unlimited drafts, growing storage indefinitely.

  6. Cancel deletes draft without confirmation (packages/web/app/components/create-climb/create-climb-form.tsx:339-358): Pressing cancel immediately deletes the draft with unsaved work. Should prompt user if draft has content.

  7. Reorder function modifies updatedAt semantics (packages/web/app/lib/draft-climbs-db.ts:227-248): Using updatedAt for ordering is fragile - timestamps are meant to reflect actual update time, not display order. Future updates will break the order. Consider adding a dedicated order field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants