From 0af68ea88291bf4cf6030edfb70f7dab672436d3 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 21 Aug 2026 14:55:28 +0200 Subject: [PATCH] Stop authored links reloading the document on public pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A simple_module app is client-rendered: the root template ships `
` empty and app.tsx fills it with createRoot().render(). So a navigation that creates a *new document* paints a blank white body and only fills in once the bundle has run. Admin screens never hit this — the shell navigates with Inertia's . Authored content does. Pagebuilder widgets, and the markdown and rich-text fields inside them, render author-entered URLs as plain ``, so a public site reloads the whole document every time a visitor clicks its own nav. A downstream site measured one fully blank frame per click, and ~330ms of blank viewport on Fast-3G with a 4x CPU slowdown. Fixing this per widget does not work: 23 widget files render hrefs, and the markdown/rich-text ones turn `[label](href)` into anchors inside a parser, where there is no component to swap for a . One delegated listener on the document catches them all, whatever produced the anchor. It is opt-in rather than an import side effect — a UI package should not install a global click listener just by being imported — and returns a teardown, which is what the tests use. The smpy new template calls it; apps scaffolded before this own their app.tsx and have to add the line, so the CHANGELOG says so. The rules are deliberately biased towards leaving links alone, because taking over a URL Inertia cannot render turns a working download into an error modal while missing one only costs the reload. Left to the browser: other origins, non-http schemes, paths that look like a file, in-page anchors, download/target/ rel=external/data-native-link, modified and non-left clicks, and anchors inside a Puck editor surface — pagebuilder's site-layout editor renders with the iframe disabled, so the edited page's real nav anchors sit in the admin document. Running in the bubble phase means an Inertia is already defaultPrevented and is left alone rather than visited twice. A visit that returns without an x-inertia header falls back to a hard navigation. Claude-Session: https://claude.ai/code/session_01YMPtPuP8YqsVNh3p1hLCuS --- CHANGELOG.md | 20 ++ docs/frontend/inertia.md | 49 +++++ .../templates/host/client_app/app.tsx | 6 + host/client_app/app.tsx | 6 + packages/ui/src/index.ts | 1 + packages/ui/src/lib/spa-links.test.ts | 184 ++++++++++++++++++ packages/ui/src/lib/spa-links.ts | 154 +++++++++++++++ 7 files changed, 420 insertions(+) create mode 100644 packages/ui/src/lib/spa-links.test.ts create mode 100644 packages/ui/src/lib/spa-links.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 215a7f0b..9c2cd6c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,26 @@ All notable changes to this project are documented in this file. The format is b production-mode containers pass `UsersSettings` boot validation. ### Fixed +- Public pages no longer reload the whole document when a visitor clicks a link + in authored content. A simple_module app is client-rendered — the root + template ships `
` empty — so a navigation that creates a + new document paints a blank white body until the bundle has booted. Admin + screens were never affected because the shell navigates with Inertia's + ``, but pagebuilder widgets and their markdown/rich-text fields render + author-entered URLs as plain ``, and there is no component to swap for + a `` when the anchor comes out of a markdown parser. A downstream site + measured ~330ms of blank viewport per click on a throttled connection. + `@simple-module-py/ui` now exports `startSpaLinkInterception()`, a delegated + click handler that routes same-origin page links through Inertia whatever + produced the anchor; the `smpy new` app template calls it. It deliberately + leaves alone anything Inertia cannot render — other origins, non-http schemes, + paths that look like a file, in-page anchors, `download`/`target`/ + `rel="external"`/`data-native-link`, and anchors inside a Puck editor surface + — and falls back to a hard navigation if a visit returns without an + `x-inertia` header, so a media download can never be replaced by an error + modal. **Existing apps must add the one-line call to their own + `host/client_app/app.tsx`**, which is scaffold output and so is not upgraded + for them. - `smpy gen-pages` now emits module stylesheet `@import` lines as **absolute paths** instead of `#module/` alias specifiers. The alias only resolved if the host's `vite.config.ts` defined a matching `resolve.alias` — but that diff --git a/docs/frontend/inertia.md b/docs/frontend/inertia.md index df72c95e..424d0ec9 100644 --- a/docs/frontend/inertia.md +++ b/docs/frontend/inertia.md @@ -149,6 +149,55 @@ createInertiaApp({ `resolvePage` lives in `host/client_app/pages.ts` (hand-written). It builds a page-key → loader map from `moduleGlobs` (the `import.meta.glob` calls in the generated `modules.generated.ts`) plus the host's own `./pages/**/*.tsx` glob, mapping a page key (e.g. `"Orders/Browse"`) to a dynamic import of the matching `.tsx` file. + +## Link navigation + +The app is client-rendered: the root template ships `
` empty +and `app.tsx` fills it. So a navigation that creates a **new document** shows a +blank white page until the bundle has booted — which is what a plain +`` does. + +Admin screens use Inertia's `` and are fine. The problem is authored +content: pagebuilder widgets, and the markdown and rich-text fields inside them, +turn author-entered URLs into plain anchors. An anchor that comes out of a +markdown parser has no component to swap for a ``. + +`app.tsx` therefore calls `startSpaLinkInterception()` once, before the first +render: + +```tsx +import { startSpaLinkInterception } from "@simple-module-py/ui/lib/spa-links"; + +setup({ el, App, props }) { + startSpaLinkInterception(); + createRoot(el).render(); +} +``` + +It delegates one `click` listener on the document and routes same-origin page +links through `router.visit()`, whatever produced the anchor. It runs in the +bubble phase, so a `` — which cancels the event itself — is already +`defaultPrevented` and is left alone rather than visited twice. + +The bias is towards **not** interfering. These keep the browser's own behaviour: + +| Left alone | Why | +| --- | --- | +| Another origin, or a non-http scheme (`mailto:`, `tel:`) | Not ours to route | +| A path whose last segment looks like a file (`/media/report.pdf`) | Inertia would request it expecting a page and raise its error modal over the download | +| `#`, `#section`, or a link to the current path | The browser scrolls correctly; `href="#"` is also what an unfilled pagebuilder nav row holds | +| `download`, `target` other than `_self`, `rel="external"` | The author asked for it | +| `data-native-link` | Explicit opt-out for anything the rules miss | +| Anything inside `[data-puck-preview]` / `[data-puck-component]` | A Puck editor rendering without an iframe puts the edited page's real anchors in the admin document | +| Modified clicks (⌘/ctrl/shift/alt) and non-left buttons | Open-in-new-tab must keep working | + +As a backstop, a visit that comes back **without** an `x-inertia` header is +handed to the browser as a hard navigation, so a URL that turns out not to be a +page still resolves instead of raising the error modal. + +`smpy new` wires this into the app template. An app scaffolded before this +landed owns its own `app.tsx` and must add the call itself. + ## CSRF There is no explicit CSRF token middleware. Protection comes from `SameSite=Lax` on the session cookie: diff --git a/framework/cli/simple_module_cli/templates/host/client_app/app.tsx b/framework/cli/simple_module_cli/templates/host/client_app/app.tsx index 5522a055..5f4b535b 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/app.tsx +++ b/framework/cli/simple_module_cli/templates/host/client_app/app.tsx @@ -1,5 +1,6 @@ import { createInertiaApp, router } from '@inertiajs/react'; import { configureI18n, updateI18n } from '@simple-module-py/i18n'; +import { startSpaLinkInterception } from '@simple-module-py/ui/lib/spa-links'; import { createRoot } from 'react-dom/client'; import { resolvePage } from './pages'; @@ -28,6 +29,11 @@ createInertiaApp({ activeLocale = block.locale; } }); + // Authored content (pagebuilder widgets, markdown and rich-text fields) + // renders author-entered URLs as plain , which the browser would + // follow with a full document load. This app is client-rendered, so that + // means a blank page until the bundle boots. Route them through Inertia. + startSpaLinkInterception(); createRoot(el).render(); }, progress: { diff --git a/host/client_app/app.tsx b/host/client_app/app.tsx index aed2af23..04dbe279 100644 --- a/host/client_app/app.tsx +++ b/host/client_app/app.tsx @@ -1,6 +1,7 @@ import { createInertiaApp, router } from '@inertiajs/react'; import { ErrorBoundary } from '@simple-module-py/ui/components/ErrorBoundary'; import { formatTitle, setTitleAppName } from '@simple-module-py/ui/lib/app-title'; +import { startSpaLinkInterception } from '@simple-module-py/ui/lib/spa-links'; import { useEffect, useRef } from 'react'; import { createRoot } from 'react-dom/client'; import { bootI18nFromInitialPage, subscribeI18nToNavigation } from './i18n'; @@ -29,9 +30,14 @@ createInertiaApp({ useEffect(() => { const stopReset = router.on('navigate', () => boundaryRef.current?.reset()); const stopI18n = subscribeI18nToNavigation(); + // Authored content renders author-entered URLs as plain , which + // the browser follows with a full document load — a blank page until + // this client-rendered app boots again. Route them through Inertia. + const stopLinks = startSpaLinkInterception(); return () => { stopReset(); stopI18n(); + stopLinks(); }; }, []); diff --git a/packages/ui/src/index.ts b/packages/ui/src/index.ts index 75af41e9..5019310d 100644 --- a/packages/ui/src/index.ts +++ b/packages/ui/src/index.ts @@ -10,5 +10,6 @@ export { AppLayout } from './layouts/AppLayout'; export { AuthenticatedLayout } from './layouts/AuthenticatedLayout'; export { PublicLayout } from './layouts/PublicLayout'; export { SidebarLayout } from './layouts/SidebarLayout'; +export { shouldInterceptNavigation, startSpaLinkInterception } from './lib/spa-links'; export { TONE, type Tone } from './lib/tone'; export type { MenuItem, SharedProps } from './types'; diff --git a/packages/ui/src/lib/spa-links.test.ts b/packages/ui/src/lib/spa-links.test.ts new file mode 100644 index 00000000..735501c9 --- /dev/null +++ b/packages/ui/src/lib/spa-links.test.ts @@ -0,0 +1,184 @@ +import { router } from '@inertiajs/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { shouldInterceptNavigation, startSpaLinkInterception } from './spa-links'; + +/** + * The rule has to be conservative in both directions. Taking over a link + * Inertia cannot render replaces a working download with an error modal; + * leaving an ordinary page link alone puts back the full document reload — + * and with it the blank frame a client-rendered app paints while it boots. + * + * The cases pinned here are the ones a naive "same origin?" test gets wrong. + */ + +const HERE = new URL('http://localhost:3000/p/who-we-are'); + +function anchor(html: string): HTMLAnchorElement { + document.body.innerHTML = html; + const el = document.body.querySelector('a'); + if (!el) throw new Error(`no anchor in ${html}`); + return el; +} + +beforeEach(() => { + document.body.innerHTML = ''; +}); + +describe('shouldInterceptNavigation', () => { + it('takes over an ordinary same-origin page link', () => { + expect(shouldInterceptNavigation(anchor('Outputs'), HERE)).toBe(true); + expect(shouldInterceptNavigation(anchor('Home'), HERE)).toBe(true); + expect( + shouldInterceptNavigation(anchor('FAQs'), HERE), + ).toBe(true); + }); + + it('leaves other origins to the browser', () => { + expect( + shouldInterceptNavigation(anchor('Away'), HERE), + ).toBe(false); + }); + + it('leaves non-http schemes alone', () => { + expect(shouldInterceptNavigation(anchor('Mail'), HERE)).toBe( + false, + ); + expect(shouldInterceptNavigation(anchor('Call'), HERE)).toBe(false); + }); + + it('leaves anything that looks like a file to the browser', () => { + // Editor-uploaded documents are served from the same origin; routing one + // through Inertia swaps the download for its error modal. + for (const href of ['/media/report.pdf', '/uploads/logo.png', '/static/dist/main.js']) { + expect(shouldInterceptNavigation(anchor(`f`), HERE)).toBe(false); + } + }); + + it('respects an explicit opt-out', () => { + expect(shouldInterceptNavigation(anchor('Save'), HERE)).toBe(false); + expect(shouldInterceptNavigation(anchor('New'), HERE)).toBe( + false, + ); + expect(shouldInterceptNavigation(anchor('Out'), HERE)).toBe( + false, + ); + expect(shouldInterceptNavigation(anchor('P'), HERE)).toBe( + false, + ); + }); + + it('keeps target="_self" — it means this frame, which is what we do', () => { + expect(shouldInterceptNavigation(anchor('Here'), HERE)).toBe( + true, + ); + }); + + it('leaves in-page anchors to the browser', () => { + // Including `href="#"`, which is what a pagebuilder nav row holds before an + // editor fills it in — taking it over would re-fetch the current page. + for (const href of ['#', '#section', '/p/who-we-are', '/p/who-we-are#team']) { + expect(shouldInterceptNavigation(anchor(`a`), HERE)).toBe(false); + } + }); + + it('does not touch anchors inside a Puck editor surface', () => { + // An editor rendering with `iframe={{ enabled: false }}` puts the edited + // page's real nav anchors into the admin document. + for (const wrapper of ['data-puck-preview', 'data-puck-component']) { + const el = anchor(`
Outputs
`); + expect(shouldInterceptNavigation(el, HERE)).toBe(false); + } + }); + + it('ignores an anchor with no href', () => { + document.body.innerHTML = 'no href'; + const el = document.body.querySelector('a') as HTMLAnchorElement; + expect(shouldInterceptNavigation(el, HERE)).toBe(false); + }); +}); + +describe('startSpaLinkInterception', () => { + let stop: () => void; + let visit: ReturnType; + + beforeEach(() => { + window.history.pushState({}, '', '/p/who-we-are'); + visit = vi.spyOn(router, 'visit').mockImplementation(() => undefined); + stop = startSpaLinkInterception(); + }); + + afterEach(() => { + stop(); + visit.mockRestore(); + }); + + /** + * Dispatch a click and report whether the handler under test took it over. + * + * The verdict is read on `window` — the last stop in the bubble path, after + * the document listener — and the event is then cancelled unconditionally. + * Without that, every click this code correctly declines would reach jsdom's + * link activation, which floods the run with "Not implemented: navigation" + * and would drown a real error. + */ + function click(el: Element, init: MouseEventInit = {}): { prevented: boolean } { + let prevented = false; + const guard = (event: Event): void => { + prevented = event.defaultPrevented; + event.preventDefault(); + }; + window.addEventListener('click', guard); + el.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true, button: 0, ...init }), + ); + window.removeEventListener('click', guard); + return { prevented }; + } + + it('visits an internal link instead of letting the browser navigate', () => { + expect(click(anchor('Outputs')).prevented).toBe(true); + expect(visit).toHaveBeenCalledWith('http://localhost:3000/p/outputs'); + }); + + it('follows a click on an element nested inside the link', () => { + // Authored links wrap icons and spans; the click target is not the anchor. + document.body.innerHTML = 'Go'; + click(document.getElementById('inner') as Element); + expect(visit).toHaveBeenCalledWith('http://localhost:3000/p/outputs'); + }); + + it('leaves a click another handler already took', () => { + // This is what keeps an Inertia from being visited twice: its own + // onClick preventDefaults before the event reaches the document. + const el = anchor('Outputs'); + el.addEventListener('click', (e) => e.preventDefault()); + click(el); + expect(visit).not.toHaveBeenCalled(); + }); + + it('leaves modified and non-left clicks to the browser', () => { + const el = anchor('Outputs'); + for (const init of [ + { metaKey: true }, + { ctrlKey: true }, + { shiftKey: true }, + { altKey: true }, + { button: 1 }, + ]) { + expect(click(el, init).prevented).toBe(false); + } + expect(visit).not.toHaveBeenCalled(); + }); + + it('leaves a link it should not take over', () => { + expect(click(anchor('Report')).prevented).toBe(false); + expect(visit).not.toHaveBeenCalled(); + }); + + it('stops intercepting once torn down', () => { + stop(); + expect(click(anchor('Outputs')).prevented).toBe(false); + expect(visit).not.toHaveBeenCalled(); + stop = () => undefined; // afterEach must not tear down twice + }); +}); diff --git a/packages/ui/src/lib/spa-links.ts b/packages/ui/src/lib/spa-links.ts new file mode 100644 index 00000000..f1a1ee8b --- /dev/null +++ b/packages/ui/src/lib/spa-links.ts @@ -0,0 +1,154 @@ +import { router } from '@inertiajs/react'; + +/** + * Route in-app link clicks through Inertia instead of the browser. + * + * A simple_module app is client-rendered: the root template ships + * `
` empty and `app.tsx` fills it with + * `createRoot().render()`. So a navigation that creates a *new document* paints + * an empty white body first, and only fills in once the bundle has run — one + * blank frame on a fast connection, and the whole boot time on a slow one. + * + * Admin screens never hit this, because the shell navigates with Inertia's + * ``. Authored content is the problem: pagebuilder widgets, and the + * markdown/rich-text fields inside them, render author-entered URLs as plain + * ``. There is no component to swap for a `` — the anchors come + * out of a markdown parser — so a public site reloaded the whole document on + * every click of its own nav. + * + * Delegating on the document catches all of them at once, whatever produced + * the anchor. Call `startSpaLinkInterception()` once, from `app.tsx`, before + * the first render. + * + * It is opt-in rather than an import side effect: installing a global click + * listener is not something a UI package should do just by being imported. + */ + +/** A trailing `.ext` on the last path segment — `/media/report.pdf`. */ +const LOOKS_LIKE_A_FILE = /\.[a-z0-9]+$/i; + +/** + * Marks a subtree whose anchors must keep the browser's own behaviour. + * + * Puck sets these on its editor surfaces. An editor that renders with + * `iframe={{ enabled: false }}` — pagebuilder's site-layout editor does — + * puts the real nav anchors of the page being edited into the admin document, + * where taking them over would be wrong. + */ +const EDITOR_SURFACE = '[data-puck-preview],[data-puck-component]'; + +/** + * Whether the SPA should take over this anchor, given the page it is on. + * + * `here` is a parameter rather than a read of `window.location` so the decision + * is a pure function of its inputs, and so the base for resolving the href is + * explicit — `anchor.href` would quietly resolve against the document's base + * URL instead. + * + * The bias is towards leaving links alone. Taking over a URL Inertia cannot + * render turns a working download into an error modal, while missing one only + * costs the reload this is trying to avoid. + */ +export function shouldInterceptNavigation(anchor: HTMLAnchorElement, here: URL): boolean { + const href = anchor.getAttribute('href'); + if (!href) return false; + + // Author opt-outs, plus the two the HTML spec already provides. + if (anchor.hasAttribute('download')) return false; + if (anchor.hasAttribute('data-native-link')) return false; + const target = anchor.getAttribute('target'); + if (target && target !== '_self') return false; + if ((anchor.getAttribute('rel') ?? '').split(/\s+/).includes('external')) return false; + + if (anchor.closest(EDITOR_SURFACE)) return false; + + let url: URL; + try { + url = new URL(href, here.href); + } catch { + return false; + } + + // `mailto:`, `tel:`, and anything else the browser owns outright. + if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; + if (url.origin !== here.origin) return false; + + // Same document: `#`, `#section`, or a link back to the current page. The + // browser handles all three correctly, and `href="#"` is what a pagebuilder + // nav row holds before an editor fills it in. + if (url.pathname === here.pathname && url.search === here.search) return false; + + // Same origin but not a page: media uploads, module static mounts. Inertia + // would request these expecting a page payload and raise its error modal + // over the file. `startSpaLinkInterception` keeps a hard-navigation fallback + // for whatever still gets through. + if (LOOKS_LIKE_A_FILE.test(url.pathname.split('/').pop() ?? '')) return false; + + return true; +} + +/** A click the browser would otherwise handle by navigating this frame. */ +function isPlainLeftClick(event: MouseEvent): boolean { + return ( + event.button === 0 && + !event.metaKey && + !event.ctrlKey && + !event.shiftKey && + !event.altKey && + !event.defaultPrevented + ); +} + +/** + * Install the delegated click handler. Call once, from `app.tsx`. + * + * Returns a function that removes it again, which is what tests use; an app has + * no reason to. + */ +export function startSpaLinkInterception(): () => void { + // Set when a click is taken over, cleared when the visit comes back as a real + // Inertia response. If it does not, the URL was not a page after all and the + // browser should have had it — hand it back rather than showing Inertia's + // error modal over a link that used to work. + let takenOver: string | null = null; + + const offSuccess = router.on('success', () => { + takenOver = null; + }); + + const offInvalid = router.on('invalid', (event) => { + if (!takenOver) return; + const url = takenOver; + takenOver = null; + event.preventDefault(); + window.location.href = url; + }); + + // Bubble phase on the document, so React's own handlers have already run — + // they are attached at the root container, inside this. An Inertia `` + // therefore shows up here as `defaultPrevented`, and is left alone instead of + // being visited a second time. + const onClick = (event: MouseEvent): void => { + if (!isPlainLeftClick(event)) return; + const target = event.target; + if (!(target instanceof Element)) return; + const anchor = target.closest('a[href]'); + if (!(anchor instanceof HTMLAnchorElement)) return; + + const here = new URL(window.location.href); + if (!shouldInterceptNavigation(anchor, here)) return; + + const url = new URL(anchor.getAttribute('href') as string, here.href).href; + event.preventDefault(); + takenOver = url; + router.visit(url); + }; + + document.addEventListener('click', onClick); + + return () => { + document.removeEventListener('click', onClick); + offSuccess(); + offInvalid(); + }; +}