UN-3185: Defer heavy chunks on login route + cache fingerprinted assets#2114
Merged
Conversation
Fix 1 (nginx.conf): serve content-hashed /assets/* with a 1y immutable Cache-Control while keeping index.html and config/runtime-config.js non-cached, so repeat visits stop re-fetching the whole bundle. Fix 2: stop the unauthenticated /landing page from eagerly downloading the full app. Convert OSS route pages and enterprise plugin route elements to React.lazy behind a single <Suspense>, and lazy-load the app shell (PageLayout/FullPageLayout) which transitively pulled the PDF viewer, charts and lookup-studio onto /landing. New helper src/helpers/pluginRegistry.js (lazyPlugin) defers plugin chunks to navigation; OSS builds resolve the stub and fall back to NotFound, so absent-plugin routes 404 harmlessly as before. Measured /landing script requests: 201 -> 93; pdf-vendor, recharts and Monaco no longer load until navigated to. OSS-parity build (no src/plugins) verified.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe frontend now lazy-loads selected routes and plugin-backed pages through shared helpers and suspense/error boundaries, while nginx maps cache headers by URI and serves hashed assets with long-lived caching. ChangesFrontend lazy loading and cache policy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Contributor
|
| Filename | Overview |
|---|---|
| frontend/nginx.conf | Adds map $uri $cache_control at http scope and a server-level add_header Cache-Control so immutable caching for /assets/* composes cleanly with existing security headers instead of overriding them; dedicated /assets/ location block returns hard 404 for missing files. |
| frontend/src/helpers/pluginRegistry.js | New lazyPlugin helper wraps enterprise plugin imports as React.lazy components; correctly maps only the 'Optional plugin not available' stub error to NotFound and re-throws all others, but m[exportName] ?? m.default silently falls back to the default export when a named export is absent. |
| frontend/src/components/error/LazyOutlet/LazyOutlet.jsx | New component that scopes Suspense+ErrorBoundary to the content area; auto-reload-once logic for stale-chunk failures uses a 10-second sessionStorage guard to prevent reload loops; navigation resets the boundary via resetKeys. |
| frontend/src/routes/Router.jsx | Wraps routes in a top-level ErrorBoundary+Suspense; converts eagerly-imported pages and plugin components to lazy equivalents; guarded await imports for hook-returning route plugins remain with improved error logging. |
| frontend/src/routes/useMainAppRoutes.js | Replaces all eager imports with lazyNamed/lazyPlugin; layouts are now lazy-loaded so the SideNavBar/lookup-studio/PDF graph is no longer pulled onto /landing; PRODUCT_NAMES guard refined to check the specific unstract key. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Browser navigates to /landing"] --> B["nginx serves index.html\n(Cache-Control: no-cache)"]
B --> C["React app boots\nEager: LandingPage only"]
C --> D{"Authenticated?"}
D -- No --> E["Render LandingPage\nNo heavy chunks fetched"]
D -- Yes --> F["Navigate to authenticated route"]
F --> G["Outer Suspense+ErrorBoundary\nin Router.jsx"]
G --> H{"Layout type?"}
H -- PageLayout / FullPageLayout --> I["lazyNamed loads layout chunk\n/assets/PageLayout-hash.js\n(Cache-Control: immutable)"]
I --> J["Layout renders LazyOutlet\n(inner Suspense+ErrorBoundary)"]
J --> K["lazyNamed / lazyPlugin\nloads page chunk on navigation"]
K --> L["Page renders in content area\nShell stays mounted"]
H -- No layout route --> M["lazyPlugin loads plugin chunk"]
M --> N{"Plugin absent in OSS?"}
N -- Yes --> O["Stub throws sentinel\nlazyPlugin returns NotFound"]
N -- No --> P["Component renders normally"]
K -- "Chunk load error" --> Q["Inner ErrorBoundary catches\nhandleRouteError: auto-reload once\n(sessionStorage guard)"]
Q -- "Reloaded" --> R["Fresh index.html\nNew content-hashed chunk URLs"]
Q -- "Within 10s window" --> S["Show RouteLoadError\n'Reload' button"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["Browser navigates to /landing"] --> B["nginx serves index.html\n(Cache-Control: no-cache)"]
B --> C["React app boots\nEager: LandingPage only"]
C --> D{"Authenticated?"}
D -- No --> E["Render LandingPage\nNo heavy chunks fetched"]
D -- Yes --> F["Navigate to authenticated route"]
F --> G["Outer Suspense+ErrorBoundary\nin Router.jsx"]
G --> H{"Layout type?"}
H -- PageLayout / FullPageLayout --> I["lazyNamed loads layout chunk\n/assets/PageLayout-hash.js\n(Cache-Control: immutable)"]
I --> J["Layout renders LazyOutlet\n(inner Suspense+ErrorBoundary)"]
J --> K["lazyNamed / lazyPlugin\nloads page chunk on navigation"]
K --> L["Page renders in content area\nShell stays mounted"]
H -- No layout route --> M["lazyPlugin loads plugin chunk"]
M --> N{"Plugin absent in OSS?"}
N -- Yes --> O["Stub throws sentinel\nlazyPlugin returns NotFound"]
N -- No --> P["Component renders normally"]
K -- "Chunk load error" --> Q["Inner ErrorBoundary catches\nhandleRouteError: auto-reload once\n(sessionStorage guard)"]
Q -- "Reloaded" --> R["Fresh index.html\nNew content-hashed chunk URLs"]
Q -- "Within 10s window" --> S["Show RouteLoadError\n'Reload' button"]
Reviews (6): Last reviewed commit: "UN-3185: Auto-reload once on chunk-load ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/routes/useMainAppRoutes.js (1)
351-356: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the product wrapper on the value actually used.
Object.keys(PRODUCT_NAMES).lengthcan be true even whenPRODUCT_NAMES.unstractis absent, but Line 355 passes that value astype. Use theunstractentry itself as the condition to avoid wrapping all app routes with an undefined product type.Proposed fix
- if (OnboardProduct && Object.keys(PRODUCT_NAMES)?.length) { + const unstractProduct = PRODUCT_NAMES?.unstract; + + if (OnboardProduct && unstractProduct) { return ( <Route path="" - element={<OnboardProduct type={PRODUCT_NAMES?.unstract} />} + element={<OnboardProduct type={unstractProduct} />} >🤖 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 `@frontend/src/routes/useMainAppRoutes.js` around lines 351 - 356, The product wrapper guard in useMainAppRoutes is checking Object.keys(PRODUCT_NAMES).length, but the route element actually depends on PRODUCT_NAMES.unstract; update the condition in the OnboardProduct/Route block to gate on the same unstract value that is passed as type, so the wrapper is only rendered when that product type exists.
🤖 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 `@frontend/nginx.conf`:
- Around line 61-68: The location blocks that use add_header are overriding the
server-level security headers, so explicitly re-add the security headers in each
affected location stanza. Update the nginx.conf location handlers for /assets/,
/index.html, and /config/runtime-config.js so they preserve the same
X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and
Content-Security-Policy-Report-Only values defined at the server level, while
keeping the existing cache-related headers in place.
In `@frontend/src/helpers/pluginRegistry.js`:
- Around line 26-28: Update the lazy import error handling in pluginRegistry.js
so chunk fetch failures are not treated as missing plugins. In lazyPlugin, the
catch path currently relies on isModuleMissing(err) and returns NotFound for
both the build-time stub and transient dynamic import/network failures; tighten
isModuleMissing to only match the optional-plugin stub error or change the
lazyPlugin catch logic to only map that specific case to NotFound and rethrow
all other errors.
In `@frontend/src/routes/Router.jsx`:
- Around line 113-127: The top-level dynamic imports in Router.jsx are still
running during module evaluation, which keeps useLlmWhispererRoutes and
useVerticalsRoutes on the startup critical path. Move these imports out of the
Router module scope and into lazy-loaded route/component wrappers so the plugins
load only when their routes are actually needed. Use the existing symbols
llmWhispererRouter, verticalsRouter, useLlmWhispererRoutes, and
useVerticalsRoutes to relocate the loading logic without blocking /landing
rendering.
---
Outside diff comments:
In `@frontend/src/routes/useMainAppRoutes.js`:
- Around line 351-356: The product wrapper guard in useMainAppRoutes is checking
Object.keys(PRODUCT_NAMES).length, but the route element actually depends on
PRODUCT_NAMES.unstract; update the condition in the OnboardProduct/Route block
to gate on the same unstract value that is passed as type, so the wrapper is
only rendered when that product type exists.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92f99d45-c009-4324-8547-15d11fbbaa70
📒 Files selected for processing (4)
frontend/nginx.conffrontend/src/helpers/pluginRegistry.jsfrontend/src/routes/Router.jsxfrontend/src/routes/useMainAppRoutes.js
…bsent check
- nginx: drive Cache-Control from a $uri map at server scope instead of
per-location add_header. Location-level add_header replaces (not merges)
the inherited server headers, which dropped X-Content-Type-Options/
X-Frame-Options/Referrer-Policy/CSP from /assets, index.html and
runtime-config.js. Now both locations carry no add_header and inherit all
security + cache headers.
- pluginRegistry: only treat the build-time stub ('Optional plugin not
available') as plugin-absent; rethrow transient chunk-load failures of a
shipped plugin instead of masking them as NotFound.
- useMainAppRoutes: gate the OnboardProduct wrapper on PRODUCT_NAMES.unstract
(the value passed as type) rather than the map being non-empty.
Contributor
Author
|
Thanks for the reviews 🙏 Addressed in
Build green, biome clean. One caveat: I couldn't run |
Pull the repeated 'lazy a named export' pattern into src/helpers/lazyNamed.js
and use it in Router.jsx and useMainAppRoutes.js instead of inline
.then((m) => ({ default: m.X })) / a per-file 'named' helper. Behaviour is
identical; /landing chunk count unchanged. Addresses cloud-PR review feedback
about the duplicated helper (the two route hooks import the same util).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/helpers/lazyNamed.js`:
- Around line 11-12: The lazyNamed helper currently maps any requested export to
{ default: m[exportName] }, which can silently return undefined and fail later
in React.lazy. Update lazyNamed(loader, exportName) to explicitly validate that
the loaded module contains exportName before resolving, and throw a descriptive
error during the loader promise chain when it is missing. Keep the fix localized
to lazyNamed and ensure the error makes it clear which missing named export
caused the load failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70d9876e-a614-4c86-9aa0-e5dbd86bb5de
📒 Files selected for processing (3)
frontend/src/helpers/lazyNamed.jsfrontend/src/routes/Router.jsxfrontend/src/routes/useMainAppRoutes.js
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/routes/useMainAppRoutes.js
- frontend/src/routes/Router.jsx
- Wrap the route <Suspense> in Router.jsx with the existing ErrorBoundary and
a reload-prompt fallback. lazyPlugin/lazyNamed rethrow non-stub failures
(e.g. a transient chunk-load blip), and App rendered <Router/> with no
boundary — so such a failure would unmount the tree to a blank screen.
The boundary now contains it and offers a reload (which re-fetches the chunk).
- lazyNamed: throw a descriptive error when the requested named export is
missing instead of handing React.lazy { default: undefined } (opaque error).
Addresses greptile P1 (no ErrorBoundary) and CodeRabbit (lazyNamed validation).
jaseemjaskp
requested review from
Deepak-Kesavan,
tahierhussain and
vishnuszipstack
jaseemjaskp
commented
Jun 25, 2026
jaseemjaskp
left a comment
jaseemjaskp
left a comment
Contributor
Author
There was a problem hiding this comment.
Automated follow-up review (PR Review Toolkit). I raised the bar to only correctness/contract/resilience issues that weren't already covered in earlier rounds. Three findings below; the rest (comment-accuracy nit on the FullPageLayout rationale, nginx no-cache wording, missing helper unit tests, a behavior-preserving dedup of the guarded-await import blocks) are minor and left out of inline comments.
… dead guard
Review round 3:
- lazyPlugin: validate the resolved export (mirror lazyNamed). A shipped plugin
whose named export was renamed/removed now throws a descriptive error instead
of handing React.lazy { default: undefined }; isPluginAbsent doesn't match it,
so it re-throws to the ErrorBoundary rather than masking as NotFound.
- ErrorBoundary: support resetKeys — clear the error when a key changes (e.g.
location), so navigation recovers without a full reload.
- New LazyOutlet (content-scoped <Suspense> + nav-resettable ErrorBoundary +
<Outlet/>); PageLayout/FullPageLayout use it instead of a bare <Outlet/>.
Per-page load spinners and chunk-load failures now stay in the content area
with the shell mounted; the app-wide boundary in Router.jsx remains the
backstop and is now also location-reset. Fixes the blast-radius / no-recovery
and shell-blanks-on-first-nav issues from the single top-level boundary.
- useMainAppRoutes: remove the now-dead 'ReadOnlyReviewPage && !ReviewLayout'
warning — with lazyPlugin both are always truthy so it could never fire; the
route degrades to NotFound if manual-review is absent.
Build green (with and without src/plugins); biome clean.
…ud S7764) Prefer globalThis over window for the reload handler, matching existing globalThis.location usage in the codebase (e.g. SideNavBar). Clears the only open SonarCloud issue on this PR (javascript:S7764, minor code smell).
vishnuszipstack
approved these changes
Jun 26, 2026
…loy) The route ErrorBoundary already catches a rejected dynamic import() (a <Suspense> alone only handles the pending state). Add chunk-error handling so the common production trigger — a stale hashed chunk after a redeploy (client requests a filename the CDN no longer serves) — auto-recovers: - isChunkLoadError() detects the failed-dynamic-import error across browsers. - handleRouteError() (wired as onError on both the app-wide boundary in Router.jsx and the content-scoped one in LazyOutlet) reloads ONCE to pick up fresh chunk hashes, guarded by a sessionStorage timestamp so a genuinely-gone chunk falls through to the manual Reload fallback instead of looping. Non-chunk render errors are never auto-reloaded. This covers the outermost lazy route elements too (e.g. FullPageLayout for verticals, OnboardProduct for llm-whisperer), which render under the Router boundary.
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
This was referenced Jun 26, 2026
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.
What
/landing(login) page from eagerly downloading the full app, and cache fingerprinted assets so repeat visits don't re-fetch the bundle.frontend/nginx.conf: serve content-hashed/assets/*withCache-Control: public, max-age=31536000, immutable; keepindex.htmlandconfig/runtime-config.jsno-cache.React.lazybehind a single<Suspense>inRouter.jsx; new helpersrc/helpers/pluginRegistry.js(lazyPlugin) defers plugin chunks to navigation; app shell (PageLayout/FullPageLayout) made lazy.Why
/landingrequesting 190+ chunks (PDF viewer, recharts, Monaco/ToolIde, every enterprise plugin) before the login screen paints, and content-hashed assets served withCache-ControlTTL 0 (~403 kB re-fetched every visit). Both hurt repeat visits and slow/mobile connections.How
React.lazy(() => import(...)); one<Suspense fallback={<GenericLoader/>}>around<Routes>.lazyPlugin(loader, exportName)wraps a plugin dynamic import as a lazy element; only referenced entrypoints enter the graph. In OSS theoptionalPluginImportsstub makes the import reject, and the element falls back toNotFound, so absent-plugin routes 404 harmlessly (same UX as before).useVerticalsRoutes,useLlmWhispererRoutes) andPRODUCT_NAMESstay guardedawait imports (consumed synchronously). Their heavy page/shell imports are lazy-loaded in the cloud plugin repo (companion PR).PageLayout/FullPageLayoutmade lazy — they statically pulledSideNavBar→lookup-studio→ PDF/charts onto/landing.Can this PR break any existing features?
RequireAuth/RequireGuest/RequireAdmin), and behavior are unchanged. Lazy routes now show a briefGenericLoaderon first load of each chunk. OSS-parity build (withsrc/pluginsremoved) verified green: missing plugins resolve to the stub and routes fall through to NotFound exactly as before.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
unstract-cloudlazy-loads the verticals / llm-whisperer route hooks' pages.Dependencies Versions
lazy/Suspense, already present).Notes on Testing
npm run buildgreen (with and withoutsrc/plugins). Biome clean./landingscript requests via Chrome DevTools: 201 → 93.pdf-vendor(548K), rechartsCategoricalChart(284K), Monaco/ToolIde, and verticals/llm-whisperer pages no longer load until navigation.Screenshots
Checklist
I have read and understood the Contribution Guidelines.