Bug 2044976 - Fix react suspense lazy load on deploy#9633
Open
camd wants to merge 1 commit into
Open
Conversation
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.
Bug 2044976 - Recover from stale lazy-loaded chunks instead of spinning forever
The log viewer (and occasionally other views) would sometimes never load for a user — an infinite loading spinner — until they cleared their cache or did a hard refresh. A normal reload did not fix it.
Root cause
Each top-level app is code-split via
lazy(() => import(...))inui/App.jsx, wrapped only in<Suspense>. This helps with load times immensely when someone loads/jobsthey don't pay the import price for perfherder, and vise-versa.Note
Chunkshere are chunks of Treeherder deployed code, not test-chunking of jobs/tasks.<Suspense>handles the pending state of a dynamic import, but it does not catch a rejected one. With no error boundary above it, a chunk that fails to load left React with nothing to catch — and the user stuck on the spinner.A chunk load fails in two ways that both match the report:
clean: true). Treeherder is an SPA people keep open for hours, so a tab still running the pre-deploy runtime requests an old chunk URL that no longer exists →ChunkLoadError. The app shell HTML isno-store(verified in prod), so a reload always pulls current assets — but nothing was triggering that reload.immutablecached asset. Hashed assets are servedCache-Control: …immutable, so the browser never revalidates them on a normal reload — it keeps serving a bad cached copy. Only a cache-bypassing load replaces it. This matches Henrik's and Julian's description: reload doesn't help, "Disable Cache" fixes it permanently, intermittent, single-profile only.Note
Right now I believe that "always Wd3" is incidental: that log content renders through the
/logviewerlazy route, so that's the chunk he kept hitting. My suspicion was that it was one or more tabs that hadn't yet been reloaded after a deploy. After this fix is deployed, we can reassess if there is another issue.Fix
Add a
ChunkErrorBoundaryaround the lazy routes:immutable-cache case); instead it shows a readable message prompting a hard refresh.Caching behavior is intentionally unchanged —
immutablehashing is correct and stays. This change only ensures a failed chunk load becomes recoverable rather than a permanent stuck spinner.Notes / possible follow-ups
This is a reactive fix (recovers after a failure). A proactive "a new version is available — reload" banner (poll a version hash, offer a manual reload) could further reduce occurrences; intentionally left out of scope here. But we may keep that as a future option.