Skip to content

fix(favorites): keep favorites in sync across devices - #1463

Merged
2witstudios merged 2 commits into
masterfrom
pu/favorites
May 31, 2026
Merged

2witstudios merged 2 commits into
masterfrom
pu/favorites

Conversation

@2witstudios

@2witstudios 2witstudios commented May 31, 2026 •

Copy link
Copy Markdown
Owner

Problem

Favorites are database-backed, but the client reads them through a Zustand store that persisted isSynced to localStorage. Once a device had synced, it never re-fetched, so:

  • Favorites added/removed on another device (e.g. mobile) never appeared — the device kept rendering its stale localStorage copy.
  • Add/remove operated against that stale cache: remove silently no-oped when the favorite wasn't in the local favorites array (the DELETE was never sent), and add bounced off a 409 "already favorited" (the star rolled back even though it was favorited).

Fix

  • Stop persisting isSynced — every load now revalidates against the database (stale-while-revalidate; the persisted list remains only a fast first-paint cache).
  • Reset isSynced on rehydrate — not persisting it isn't enough on its own: users upgrading from a build that did persist isSynced: true would have it merged back by Zustand's default merge, re-skipping the load-time revalidation. A custom persist merge now forces isSynced: false on every rehydrate, covering the legacy-storage upgrade path. (Addresses review feedback.)
  • useFavoritesSync hook — revalidates on mount and on window focus / visibilitychange (throttled to 3s), so a backgrounded device (mobile) picks up changes made elsewhere without a manual reload. Wired into FavoritesSection, DriveSwitcher, and DrivesBrowser, replacing their duplicated if (!isSynced) fetch effects.
  • removeFavorite reconciles with the server when the favorite is missing from the local cache (fetch → find real id → delete), instead of silently no-oping.
  • addFavorite treats a duplicate (409) as success after reconciling with the server, instead of rolling the star back; only rolls back when the server genuinely can't be reached.

Files

  • apps/web/src/hooks/useFavorites.ts — store changes + useFavoritesSync hook + persist merge
  • apps/web/src/components/layout/left-sidebar/FavoritesSection.tsx
  • apps/web/src/components/layout/navbar/DriveSwitcher.tsx
  • apps/web/src/components/drives/DrivesBrowser.tsx
  • apps/web/src/hooks/__tests__/useFavorites.test.ts — reconcile-path tests

Validation

  • CI green: Unit Tests, Lint & TypeScript Check, CodeRabbit, and Vercel preview all pass.
  • Added store tests for both reconcile paths (stale add → 409 reconcile resolves; stale remove → fetch-then-delete calls the API) and updated the two existing failure-path tests to be deterministic about the new reconcile fetch.

Notes

  • Possible follow-up: real-time invalidation over Socket.IO so other open sessions update live without waiting for focus/reload.

🤖 Generated with Claude Code

Favorites are DB-backed, but the client store persisted `isSynced` to
localStorage, so a device that had synced once never revalidated. Changes
made on another device (e.g. mobile) never appeared, and add/remove operated
against a stale cache (remove silently no-oped; add bounced off a 409).

- Stop persisting `isSynced` so every load revalidates (stale-while-revalidate)
- Add `useFavoritesSync`: revalidate on mount and on window focus /
  visibilitychange (throttled) so backgrounded devices pick up remote changes
- `removeFavorite` reconciles with the server when the entry is missing from
  the local cache instead of silently no-oping
- `addFavorite` treats a duplicate (409) as success after reconciling
- Wire the hook into FavoritesSection, DriveSwitcher, DrivesBrowser
- Add store tests for the reconcile paths

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 31, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
pagespace-marketing Ready Ready Preview, Comment May 31, 2026 3:42pm

@coderabbitai

coderabbitai Bot commented May 31, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR refactors favorites synchronization across the codebase by introducing a dedicated useFavoritesSync hook and improving the core useFavorites hook's optimistic update and server reconciliation logic. Components no longer manually manage favorites initialization via useEffect; they call useFavoritesSync() instead, reducing duplicated logic and improving consistency.

Changes

Favorites Synchronization Refactor

Layer / File(s) Summary
useFavorites hook: optimistic add/remove with server reconciliation
apps/web/src/hooks/useFavorites.ts
addFavorite and removeFavorite now snapshot local state before mutations, refetch from the server on errors or missing items, and roll back optimistic updates if server confirmation fails. Favorites are reconciled with server truth when inconsistencies arise.
New useFavoritesSync hook and persistence changes
apps/web/src/hooks/useFavorites.ts
A new exported useFavoritesSync() hook automatically revalidates favorites on component mount (when not yet synced) and on focus/visibilitychange events, throttled to prevent bursts. Persistence configuration is updated to stop persisting isSynced; only favorites and lookup Sets remain persisted.
Test updates for optimistic and reconciliation behavior
apps/web/src/hooks/__tests__/useFavorites.test.ts
Tests for addFavorite validate rollback on combined POST and fetch failure, and successful reconciliation when a "already favorited" error is followed by server confirmation. Tests for removeFavorite validate cleanup when items are inconsistent locally but absent on the server, and API calls when the server has the item despite local cache staleness.
Component migration to useFavoritesSync
apps/web/src/components/drives/DrivesBrowser.tsx, apps/web/src/components/layout/left-sidebar/FavoritesSection.tsx, apps/web/src/components/layout/navbar/DriveSwitcher.tsx
DrivesBrowser, FavoritesSection, and DriveSwitcher are updated to import and call useFavoritesSync(), removing manual useEffect blocks that conditionally called fetchFavorites when !isSynced. Each component now destructures only the state and actions it needs from useFavorites, simplifying initialization logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • 2witstudios/PageSpace#306: The main PR's refactor of useFavorites/favorites syncing (adding and wiring useFavoritesSync, changing isSynced persistence, and adjusting favorite initialization in UI components) is directly built on the earlier dashboard work that introduced the DB-synced, optimistic useFavorites flow and favorites API/schema changes.

  • 2witstudios/PageSpace#706: Both PRs touch the favorites integration in the Drives experience—main PR refactors DrivesBrowser.tsx to initialize/sync favorites via useFavoritesSync, while the retrieved PR's DrivesBrowser.tsx wires into favorites using useFavorites and performs initial favorites syncing—so they overlap at the code level in favorites fetching/synchronization logic.

Poem

🐰 A hop, a skip, favorites sync!
No more effect chains, no more link—
New hook takes the wheel with grace,
Reconciles state at rapid pace.
Optimistic updates roll just right,
Refactored bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'fix(favorites): keep favorites in sync across devices' directly reflects the main objective: fixing stale favorites state by removing persisted isSynced and adding revalidation logic across devices. It is concise, specific, and accurately summarizes the core fix.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pu/favorites

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8be3ccac1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/web/src/hooks/useFavorites.ts
Not persisting `isSynced` only stops future writes; users upgrading from a
build that persisted it still have `isSynced: true` in localStorage, which
Zustand's default merge restores on rehydrate — skipping the load-time
revalidation for exactly the stale-cache population this PR targets. Add a
custom persist `merge` that always resets `isSynced` to false.

Addresses review feedback on useFavorites.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@2witstudios
2witstudios merged commit 92ad2e8 into master May 31, 2026
5 checks passed

This branch was previously deployed

1 inactive deployment
Preview — be4370e5 Deployed May 31, 2026 by vercel[bot]
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.

1 participant