diff --git a/docs/reference/api/rewrites.md b/docs/reference/api/rewrites.md index 965634396..4705c0b72 100644 --- a/docs/reference/api/rewrites.md +++ b/docs/reference/api/rewrites.md @@ -10,7 +10,7 @@ URL rewrites re-dispatch a request at a different internal path without changing Re-dispatches the current request at `destination` and returns the resulting `Response`. The browser URL does not change — this is a server-side hop. If `destination` has no query string, the original request query is preserved. If `destination` includes a query string, it replaces the original query. `destination` must start with `/`; known generated routes autocomplete via [`RewriteDestination`](#rewritedestination), while arbitrary strings remain accepted for dynamic paths. -`c.redirect()` also preserves request query params for internal destinations, but it merges rather than replaces: destination params win on collision, and request-only params are appended. +`c.redirect()` also preserves request query params for internal destinations, but it merges rather than replaces: destination params win on collision, and request-only params are appended. Inside a [named action](../../guide/pages-routing/actions-and-forms.md#named-actions), the action key (`?delete` in `POST /users/42?delete`) is not carried over. ```ts import { defineHandler } from 'void'; diff --git a/packages/void/src/runtime/routing-pages.ts b/packages/void/src/runtime/routing-pages.ts index 99833f841..180dae577 100644 --- a/packages/void/src/runtime/routing-pages.ts +++ b/packages/void/src/runtime/routing-pages.ts @@ -22,6 +22,12 @@ export type PageRouteOptions = { renderOptions: PageGetArguments[7]; namedActions?: boolean; }; +// Requests whose first query key selected a named action. That key addresses +// the action, not the page, so c.redirect() must not carry it over. +const actionQueryRequests = new WeakSet(); +export function hasActionQueryKey(request: Request): boolean { + return actionQueryRequests.has(request); +} export function createPageGetHandler(options: PageRouteOptions): (c: Context) => Promise { return async (context) => { const mod = options.load ? await options.load() : { head: options.head }; @@ -60,6 +66,9 @@ export function createPageMutationHandler( status: 404, }); } + if (actionName) { + actionQueryRequests.add(context.req.raw); + } return handlePageMutation( handler, mod.loader, diff --git a/packages/void/src/runtime/routing.ts b/packages/void/src/runtime/routing.ts index 35e19a455..a717bfcde 100644 --- a/packages/void/src/runtime/routing.ts +++ b/packages/void/src/runtime/routing.ts @@ -11,6 +11,7 @@ import type { RedirectStatusCode } from 'hono/utils/http-status'; import { VoidAssetRewriteError } from './handler.ts'; import { convertReturnValue } from './response.ts'; import { ASSET_EXTENSIONS } from './routing-assets.ts'; +import { hasActionQueryKey } from './routing-pages.ts'; export { createErrorRenderer } from './routing-errors.ts'; export type RoutingRule = { @@ -175,7 +176,7 @@ export function createRoutingRuntime( const redirect = c.redirect.bind(c); c.redirect = (location, status) => { if (typeof location === 'string' && location.startsWith('/') && !location.startsWith('//')) { - return redirect(mergeQueryIntoLocation(location, new URL(c.req.raw.url).search), status); + return redirect(mergeQueryIntoLocation(location, redirectSearch(c.req.raw)), status); } return redirect(location, status); }; @@ -245,6 +246,26 @@ export function createRoutingRuntime( }; } +function redirectSearch(request: Request): string { + const search = new URL(request.url).search; + if (!hasActionQueryKey(request)) { + return search; + } + // The action key is the first non-empty segment; keep the rest as sent. + let start = 1; + while (search[start] === '&') { + start++; + } + let rest = search.indexOf('&', start); + if (rest === -1) { + return ''; + } + while (search[rest] === '&') { + rest++; + } + return rest < search.length ? '?' + search.slice(rest) : ''; +} + export function markNoMatch(request: Request, response: Response): Response { const url = new URL(request.url); if ( diff --git a/packages/void/test/unit/routing-runtime.test.ts b/packages/void/test/unit/routing-runtime.test.ts index 018f0c224..d7f04a46f 100644 --- a/packages/void/test/unit/routing-runtime.test.ts +++ b/packages/void/test/unit/routing-runtime.test.ts @@ -1,4 +1,4 @@ -import { Hono } from 'hono'; +import { Hono, type Context } from 'hono'; import { HTTPException } from 'hono/http-exception'; import { describe, expect, it, vi } from 'vite-plus/test'; import { createClientAssetTags, createManifestLoader } from '../../src/runtime/routing-assets.ts'; @@ -456,6 +456,37 @@ describe('typed Pages and SSR execution', () => { expect(response.status).toBe(status); } }); + it.each([ + ['/named?delete', '/named'], + ['/named?delete&page=2', '/named?page=2'], + ['/named?delete=1&page=2', '/named?page=2'], + ['/named?&delete&page=2', '/named?page=2'], + ['/named?delete&delete=x&page=2', '/named?delete=x&page=2'], + ['/named?delete&q=a%20b&flag', '/named?q=a%20b&flag'], + ['/single?page=2', '/done?page=2'], + ])('drops only the named action key when c.redirect() carries %s', async (path, location) => { + const app = new Hono(); + const runtime = createRoutingRuntime(app, { nodeTarget: true }); + app.use(runtime.contextMiddleware); + const redirectTo = (location: string) => (c: Context) => c.redirect(location); + app.post( + '/named', + createPageMutationHandler({ + ...pageOptions, + load: async () => ({ actions: { delete: redirectTo('/named') } }), + namedActions: true, + }), + ); + app.post( + '/single', + createPageMutationHandler({ + ...pageOptions, + load: async () => ({ action: redirectTo('/done') }), + }), + ); + const response = await app.fetch(new Request('https://app.test' + path, { method: 'POST' })); + expect(response.headers.get('location')).toBe(location); + }); it('executes the real page loader and protocol', async () => { const app = new Hono(); app.get(