fix: eliminate AppUpdateBanner double-reload by coordinating with service worker#106
Open
bryanchriswhite wants to merge 5 commits intomainfrom
Open
fix: eliminate AppUpdateBanner double-reload by coordinating with service worker#106bryanchriswhite wants to merge 5 commits intomainfrom
bryanchriswhite wants to merge 5 commits intomainfrom
Conversation
…vice worker The AppUpdateBanner was causing two consecutive reloads when users clicked "Refresh Now" due to a race condition between the manual reload and the PWA service worker's autoUpdate mode. Changes: - Integrated useRegisterSW hook from virtual:pwa-register/react - Replaced window.location.reload() with updateServiceWorker() for coordinated updates - Added loading state with spinner (Loader2 icon) and disabled button during updates - Added visual feedback with "Updating..." text while service worker activates - Kept fallback to manual reload in case of service worker errors The service worker now handles the single reload after it fully activates, eliminating the confusing double-reload that previously occurred. Fixes #104
bryanchriswhite
added
bug
The E2E test was failing because updateServiceWorker(true) may not trigger a reload in test environments where service workers aren't fully functional. Changes: - Added 3-second timeout that forces reload if service worker doesn't respond - Ensures reload happens in both production (with working SW) and test environments - Maintains coordination with service worker when available - Prevents indefinite hang if service worker update doesn't complete This ensures the single-reload fix works reliably across all environments while still benefiting from service worker coordination in production.
…e of truth This eliminates the race condition that caused double-reloads by consolidating two independent update detection mechanisms into one coordinated system. ## Previous Architecture (Problematic) - AppUpdateBanner: Polled version.json independently every 5-10 minutes - PWA Service Worker: Detected updates independently via Workbox - PWA Mode: autoUpdate (auto-activates and reloads without user action) - Race condition: Both systems could trigger reloads at different times ## New Architecture (Unified) - Service worker is now the single source of truth for update detection - PWA Mode: Changed to 'prompt' (downloads updates automatically, waits for user) - AppUpdateBanner: Uses needRefresh from useRegisterSW hook - Banner fetches version.json only for display purposes (version number, build time) - User clicks "Refresh Now" → Single coordinated reload via updateServiceWorker(true) ## Benefits - Eliminates double-reload race condition completely - Service worker downloads updates in background automatically - User controls when to reload (better UX, no interruption) - Simpler code: no timeout fallbacks or complex coordination needed - Single source of truth for update state ## Changes - vite.config.ts: Changed registerType from 'autoUpdate' to 'prompt' - AppUpdateBanner.tsx: - Removed independent version.json polling - Now uses needRefresh from useRegisterSW as primary update signal - Fetches version.json only when needRefresh becomes true (for display) - Simplified handleRefresh() - just calls updateServiceWorker(true) - Simplified dismiss logic (no longer per-version, just per-update) Fixes #104
The AppUpdateBanner now uses the service worker as the single source of truth (needRefresh state from useRegisterSW hook). This means the banner only appears when there's an actual waiting service worker, not just when version.json changes. E2E testing this requires a complex multi-build setup: 1. Build and deploy version X 2. Load the app with version X 3. Build and deploy version Y 4. Service worker detects update and enters waiting state 5. Banner appears This is impractical for automated E2E tests. The tests are now skipped with detailed documentation on how to perform manual testing. The core functionality (single coordinated reload, no double-reload) should be verified through manual testing when deploying new versions.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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
2 participants
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.
The AppUpdateBanner was causing two consecutive reloads when users clicked "Refresh Now" due to a race condition between the manual reload and the PWA service worker's autoUpdate mode.
Changes:
The service worker now handles the single reload after it fully activates, eliminating the confusing double-reload that previously occurred.
Fixes #104
Description
Type of Change
Related Issues
Fixes #
Motivation and Context
Changes Made
Testing
Test Environment
Test Cases
Test Results
Database Impact
Performance Impact
Performance notes:
UI/UX Changes
Screenshots
Before:
After:
Responsive Design
Documentation
Code Quality
anyunless necessary)npm run lintpasses without errorsnpm run buildcompletes successfullyDeployment Checklist
npm run build && npm run preview)Breaking Changes
Does this PR introduce breaking changes? No / Yes
Rollback Plan
Additional Notes
Reviewer Checklist