Fix offline access render loop and restore browser integration checks - #271
Merged
Merged
Conversation
6 tasks
|
🚅 Deployed to the boop-pr-271 environment in Friends
|
brianorwhatever
marked this pull request as ready for review
October 4, 2026 22:17
6 tasks
This was referenced Oct 4, 2026
There was a problem hiding this comment.
ℹ️ No critical issues. One test assertion could be tighter.
Reviewed changes
I reviewed the fix for the OfflineAccessMonitor render loop, the new real-hook regression test, and the browser fixture and copy updates that bring the restored e2e suite back in line with main.
- Stable
useQueriesrequests: all threeuseQueriescall sites now memoize their request object, including the signed-out{}. Before, Convex'suseSubscriptioncalledsetStateduring render on every new object identity, which looped forever. I confirmed that the memo deps on parent-supplied batch arrays still change on each parent render. That costs one extra render per change but cannot loop, andQueriesObserver.setQueriescompares args by JSON, so no subscriptions churn. - Regression test with real Convex hooks: I checked
scripts/offline-monitor-subscription.test.mjsboth ways. Withmain'sOfflineAccessMonitor.tsxit fails with "Too many re-renders". On this branch it passes under bothnode --testandbun test, alongside the existingoffline-access-monitor.test.mjs. - Browser fixture queries: the
getOfflineAccess/getOfflineDraftAccessresponses match the shapes inconvex/items.ts(presentItemIds,missingItemIds,canEdit,checkedAt), andgetOfflineAccountnow returnsdid. - e2e copy: the new strings ("Publish publicly", "Share list", "Unpublish", "This list is published publicly",
og-image-boop.png) match the currentShareModal.tsxandindex.html.
claude-opus-5-5 | 𝕏
There was a problem hiding this comment.
✅ No new issues found. The earlier test-assertion feedback is addressed.
Reviewed changes
I reviewed the two commits pushed since the prior review at 36eb544.
- Tightened the account-switch assertion: the regression test now asserts
active.size === 4after the account switch, so it can no longer pass vacuously. It passes locally undernode --test. - Updated
tests/sharing.e2e.tscopy: the new strings ("Share list", "Publish publicly", "This legacy invite link is no longer supported") match the current<h2>and button text inShareModal.tsxandJoinList.tsx.
claude-opus-5-5 | 𝕏
6 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Authenticated routes could render a blank page after the private-revocation changes: OfflineAccessMonitor passed a new request object to Convex useQueries on every render, triggering an infinite React render loop. Memoize each subscription request, including signed-out requests, and keep default item locators stable.
Add a regression using the real Convex React hooks covering sign-in, list/item/draft batches, revocation delivery, token renewal, account switching and sign-out. It fails on main with “Too many re-renders” and passes with this fix. Update the browser fixture for current access/profile APIs and the existing publication/branding copy so the restored suite can exercise current main.
Related to #260; this fixes a regression, not the outstanding legacy URL cutover, ambiguous offline-work recovery, or live/native verification criteria.
Validation: latest commit dfd0c42 passes CI unit tests, all 116 browser tests, web build, Android build, unsigned iOS build check, Lighthouse and automated review. Focused monitor suites, frontend/backend/E2E TypeScript and changed-source lint passed locally. The real-hook regression fails before the production fix and passes after it. Independent delegated review findings were fixed and follow-up review found no remaining issues. A broad local rerun hit disk exhaustion; complete CI suites passed. GitHub reports no merge conflict. Live/native cutover behavior remains outside this regression fix.
Note
Fix offline access render loop by memoizing Convex query configs in
OfflineAccessMonitoruseQueriesin OfflineAccessMonitor.tsx. Each config rebuilds only when the auth token or relevant inputs change, and clears to an empty config while signed out. This stops a render loop from unstable request objects.Macroscope summarized 36eb544.