diff --git a/.changeset/rewrite-spa-navigation.md b/.changeset/rewrite-spa-navigation.md new file mode 100644 index 00000000000..08661084af5 --- /dev/null +++ b/.changeset/rewrite-spa-navigation.md @@ -0,0 +1,5 @@ +--- +'@qwik.dev/router': patch +--- + +fix: client navigation to a URL that only exists through a server `rewrite()` now renders the rewrite target instead of the 404 page. When no page matches on the client, the router asks the server (`q-route.json`) where the URL routes to and loads that route, keeping the original URL. diff --git a/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/plugin@rewrite-spa.ts b/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/plugin@rewrite-spa.ts new file mode 100644 index 00000000000..aa8bed4d794 --- /dev/null +++ b/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/plugin@rewrite-spa.ts @@ -0,0 +1,14 @@ +import type { RequestHandler } from '@qwik.dev/router'; + +// Only `/rewrite-spa/target/` is a route. `/rewrite-spa/pretty/` exists only through this rewrite. +export const onRequest: RequestHandler = ({ url, rewrite, cacheControl, headers }) => { + if (url.pathname === '/qwikrouter-test/rewrite-spa/pretty/') { + throw rewrite('/qwikrouter-test/rewrite-spa/target/'); + } + // Same rewrite, but the middleware decides the answer is cacheable and tagged. + if (url.pathname === '/qwikrouter-test/rewrite-spa/pretty-cached/') { + cacheControl({ maxAge: 60 }, 'CDN-Cache-Control'); + headers.set('Cache-Tag', 'rewrite-spa-target'); + throw rewrite('/qwikrouter-test/rewrite-spa/target/'); + } +}; diff --git a/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/rewrite-spa/index.tsx b/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/rewrite-spa/index.tsx new file mode 100644 index 00000000000..500f6eb39d9 --- /dev/null +++ b/e2e/qwik-e2e/apps/qwikrouter-test/src/routes/rewrite-spa/index.tsx @@ -0,0 +1,13 @@ +import { component$ } from '@qwik.dev/core'; +import { Link } from '@qwik.dev/router'; + +export default component$(() => { + return ( + <> +
{target.value.original}
+{target.value.served}
+ > + ); +}); diff --git a/e2e/qwik-e2e/dev-server.ts b/e2e/qwik-e2e/dev-server.ts index c9eae751426..b391a53111d 100644 --- a/e2e/qwik-e2e/dev-server.ts +++ b/e2e/qwik-e2e/dev-server.ts @@ -198,6 +198,7 @@ export { router } ], // Legacy e2e tests rely on actions re-running all loaders strictLoaders: false, + serverRewrites: true, }) as PluginOption // qwikRouterSsg.nodeServerAdapter({ // ssg: null, diff --git a/e2e/qwik-e2e/tests/qwikrouter/rewrite-spa.e2e.ts b/e2e/qwik-e2e/tests/qwikrouter/rewrite-spa.e2e.ts new file mode 100644 index 00000000000..cb5abfc4236 --- /dev/null +++ b/e2e/qwik-e2e/tests/qwikrouter/rewrite-spa.e2e.ts @@ -0,0 +1,44 @@ +import { expect, test } from '@playwright/test'; + +const pretty = '/qwikrouter-test/rewrite-spa/pretty/'; +const target = '/qwikrouter-test/rewrite-spa/target/'; + +test.describe('rewrite to a different route', () => { + test('SSR serves the target page under the original URL', async ({ page }) => { + const response = await page.goto(pretty); + expect(response?.status()).toBe(200); + await expect(page.locator('#rewrite-spa-target')).toBeVisible(); + await expect(page.locator('#rewrite-spa-original')).toHaveText(pretty); + await expect(page.locator('#rewrite-spa-served')).toHaveText(target); + }); + + test('q-route.json uses the cache headers the rewriting middleware set', async ({ request }) => { + const plain = await request.get(`${pretty}q-route.json`); + expect(await plain.json()).toEqual({ p: target }); + expect(plain.headers()['cdn-cache-control']).toBeUndefined(); + expect(plain.headers()['cache-tag']).toBeUndefined(); + + const cached = await request.get('/qwikrouter-test/rewrite-spa/pretty-cached/q-route.json'); + expect(await cached.json()).toEqual({ p: target }); + expect(cached.headers()['cdn-cache-control']).toContain('max-age=60'); + expect(cached.headers()['cache-tag']).toBe('rewrite-spa-target'); + }); + + test('SPA navigation renders the target and keeps the URL, without a reload', async ({ + page, + }) => { + const pageErrors: Error[] = []; + page.on('pageerror', (err) => pageErrors.push(err)); + await page.goto('/qwikrouter-test/rewrite-spa/'); + await page.evaluate(() => ((window as any).__noReload = true)); + + await page.locator('[data-test-link="rewrite-spa-pretty"]').click(); + + await expect(page.locator('#rewrite-spa-target')).toBeVisible(); + await expect(page).toHaveURL(new RegExp(`${pretty}$`)); + await expect(page.locator('#rewrite-spa-original')).toHaveText(pretty); + await expect(page.locator('#rewrite-spa-served')).toHaveText(target); + expect(await page.evaluate(() => (window as any).__noReload)).toBe(true); + expect(pageErrors).toEqual([]); + }); +}); diff --git a/packages/docs/src/routes/docs/(qwikrouter)/guides/rewrites/index.mdx b/packages/docs/src/routes/docs/(qwikrouter)/guides/rewrites/index.mdx index 6067a50ae82..6c58cc3e9e2 100644 --- a/packages/docs/src/routes/docs/(qwikrouter)/guides/rewrites/index.mdx +++ b/packages/docs/src/routes/docs/(qwikrouter)/guides/rewrites/index.mdx @@ -46,6 +46,32 @@ Check `internalRequest` before applying broad rewrite rules. Qwik Router uses in requests for route loader fetches and fetch-based action submissions, and those requests must keep their internal routing behavior. +## Rewrites and client navigation + +A `rewrite()` happens on the server, so on a client navigation the router can't know which route a rewritten URL renders. If the URL has no page of its own, or only matches a dynamic (`[param]`) or wildcard route, it would render the wrong thing or the 404 page. + +Enable `serverRewrites` in the Vite plugin to have the router ask the server where such a URL finally routes to (`q-route.json`), then render that route while keeping the original URL: + +```ts title="vite.config.ts" +qwikRouter({ serverRewrites: true }); +``` + +A static route that matches the URL exactly is always rendered as-is, so don't keep a page on a URL that your middleware rewrites. + +The answer is sent with the response headers your middleware has set when it calls `rewrite()`, and `private` if it set no `Cache-Control`, like loaders. To cache it (browser, CDN, ...), set the headers before rewriting. If the rewrite depends on the user (cookies, auth), keep it private or `Vary` accordingly: + +```tsx +export const onRequest: RequestHandler = async ({ url, rewrite, cacheControl, headers }) => { + const match = url.pathname.match(/^\/articles\/(.*)$/); + if (match) { + const { id } = await db.getArticleByName(match[1]); + cacheControl({ maxAge: 300, staleWhileRevalidate: 3600 }, 'CDN-Cache-Control'); + headers.set('Cache-Tag', `article-${id}`); + throw rewrite(`/articles/${id}`); + } +}; +``` + The `rewrite()` function, which was destructured in the RequestHandler function arguments, is invoked with a pathname string. ```tsx diff --git a/packages/qwik-router/global.d.ts b/packages/qwik-router/global.d.ts index 8453b85e064..92fa6193e63 100644 --- a/packages/qwik-router/global.d.ts +++ b/packages/qwik-router/global.d.ts @@ -39,5 +39,8 @@ declare var __SSR_CACHE_SIZE__: number; */ declare var __STRICT_LOADERS__: boolean; +/** Resolve server `rewrite()` targets on client navigation (`serverRewrites` plugin option). */ +declare var __SERVER_REWRITES__: boolean; + declare var __QWIK_BUILD_DIR__: string; declare var __QWIK_ASSETS_DIR__: string; diff --git a/packages/qwik-router/src/buildtime/context.ts b/packages/qwik-router/src/buildtime/context.ts index 855c3ad5ea2..87b7ad506d8 100644 --- a/packages/qwik-router/src/buildtime/context.ts +++ b/packages/qwik-router/src/buildtime/context.ts @@ -110,6 +110,9 @@ function normalizeOptions( if (opts.strictLoaders === undefined) { opts.strictLoaders = true; } + if (opts.serverRewrites === undefined) { + opts.serverRewrites = false; + } return opts; } diff --git a/packages/qwik-router/src/buildtime/types.ts b/packages/qwik-router/src/buildtime/types.ts index 34f3be7e515..e0cbb5353e7 100644 --- a/packages/qwik-router/src/buildtime/types.ts +++ b/packages/qwik-router/src/buildtime/types.ts @@ -183,6 +183,21 @@ export interface PluginOptions { * Defaults to `true`. */ strictLoaders?: boolean; + /** + * Resolve `rewrite()` targets on client navigation. + * + * A `rewrite()` made by middleware (e.g. a database lookup that maps `/articles/some-title` to + * `/articles/7777`) only happens on the server, so the client can't tell which route to render + * for the original URL. When true, a client navigation to a URL that has no page, or only matches + * a dynamic or wildcard route, first asks the server (`q-route.json`) where the URL finally + * routes to, then renders that route while keeping the original URL. + * + * A static route that matches the URL exactly is always rendered as-is. Costs one extra request + * for those navigations. + * + * Defaults to `false`. + */ + serverRewrites?: boolean; } export interface MdxPlugins { diff --git a/packages/qwik-router/src/buildtime/vite/plugin.ts b/packages/qwik-router/src/buildtime/vite/plugin.ts index e36559935d1..ea7c7b58deb 100644 --- a/packages/qwik-router/src/buildtime/vite/plugin.ts +++ b/packages/qwik-router/src/buildtime/vite/plugin.ts @@ -349,6 +349,7 @@ function qwikRouterPlugin( viteEnv.command === 'serve' ? 0 : (userOpts?.ssrCacheSize ?? 50) ), 'globalThis.__STRICT_LOADERS__': JSON.stringify(userOpts?.strictLoaders !== false), + 'globalThis.__SERVER_REWRITES__': JSON.stringify(userOpts?.serverRewrites === true), }, appType: 'custom', resolve: { diff --git a/packages/qwik-router/src/middleware/request-handler/handlers/route-resolve-handler.ts b/packages/qwik-router/src/middleware/request-handler/handlers/route-resolve-handler.ts new file mode 100644 index 00000000000..f3bb2d32304 --- /dev/null +++ b/packages/qwik-router/src/middleware/request-handler/handlers/route-resolve-handler.ts @@ -0,0 +1,24 @@ +import type { RequestEvent, RequestHandler } from '../types'; +import { IsQRoute } from '../request-path'; + +/** + * Answers `q-route.json` requests with the pathname the URL finally routes to, after any + * `rewrite()`. It sits after the plugins and route middleware, so a rewrite has already re-resolved + * the route by the time it runs. Lets the client load a rewrite target it can't match by itself. + * + * The response carries the headers set so far, so a middleware can make a rewrite cacheable with + * `cacheControl()` / `headers.set('Cache-Tag', ...)` before `throw rewrite(...)`. + */ +export function routeResolveHandler(): RequestHandler { + return (requestEv: RequestEvent) => { + if (!requestEv.sharedMap.has(IsQRoute) || requestEv.headersSent || requestEv.exited) { + return; + } + // Caching is up to the middleware that rewrites (cacheControl(), Cache-Tag, ...), same as for + // loaders: private unless it said otherwise. + if (!requestEv.headers.has('Cache-Control')) { + requestEv.cacheControl('private'); + } + requestEv.json(200, { p: requestEv.url.pathname }); + }; +} diff --git a/packages/qwik-router/src/middleware/request-handler/request-event-core.ts b/packages/qwik-router/src/middleware/request-handler/request-event-core.ts index 53573f4eda3..49d84147cb9 100644 --- a/packages/qwik-router/src/middleware/request-handler/request-event-core.ts +++ b/packages/qwik-router/src/middleware/request-handler/request-event-core.ts @@ -15,6 +15,7 @@ import { createCacheControl } from './cache-control'; import { Cookie } from './cookie'; import { IsQAction, + IsQLoader, QActionId, QLoaderId, recognizeRequest, @@ -68,7 +69,10 @@ export function createRequestEvent( if (recognized) { url.pathname = trimRecognizedInternalPathname(url.pathname, recognized); sharedMap.set(recognized.type, true); - internalRequest = 'loader'; + // A route-resolve request is a navigation, so middleware (e.g. rewrites) must treat it as one. + if (recognized.type === IsQLoader) { + internalRequest = 'loader'; + } if (recognized.data?.loaderId) { sharedMap.set(QLoaderId, recognized.data.loaderId); } diff --git a/packages/qwik-router/src/middleware/request-handler/request-path.ts b/packages/qwik-router/src/middleware/request-handler/request-path.ts index c02cd3195b0..e72bb81b5cb 100644 --- a/packages/qwik-router/src/middleware/request-handler/request-path.ts +++ b/packages/qwik-router/src/middleware/request-handler/request-path.ts @@ -3,6 +3,7 @@ import { ensureSlash } from '../../utils/pathname'; // New per-loader/per-action URL patterns export const IsQLoader = '@isQLoader'; export const IsQAction = '@isQAction'; +export const IsQRoute = '@isQRoute'; export const QLoaderId = '@loaderId'; export const QActionId = '@actionId'; @@ -12,11 +13,16 @@ export const LOADER_REGEX = /\/(q-loader-([^/.]+)\.([^/.]+)\.json)$/; export const getLoaderName = (loaderId: string, manifestHash: string) => `q-loader-${loaderId}.${manifestHash}.json`; -export type RecognizedRequest = { - type: typeof IsQLoader; - trimLength: number; - data: { loaderId?: string; manifestHash?: string } | null; -}; +/** Matches the final `/q-route.json` path segment: the client asks where a URL really routes to. */ +export const ROUTE_REGEX = /\/q-route\.json$/; + +export type RecognizedRequest = + | { + type: typeof IsQLoader; + trimLength: number; + data: { loaderId?: string; manifestHash?: string } | null; + } + | { type: typeof IsQRoute; trimLength: number; data: null }; /** * Recognize internal request types from the URL pathname. @@ -40,6 +46,10 @@ export function recognizeRequest(pathname: string): RecognizedRequest | null { }; } + if (ROUTE_REGEX.test(pathname)) { + return { type: IsQRoute, trimLength: '/q-route.json'.length, data: null }; + } + return null; } diff --git a/packages/qwik-router/src/middleware/request-handler/request-path.unit.ts b/packages/qwik-router/src/middleware/request-handler/request-path.unit.ts index 9fdaf293aaa..6b74a53018d 100644 --- a/packages/qwik-router/src/middleware/request-handler/request-path.unit.ts +++ b/packages/qwik-router/src/middleware/request-handler/request-path.unit.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import { getLoaderName, + IsQRoute, recognizeRequest, resolveValidInternalFullPathname, trimInternalPathname, @@ -49,6 +50,17 @@ describe('request path helpers', () => { ); }); + it('recognizes q-route requests and trims them like any internal request', () => { + globalThis.__NO_TRAILING_SLASH__ = false; + const recognized = recognizeRequest('/articles/qwik-is-quick/q-route.json'); + + expect(recognized?.type).toBe(IsQRoute); + expect( + trimRecognizedInternalPathname('/articles/qwik-is-quick/q-route.json', recognized!) + ).toBe('/articles/qwik-is-quick/'); + expect(recognizeRequest('/articles/q-route.json.map')).toBeNull(); + }); + it('accepts internal full pathnames below the loader pathname', () => { expect(resolveValidInternalFullPathname('/products/123/', '/products/123/view/')).toBe( '/products/123/view/' diff --git a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts index 2514f08d922..5e05d3c30f0 100644 --- a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts +++ b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts @@ -36,6 +36,7 @@ import { import { loaderHandler, setRouteLoaderData } from './handlers/loader-handler'; import { jsonRequestWrapper } from './handlers/json-request-wrapper'; import { actionHandler } from './handlers/action-handler'; +import { routeResolveHandler } from './handlers/route-resolve-handler'; import { IsQLoader, QLoaderId } from './request-path'; import { isStaticPath } from './static-paths'; import type { ErrorCodes, RequestEvent, RequestEventBase, RequestHandler } from './types'; @@ -119,6 +120,7 @@ function createResolveRequestHandlers() { if (isPageRoute) { // Per-loader handler: returns JSON with metadata and exits if IsQLoader is set + requestHandlers.push(routeResolveHandler()); requestHandlers.push(loaderHandler(routeLoaders, route)); // Per-action handler: returns JSON and exits if IsQAction + Accept: json requestHandlers.push(actionHandler(routeActions)); diff --git a/packages/qwik-router/src/runtime/src/qwik-router-component.tsx b/packages/qwik-router/src/runtime/src/qwik-router-component.tsx index 32da94d5040..6feb11b906e 100644 --- a/packages/qwik-router/src/runtime/src/qwik-router-component.tsx +++ b/packages/qwik-router/src/runtime/src/qwik-router-component.tsx @@ -191,6 +191,29 @@ const ensureRouteInternal = ( ): RouteStateInternal => (routeInternal.untrackedValue ||= { type: 'initial', dest: routeLocation.url }); +/** + * A URL the client can't render itself (no matching page) may be a server `rewrite()`. Ask the + * server where it routes to. Returns the target pathname, `'reload'` if the server redirects, or + * `undefined` when the URL is not rewritten. + */ +const resolveServerRewrite = async (url: URL): Promise