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(); + }; +}