From 6475cb81e00c37616ff8530d76eee043f908508e Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sat, 4 Jul 2026 23:40:46 +0300 Subject: [PATCH] fix(#342 review F1-F4): chunk-load error boundary + non-blocking posthog + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1 [HIGH]: added a root ChunkLoadErrorBoundary (react-error-boundary) wrapping the routed app in main.tsx, ABOVE all the route-level/Aside/AiChatWindow Suspense boundaries. A stale-deploy chunk 404 (React.lazy reject) is caught and auto-reloads once (sessionStorage-guarded against a reload loop), else shows a manual "new version available" reload UI — instead of unmounting the whole tree to a white screen. Existing per-feature ErrorBoundaries untouched. - F2 [MED-HIGH]: posthog no longer blocks/blanks the cloud first paint. main.tsx now renders immediately for everyone, then `void initAnalytics()` — which keeps the exact cloud gate, dynamically imports posthog, and RE-RENDERS the same React root wrapped in PostHogProvider (React reconciles onto the painted DOM, so cloud ends up wrapped exactly as before). The import+init is try/catch'd: a failed analytics chunk (network / stale-404 / ad-blocker on a "posthog" chunk) degrades to no-analytics instead of a permanently blank page. - F3: sanitize-url.test.ts mirroring editor-ext's security contract (javascript:/ data:/vbscript:/obfuscated → ""; https/relative/mailto preserved). - F4: the idle-warm `void import(...)` prefetch in layout.tsx gets `.catch(()=>{})` so a failed best-effort prefetch can't surface as an unhandledrejection. No new deps (lockfile unchanged). Gate: client tsc 0, sanitize test 3/3, client build succeeds (entry chunk still 556K, posthog in separate dynamic chunks). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/chunk-load-error-boundary.tsx | 71 +++++++++++++++++++ .../src/components/layouts/global/layout.tsx | 4 +- apps/client/src/lib/sanitize-url.test.ts | 31 ++++++++ apps/client/src/main.tsx | 30 ++++++-- 4 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 apps/client/src/components/chunk-load-error-boundary.tsx create mode 100644 apps/client/src/lib/sanitize-url.test.ts diff --git a/apps/client/src/components/chunk-load-error-boundary.tsx b/apps/client/src/components/chunk-load-error-boundary.tsx new file mode 100644 index 00000000..07b870c2 --- /dev/null +++ b/apps/client/src/components/chunk-load-error-boundary.tsx @@ -0,0 +1,71 @@ +import { ReactNode } from "react"; +import { ErrorBoundary } from "react-error-boundary"; +import { Button, Center, Stack, Text } from "@mantine/core"; + +const RELOAD_FLAG = "chunk-reload-attempted"; + +// Heuristic detection of a failed dynamic import. Since the code-splitting work, +// every route (plus Aside / AiChatWindow) is React.lazy: when a new deploy +// replaces the hashed chunks, a tab left open on the old index.html requests a +// chunk URL that now 404s, and React.lazy rejects. Browsers / Vite surface these +// with a ChunkLoadError name or one of these messages. +function isChunkLoadError(error: unknown): boolean { + if (!error) return false; + const name = (error as { name?: string }).name ?? ""; + const message = (error as { message?: string }).message ?? ""; + return ( + name === "ChunkLoadError" || + /Failed to fetch dynamically imported module/i.test(message) || + /error loading dynamically imported module/i.test(message) || + /Importing a module script failed/i.test(message) + ); +} + +function handleError(error: unknown) { + if (!isChunkLoadError(error)) return; + // A stale-chunk 404 is cured by a full reload that re-fetches index.html and + // the new chunk manifest. Auto-reload once, guarding against a reload loop + // (e.g. a genuinely missing chunk) with a one-shot sessionStorage flag. If the + // flag is already set we fall through to the manual recovery UI below. + try { + if (sessionStorage.getItem(RELOAD_FLAG)) return; + sessionStorage.setItem(RELOAD_FLAG, "1"); + } catch { + // sessionStorage unavailable (private mode / disabled): skip the automatic + // reload rather than risk an unguarded loop; the fallback UI still recovers. + return; + } + window.location.reload(); +} + +// Root-level boundary that sits ABOVE every route-level Suspense boundary so a +// lazy route/component chunk failure is caught here instead of unmounting the +// whole tree into a blank white screen. Per-feature ErrorBoundaries (page.tsx, +// transclusion, page-embed) remain in place underneath for their local errors. +export function ChunkLoadErrorBoundary({ children }: { children: ReactNode }) { + return ( + { + const chunk = isChunkLoadError(error); + return ( +
+ + + {chunk ? "A new version is available" : "Something went wrong"} + + + {chunk + ? "Please reload the page to load the latest version." + : "An unexpected error occurred. Reloading the page may help."} + + + +
+ ); + }} + > + {children} + + ); +} diff --git a/apps/client/src/components/layouts/global/layout.tsx b/apps/client/src/components/layouts/global/layout.tsx index 9e931c1f..abf4d9bd 100644 --- a/apps/client/src/components/layouts/global/layout.tsx +++ b/apps/client/src/components/layouts/global/layout.tsx @@ -19,7 +19,9 @@ export default function Layout() { const ric = typeof window !== "undefined" && (window as any).requestIdleCallback; const warm = () => { - void import("@/pages/page/page"); + // Best-effort prefetch: a failed warm-up (offline, stale 404) is harmless + // and must not surface as an unhandledrejection. + void import("@/pages/page/page").catch(() => {}); }; if (ric) { const id = ric(warm); diff --git a/apps/client/src/lib/sanitize-url.test.ts b/apps/client/src/lib/sanitize-url.test.ts new file mode 100644 index 00000000..7c0591a9 --- /dev/null +++ b/apps/client/src/lib/sanitize-url.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from "vitest"; +import { sanitizeUrl } from "./sanitize-url"; + +// `sanitizeUrl` is a byte-identical client-local copy of editor-ext's wrapper +// around @braintree/sanitize-url: it maps the sanitizer's "about:blank" XSS +// sentinel to "". These assertions mirror editor-ext's own security-contract +// test so the extracted copy keeps the same guarantees. +describe("sanitizeUrl", () => { + it("blocks dangerous schemes (returns empty string)", () => { + expect(sanitizeUrl("javascript:alert(1)")).toBe(""); + expect(sanitizeUrl("data:text/html,")).toBe(""); + expect(sanitizeUrl("vbscript:msgbox(1)")).toBe(""); + // Case / whitespace obfuscation must not slip past the sanitizer. + expect(sanitizeUrl(" JaVaScRiPt:alert(1)")).toBe(""); + }); + + it("returns empty string for empty / undefined input", () => { + expect(sanitizeUrl(undefined)).toBe(""); + expect(sanitizeUrl("")).toBe(""); + }); + + it("allows safe https, relative file and mailto URLs", () => { + expect(sanitizeUrl("https://example.com/page")).toMatch( + /^https:\/\/example\.com\/page/, + ); + expect(sanitizeUrl("/api/files/abc-123")).toBe("/api/files/abc-123"); + expect(sanitizeUrl("mailto:user@example.com")).toBe( + "mailto:user@example.com", + ); + }); +}); diff --git a/apps/client/src/main.tsx b/apps/client/src/main.tsx index 6efaf778..45c7def3 100644 --- a/apps/client/src/main.tsx +++ b/apps/client/src/main.tsx @@ -14,6 +14,7 @@ import { ModalsProvider } from "@mantine/modals"; import { Notifications } from "@mantine/notifications"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { HelmetProvider } from "react-helmet-async"; +import { ChunkLoadErrorBoundary } from "@/components/chunk-load-error-boundary.tsx"; import "./i18n"; import { getPostHogHost, @@ -43,7 +44,12 @@ function renderApp(app: ReactNode) { - {app} + + {/* Root boundary above every lazy route's Suspense: a stale-chunk + 404 after a deploy is caught and recovered here instead of + blanking the whole app. */} + {app} + @@ -51,12 +57,18 @@ function renderApp(app: ReactNode) { ); } -async function bootstrap() { +async function initAnalytics() { // posthog-js (and its React provider) is only pulled in for cloud deployments // with analytics enabled, so self-hosted builds never download it. The gate is // kept identical to the previous eager code so cloud analytics behavior is // unchanged; the import is simply deferred behind it. - if (isCloud() && isPostHogEnabled) { + // + // Crucially this runs AFTER the immediate first render below, so first paint is + // never gated on the analytics chunk. Any failure (network, stale 404, or an + // ad-blocker blocking a chunk named "posthog") is swallowed so the user keeps a + // working app without analytics instead of a permanently blank page. + if (!(isCloud() && isPostHogEnabled)) return; + try { const { default: posthog } = await import("posthog-js"); const { PostHogProvider } = await import("posthog-js/react"); posthog.init(getPostHogKey(), { @@ -65,14 +77,20 @@ async function bootstrap() { disable_session_recording: true, capture_pageleave: false, }); + // Re-render with the provider now that analytics is ready. React reconciles + // the same root, attaching the PostHog context above the (already painted) + // app so the whole cloud tree is wrapped in PostHogProvider as before. renderApp( , ); - } else { - renderApp(); + } catch { + // Analytics failed to load — degrade gracefully; the app already rendered. } } -void bootstrap(); +// Paint immediately for everyone (self-hosted stays exactly as instant as before, +// cloud no longer blocks on the analytics import). Analytics is attached after. +renderApp(); +void initAnalytics();