Skip to content

refactor: apply review feedback for bookmarks broadcast implementation#3128

Draft
Copilot wants to merge 7 commits intoQF-4694-bookmarks-cross-tabsfrom
copilot/sub-pr-3082
Draft

refactor: apply review feedback for bookmarks broadcast implementation#3128
Copilot wants to merge 7 commits intoQF-4694-bookmarks-cross-tabsfrom
copilot/sub-pr-3082

Conversation

Copy link

Copilot AI commented Feb 15, 2026

Summary

Addresses performance and code quality issues identified in PR #3082 review feedback.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • ♻️ Refactoring (no functional changes)

If Breaking Change

N/A

Scope Confirmation

  • This PR addresses one feature/fix only
  • If multiple changes were needed, they are split into separate PRs

Environment Variables

N/A

Rollback Safety

  • Can be safely reverted without data issues or migrations
  • Rollback requires special steps (describe below):

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed

Testing steps:

  1. Existing unit tests pass with corrected types and behavior
  2. Verified broadcast functionality works with optimized code paths

Edge Cases Verified

  • ⏳ Loading state handled
  • ❌ Error state handled
  • 📭 Empty state handled
  • 👤 Logged-in vs guest behavior (if applicable)

Pre-Review Checklist

Code Quality

  • I have performed a self-review of my code (file by file)
  • My code follows the project style guidelines
  • No any types used (or justified if unavoidable)
  • No unused code, imports, or dead code included
  • Complex logic has inline comments explaining "why"
  • Functions are under 30 lines and follow single responsibility

Testing & Validation

  • All tests pass locally (yarn test)
  • Linting passes (yarn lint)
  • Build succeeds (yarn build)
  • Edge cases and error scenarios are handled

Documentation

  • Code is self-documenting with clear naming
  • README updated (if adding features or setup changes)
  • Inline comments added for complex logic

Localization (if UI changes)

N/A

Accessibility (if UI changes)

N/A

Screenshots/Videos

N/A

Related PRs

Reviewer Notes

All changes are surgical and minimal, addressing only the specific review feedback. No functional changes to behavior.

Changes:

  1. Eliminated duplication - Merged broadcastCollectionCreated and broadcastCollectionUpdated into single broadcastCollectionUpserted helper (identical payloads)

  2. Performance: cache key computation - Compute keys once in applyPayloadSync, pass to all helpers instead of recomputing per function:

    const applyPayloadSync = ({ cache, globalMutate, payload }) => {
      const keys = getStringCacheKeys(cache); // Once per message
      
      if (payload.touchesCollectionsList) {
        revalidateCollectionsListCaches(keys, globalMutate); // Reuse
      }
      // ... other handlers
    };
  3. Performance: surah updates - Create existingKeys Set once outside forEach in applySurahBookmarkUpdates instead of rebuilding per entry

  4. Fix: duplicate delivery - postWithStorageFallback now uses early return after successful BroadcastChannel post; localStorage only used as fallback to prevent duplicate cross-tab messages

  5. Performance: bulk operations - Convert deletedIds to Set before filtering (O(n) vs O(n*m)):

    const deletedIdsSet = new Set(deletedIds);
    bookmarks.filter(b => deletedIdsSet.has(b.id))
  6. Fix: test types - Use numeric bookmark keys 1 instead of string '1' to match production types

  7. Fix: test construction - Use Object.assign instead of StorageEvent constructor arguments per browser API conventions

AI Assistance Disclosure

  • AI tools were NOT used for this PR
  • AI tools were used, and I have thoroughly reviewed and validated all generated code

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 6 commits February 15, 2026 22:06
Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
…mance

Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
Co-authored-by: rrrokhtar <39674365+rrrokhtar@users.noreply.github.com>
Copilot AI changed the title [WIP] Add bookmarks broadcast functionality across components refactor: apply review feedback for bookmarks broadcast implementation Feb 15, 2026
Copilot AI requested a review from rrrokhtar February 15, 2026 22:10
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.

2 participants