fix: stop dashboard refresh loops#1621
Conversation
📝 WalkthroughWalkthroughService worker cache-first strategy replaced with network-first for built assets (under ChangesBlank Page Prevention via Asset Revalidation and Reload Suppression
🎯 3 (Moderate) | ⏱️ ~22 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dashboard/app/__tests__/pwa.test.ts (1)
121-124: ⚡ Quick winCover the
fontdestination in this built-asset contract test.The production branch includes
request.destination === "font", but this regression guard currently validates only script/style.Suggested patch
expect(swSource).toContain('request.destination === "script"'); expect(swSource).toContain('request.destination === "style"'); + expect(swSource).toContain('request.destination === "font"'); expect(swSource).toContain('if (isBuiltAssetRequest) {');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/__tests__/pwa.test.ts` around lines 121 - 124, The test in packages/dashboard/app/__tests__/pwa.test.ts is asserting the built-asset contract against swSource but only checks for script/style destinations; update the assertions to also require the font destination by adding an expectation that swSource contains the string 'request.destination === "font"'. Keep the other checks (e.g., 'url.pathname.startsWith("/assets/")' and 'if (isBuiltAssetRequest) {') unchanged so the test still validates the built-asset request handling logic in the service worker.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/dashboard/app/__tests__/pwa.test.ts`:
- Around line 121-124: The test in packages/dashboard/app/__tests__/pwa.test.ts
is asserting the built-asset contract against swSource but only checks for
script/style destinations; update the assertions to also require the font
destination by adding an expectation that swSource contains the string
'request.destination === "font"'. Keep the other checks (e.g.,
'url.pathname.startsWith("/assets/")' and 'if (isBuiltAssetRequest) {')
unchanged so the test still validates the built-asset request handling logic in
the service worker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac41d34d-5589-4b38-ad9f-d79bc0305189
📒 Files selected for processing (5)
.changeset/fn-blank-page-service-worker-assets.mdpackages/dashboard/app/__tests__/pwa.test.tspackages/dashboard/app/__tests__/versionCheck.test.tspackages/dashboard/app/public/sw.jspackages/dashboard/app/versionCheck.ts
Greptile SummaryThis PR addresses two related problems: a blank-page / reload-loop scenario when returning to a dashboard tab after a rebuild. It adds a network-first caching strategy in the service worker for built assets and introduces a per-tab sessionStorage flag that remembers which remote build version already triggered an auto-reload, suppressing further reloads for that same version.
Confidence Score: 3/5Safe to merge after fixing the changeset file — the SW and versionCheck logic is correct, but the malformed changeset will silently drop this fix from the next release changelog and version bump. The service-worker and version-check changes are well-reasoned and well-tested. The one hard blocker is the changeset file: it is missing the
Important Files Changed
Sequence DiagramsequenceDiagram
participant Tab as Browser Tab
participant VC as versionCheck.ts
participant SS as sessionStorage
participant SW as sw.js (network-first)
participant Server as Server
Tab->>VC: checkVersion("initial")
VC->>Server: fetch /version.json
Server-->>VC: "remote="v2" ≠ local="v1""
VC->>VC: mismatch-PENDING
Note over Tab: 60s+ passes, user returns to tab
Tab->>VC: checkVersion("focus")
VC->>Server: fetch /version.json
Server-->>VC: "remote="v2" (same)"
VC->>VC: mismatch-CONFIRMED
VC->>SS: "set fusion:version-reloaded-remote="v2""
VC->>Tab: window.location.reload()
Note over Tab: Page reloads — SW serves fresh assets
Tab->>SW: fetch /assets/app-xyz.js
SW->>Server: fetch (network-first)
Server-->>SW: fresh bundle
SW-->>Tab: fresh JS
Tab->>VC: checkVersion("initial")
VC->>Server: fetch /version.json
Server-->>VC: "remote="v2" = local="v2""
VC->>SS: clearReloadedRemoteVersion()
Note over Tab: User switches away and returns (stale bundle scenario)
Tab->>VC: checkVersion("focus")
VC->>VC: mismatch-CONFIRMED
VC->>SS: "getReloadedRemoteVersion() === "v2"?"
SS-->>VC: yes
VC->>VC: emit reload-suppressed, return
Reviews (1): Last reviewed commit: "fix: stop dashboard refresh loops" | Re-trigger Greptile |
| "@runfusion/fusion": patch | ||
|
|
||
| Revalidate dashboard service-worker assets before falling back to cache so rebuilt tabs cannot stay on stale bundles and render a blank page. |
There was a problem hiding this comment.
Missing
--- frontmatter fences — changeset will be silently ignored
The changeset file is missing the required --- delimiters. Without them the @changesets/cli parser does not recognize this file as a valid changeset, so pnpm release will not include this entry in the version bump or CHANGELOG.md. Every other changeset in the repo (e.g., fix-frozen-dashboard-animations.md) uses the three-line frontmatter pattern.
| if (autoReloadEnabled) { | ||
| setReloadedRemoteVersion(remote); | ||
| } | ||
| reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`); |
There was a problem hiding this comment.
setReloadedRemoteVersion is stored before reloadOnce is called. If reloadOnce is suppressed because RELOAD_FLAG is already in sessionStorage, the remote version gets permanently marked as "already reloaded" even though no version-mismatch reload occurred — silently blocking future reload attempts for that version. Moving the call after the flag check in reloadOnce (or reading RELOAD_FLAG state here first) would ensure the marker is only set when a reload actually proceeds.
| if (autoReloadEnabled) { | |
| setReloadedRemoteVersion(remote); | |
| } | |
| reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`); | |
| const alreadyAttempted = Boolean((() => { try { return sessionStorage.getItem(RELOAD_FLAG); } catch { return null; } })()); | |
| if (autoReloadEnabled && !alreadyAttempted) { | |
| setReloadedRemoteVersion(remote); | |
| } | |
| reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`); |
Summary
Returning to an existing dashboard tab no longer risks a blank page or a repeated refresh loop after a rebuild. Built assets now revalidate against the running server before falling back to service-worker cache, so stale JS/CSS cannot keep an old app shell alive.
The version checker also remembers which remote build it already auto-reloaded for. If focus or visibility events keep seeing the same remote build from an old bundle, Fusion suppresses repeat reloads instead of refreshing every time the browser regains focus.
Verification
pnpm --filter @fusion/dashboard exec vitest run --project dashboard-app app/__tests__/versionCheck.test.ts app/__tests__/pwa.test.tspnpm --filter @fusion/dashboard buildSummary by CodeRabbit
New Features
Bug Fixes