From 9eb7130cf92ce72782d6e2f0301b2ee563980cc7 Mon Sep 17 00:00:00 2001 From: Ashunar0 Date: Wed, 21 Oct 2026 13:30:42 +0900 Subject: [PATCH 1/2] fix(pages): keep the named action key out of c.redirect() query merging c.redirect() merges the request query into internal destinations. Inside a named action the first query key only selects the action, so redirecting from POST /books?delete to /books ended up at /books?delete. Drop that key before merging and keep the rest of the query. --- docs/reference/api/rewrites.md | 2 +- packages/void/src/runtime/routing-pages.ts | 9 ++++++ packages/void/src/runtime/routing.ts | 15 ++++++++- .../void/test/unit/routing-runtime.test.ts | 31 ++++++++++++++++++- 4 files changed, 54 insertions(+), 3 deletions(-) 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..506d52ef5 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; }; +// The query key that selected a named action. It addresses the action, not +// the page, so c.redirect() must not carry it into the destination. +const actionQueryKeys = new WeakMap(); +export function getActionQueryKey(request: Request): string | undefined { + return actionQueryKeys.get(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) { + actionQueryKeys.set(context.req.raw, actionName); + } return handlePageMutation( handler, mod.loader, diff --git a/packages/void/src/runtime/routing.ts b/packages/void/src/runtime/routing.ts index 35e19a455..cfd7dd75d 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 { getActionQueryKey } 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,18 @@ export function createRoutingRuntime( }; } +function redirectSearch(request: Request): string { + const search = new URL(request.url).search; + const actionKey = getActionQueryKey(request); + if (actionKey === undefined) { + return search; + } + const params = new URLSearchParams(search); + params.delete(actionKey); + const rest = params.toString(); + return rest ? '?' + 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..21970d920 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,35 @@ describe('typed Pages and SSR execution', () => { expect(response.status).toBe(status); } }); + it('drops the named action key from the query that c.redirect() carries over', async () => { + 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') }), + }), + ); + for (const [path, location] of [ + ['/named?delete', '/named'], + ['/named?delete&page=2', '/named?page=2'], + ['/single?page=2', '/done?page=2'], + ] as const) { + 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( From 77185c08946882c1afc9c879b13af82be4925d76 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 14:59:46 +0800 Subject: [PATCH 2/2] fix(pages): drop only the action segment from the redirect query The action is picked by the first query key, so remove only that first non-empty segment and keep the rest of the query as sent. Deleting the key with URLSearchParams also dropped a page param with the same name (?delete&delete=x) and re-encoded the rest (%20 -> +, flag -> flag=), unlike the same redirect from a single action. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/void/src/runtime/routing-pages.ts | 12 +++++----- packages/void/src/runtime/routing.ts | 22 +++++++++++++------ .../void/test/unit/routing-runtime.test.ts | 20 +++++++++-------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/packages/void/src/runtime/routing-pages.ts b/packages/void/src/runtime/routing-pages.ts index 506d52ef5..180dae577 100644 --- a/packages/void/src/runtime/routing-pages.ts +++ b/packages/void/src/runtime/routing-pages.ts @@ -22,11 +22,11 @@ export type PageRouteOptions = { renderOptions: PageGetArguments[7]; namedActions?: boolean; }; -// The query key that selected a named action. It addresses the action, not -// the page, so c.redirect() must not carry it into the destination. -const actionQueryKeys = new WeakMap(); -export function getActionQueryKey(request: Request): string | undefined { - return actionQueryKeys.get(request); +// 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) => { @@ -67,7 +67,7 @@ export function createPageMutationHandler( }); } if (actionName) { - actionQueryKeys.set(context.req.raw, actionName); + actionQueryRequests.add(context.req.raw); } return handlePageMutation( handler, diff --git a/packages/void/src/runtime/routing.ts b/packages/void/src/runtime/routing.ts index cfd7dd75d..a717bfcde 100644 --- a/packages/void/src/runtime/routing.ts +++ b/packages/void/src/runtime/routing.ts @@ -11,7 +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 { getActionQueryKey } from './routing-pages.ts'; +import { hasActionQueryKey } from './routing-pages.ts'; export { createErrorRenderer } from './routing-errors.ts'; export type RoutingRule = { @@ -248,14 +248,22 @@ export function createRoutingRuntime( function redirectSearch(request: Request): string { const search = new URL(request.url).search; - const actionKey = getActionQueryKey(request); - if (actionKey === undefined) { + if (!hasActionQueryKey(request)) { return search; } - const params = new URLSearchParams(search); - params.delete(actionKey); - const rest = params.toString(); - return rest ? '?' + rest : ''; + // 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 { diff --git a/packages/void/test/unit/routing-runtime.test.ts b/packages/void/test/unit/routing-runtime.test.ts index 21970d920..d7f04a46f 100644 --- a/packages/void/test/unit/routing-runtime.test.ts +++ b/packages/void/test/unit/routing-runtime.test.ts @@ -456,7 +456,15 @@ describe('typed Pages and SSR execution', () => { expect(response.status).toBe(status); } }); - it('drops the named action key from the query that c.redirect() carries over', async () => { + 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); @@ -476,14 +484,8 @@ describe('typed Pages and SSR execution', () => { load: async () => ({ action: redirectTo('/done') }), }), ); - for (const [path, location] of [ - ['/named?delete', '/named'], - ['/named?delete&page=2', '/named?page=2'], - ['/single?page=2', '/done?page=2'], - ] as const) { - const response = await app.fetch(new Request('https://app.test' + path, { method: 'POST' })); - expect(response.headers.get('location')).toBe(location); - } + 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();