Feature: lazy load reload - #297
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughAdds a React/webpack chunk-load recovery utility with per-build reload protection, global error handling, Sentry filtering, Jest coverage, a Webpack entry, and a package version update. ChangesChunk Error Recovery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReactLazy
participant lazyWithReload
participant sessionStorage
participant Window
participant Sentry
ReactLazy->>lazyWithReload: import module
lazyWithReload->>sessionStorage: check build reload state
lazyWithReload->>Window: reload on matching chunk error
Window->>lazyWithReload: repeated failure after reload
lazyWithReload->>Sentry: report recovery failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@src/utils/lazy-with-reload.js`:
- Around line 40-55: Update isHtmlParsedAsScriptError to recognize Firefox’s
SyntaxError wording, “expected expression, got '<',” in addition to the existing
“Unexpected token '<'” check. Preserve the existing SyntaxError detection and
filename pattern guard, while allowing either characteristic message to trigger
the stale-chunk fallback.
- Around line 124-143: Update reloadOnChunkError to track, in memory, whether a
reload has already been triggered during the current page lifecycle or tick
before consulting persisted reload state. Have subsequent concurrent chunk
failures return without calling reportRecoveryFailed, while preserving the
existing persisted-state check for failures occurring after navigation and the
current write/sessionStorage/reload flow for the first failure.
- Around line 210-224: Update chunkErrorSentryBeforeSend to extract the relevant
filename from each Sentry exception’s stack frames and pass it with the
exception type and value to isChunkLoadError. Preserve the existing filtering
behavior for genuine chunk-load errors while ensuring the HTML-as-script
SyntaxError path requires a filename matching chunkFilenamePattern.
- Around line 61-82: Compute and cache the build fingerprint eagerly during
module evaluation, before any lazy chunks can load, instead of initializing
cachedBuildFingerprint inside getBuildFingerprint. Keep getBuildFingerprint as a
read-only accessor and preserve the existing document-scripts fingerprinting and
non-browser fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5f6667e-34d6-40cb-a728-10d5b64ff9b6
📒 Files selected for processing (4)
package.jsonsrc/utils/__tests__/lazy-with-reload.test.jssrc/utils/lazy-with-reload.jswebpack.common.js
| if (!looksLikeSyntaxError || !/Unexpected token '<'/i.test(message)) { | ||
| return false; | ||
| } | ||
| return !filename || chunkFilenamePattern.test(filename); |
There was a problem hiding this comment.
@santipalenque The filename-less branch of the SyntaxError heuristic turns unrelated JSON.parse failures into page reloads and hides them from Sentry.
return !filename || chunkFilenamePattern.test(filename) matches any SyntaxError whose message contains "Unexpected token '<'" when no filename is available — and the two call sites that pass no filename are exactly the ones where unrelated errors arrive. Verified concrete scenario (V8/Chromium): new Response('<html>502</html>').json() rejects with SyntaxError: Unexpected token '<', "<html>502</html>" is not valid JSON. If that rejection goes unhandled (a fetch of an API that returned an HTML 502/proxy/auth page), onUnhandledRejection (line 189) passes it here with filename === undefined, and the user's page does a full reload mid-session — once per build, with the next occurrence filing a false "Chunk load error persisted after auto-reload" report. The same filename-less call in chunkErrorSentryBeforeSend (line 218) permanently drops this whole class of errors from Sentry monitoring.
Tightening this loses no genuine webpack case: webpack's JSONP runtime sets error.name = "ChunkLoadError" on every chunk-load failure it detects through the import() promise, including the "script responded 200 with HTML but the chunk never registered" case (webpack/lib/web/JsonpChunkLoadingRuntimeModule.js, loadingEnded handler, errorType 'missing') — so the ChunkLoadError path already covers the stale-chunk failure this branch is trying to catch.
Suggested fix:
- In
onUnhandledRejection, match onlyChunkLoadError(import() rejections are never raw SyntaxErrors). - In
isHtmlParsedAsScriptError, require a filename that matcheschunkFilenamePattern(drop the!filename ||escape) — the genuine HTML-as-script error arrives via the globalerrorevent withevent.filenameset. - In
chunkErrorSentryBeforeSend, extract the culprit filename from the exception's stack frames and pass it through (as the existing CodeRabbit thread on the beforeSend hook suggests). - Update the test "matches a SyntaxError for 'Unexpected token <' with no filename" (lazy-with-reload.test.js line 57), which currently pins the permissive behavior, and add a regression test asserting a JSON.parse-style SyntaxError does NOT trigger a reload via unhandledrejection.
There was a problem hiding this comment.
Pull request overview
Adds a reusable chunk-load recovery utility to the UI core library so applications can automatically recover from stale/unavailable lazy-loaded webpack chunks by triggering a guarded page reload, with reporting for non-recoverable cases.
Changes:
- Added
lazyWithReload,initChunkErrorRecovery, and Sentry filtering helpers to detect webpack chunk-load failures and recover via a one-time-per-build reload. - Added Jest coverage for detection, reload-guard behavior, global error/unhandledrejection handling, and Sentry filtering behavior.
- Exposed the new utility via the webpack build entrypoints and bumped the package version to a beta release.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| webpack.common.js | Exposes the new utils/lazy-with-reload entry so consumers can import it from the built package. |
| src/utils/lazy-with-reload.js | Implements chunk-load error detection, guarded reload recovery, and Sentry beforeSend filtering helper. |
| src/utils/tests/lazy-with-reload.test.js | Adds unit tests covering detection and recovery behavior. |
| package.json | Updates package version to 5.0.45-beta.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@santipalenque please review
https://app.clickup.com/t/9014802374/86bb1gp6z
Summary by CodeRabbit
New Features
Tests
Chores
5.0.45-beta.0.