Skip to content

Commit 9f67470

Browse files
Brooooooklynclaude
andauthored
fix(pages): keep the named action key out of middleware redirects, rewrites and the page URL (#530)
Follow-ups to #522. That PR dropped the action key (`?delete`) only from `c.redirect()` inside the action. This covers the other places it leaked. ## What changes | Case: `POST /books?delete&page=2` | Before | After | |---|---|---| | Auth / user middleware `c.redirect('/login')` | `/login?delete&page=2` | `/login?page=2` | | Action `c.rewrite('/api/x')`, then `c.redirect('/done')` | `/done?delete&page=2` | `/done?page=2` | | `c.redirect('/books-v2', 307)` (or 308) | `/books-v2?page=2` (since #522) | `/books-v2?delete&page=2` | | `PUT /books?update` + `c.redirect('/books')` (302) | `/books` (since #522) | `/books?update` | | API route under a catch-all named-action page: `POST /api/x?return=/a` + redirect | `?return` dropped (earlier commit here) | `?return=/a` kept | | Action re-renders the page: `page.url` | `/books?delete&page=2` | `/books?page=2` | | …address bar after `useForm` / `action()` | `/books?delete&page=2` | `/books?page=2` | The key goes only when the browser follows with a GET (303, or 301/302 after a POST). Other redirects send the same method again, and the destination needs the key to pick the action. Redirect and rewrite **rules** (`void.json` routing) are unchanged: they keep the query as sent. On Void Cloud, platform dispatch applies them before the Worker and knows nothing about pages, so every target leaves them alone. ## How ``` compile.ts app.use(createNamedActionMarker(matchedRoutes)) ← after routing rules, before auth and user middleware matchedRoutes comes from the app's own resolved hono/route (Hono keys it by a per-copy Symbol) marks only if the first matched Void route handler (tagged: API, page GET, page mutation) is a named-action page; user middleware of any arity is skipped routing.ts c.redirect: drop the first query segment if marked and the browser follows with a GET c.rewrite: no new query → mark the new request too protocol.ts mutation page.url: drop the key if marked client after a POST that renders: if page.url's query == our URL minus its first segment, write that to history pages/action-key.ts shared WeakSet + string helper (no regex). Rolldown drops the WeakSet from client bundles. ``` Docs: the actions guide now says what happens to the query. The rewrites reference mentions middleware and 307/308. ## Flaky tests (second commit) Each of these failed in a local full run under load: | Test | Cause | Fix | |---|---|---| | `plugin-class-a-wrapper` `… during build` | macOS reports `server.mjs`, written just before startup, as changed (2 of 6 runs in a probe). The reload gives the second import a new `Counter`. | `watch: null` (12 of 12 runs stable) | | `plugin-vite-environment` #472 test | Real dev server: about 1 s alone, >5 s under load | `watch: null`, lighter schema file, 30 s limit | | `plugin-entry-imports` | Imports the whole plugin entry: about 4.2 s alone | 30 s limit | ## Tests - `routing-runtime.test.ts`: middleware, 307, rewrite, rewrite with its own query, and page URL. 5 rows fail on main. - `pages-client.test.ts`: history URL after a POST, 4 rows. 2 fail on main; the other 2 check that single-action URLs stay unchanged. - `pages-compile.test.ts`: the marker is emitted after `routingMiddleware` and before user middleware, only for `actions`. - `vp run test --unit`: all pass, except one `cf-version-runner` test that timed out under load and passes alone (not touched here). Lint, format and typecheck are clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 0cf6695 commit 9f67470

25 files changed

Lines changed: 329 additions & 68 deletions

‎docs/guide/pages-routing/actions-and-forms.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ export const actions = {
197197
};
198198
```
199199

200-
Named actions are dispatched via a `?actionName` suffix on the URL (e.g. `/users/42?update`). The client primitives `useForm` and `action()` handle this automatically.
200+
Named actions are dispatched via a `?actionName` suffix on the URL (e.g. `/users/42?update`). The client primitives `useForm` and `action()` handle this automatically. The action key stays out of the page URL. When `POST /users/42?update&tab=profile` renders the page again, `useForm` and `action()` leave the address bar at `/users/42?tab=profile`, and `c.redirect('/users/42')` goes to `/users/42?tab=profile`.
201201

202202
Use `action` or `actions` in a file, not both.
203203

‎docs/reference/api/rewrites.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ URL rewrites re-dispatch a request at a different internal path without changing
1010

1111
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.
1212

13-
`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.
13+
`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. On a request to 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, whether the action or middleware redirects. A redirect after which the browser sends the same request again keeps it: `307` and `308`, and `301` or `302` after a `PUT`, `PATCH` or `DELETE`.
1414

1515
```ts
1616
import { defineHandler } from 'void';
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// On a page with named actions, the first query key selects the action
2+
// (`/books?delete&page=2`). It addresses the action, not the page, so the page
3+
// URL, history and c.redirect() leave it out.
4+
const actionKeyRequests = new WeakSet<Request>();
5+
6+
export function markActionKey(request: Request): void {
7+
actionKeyRequests.add(request);
8+
}
9+
10+
export function hasActionKey(request: Request): boolean {
11+
return actionKeyRequests.has(request);
12+
}
13+
14+
/** `search` without its first non-empty segment; the rest is kept as sent. */
15+
export function searchWithoutActionKey(search: string): string {
16+
let start = 1;
17+
while (search[start] === '&') {
18+
start++;
19+
}
20+
let rest = search.indexOf('&', start);
21+
if (rest === -1) {
22+
return '';
23+
}
24+
while (search[rest] === '&') {
25+
rest++;
26+
}
27+
return rest < search.length ? '?' + search.slice(rest) : '';
28+
}

‎packages/void/src/pages/navigation-engine.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/// <reference lib="dom" />
22

3+
import { searchWithoutActionKey } from './action-key.ts';
34
import { createActionErrorFromResponse, isAbortError } from './action-result.ts';
45
import { initClientHead, applyClientHead } from './head-client.ts';
56
import type { HeadDescriptor } from './head.ts';
@@ -9,6 +10,22 @@ import type { PageObject } from './protocol.ts';
910
import { createScrollRestoration } from './scroll-restoration.ts';
1011
import { isSameOrigin } from './url.ts';
1112

13+
/**
14+
* The server leaves a named action's key out of the page URL. When the page URL
15+
* shows that, drop the key from the URL written to history as well.
16+
*/
17+
function withoutActionKey(fetchUrl: string, pageUrl: string): string {
18+
const q = fetchUrl.indexOf('?');
19+
if (q === -1) {
20+
return fetchUrl;
21+
}
22+
const search = searchWithoutActionKey(fetchUrl.slice(q));
23+
const pageQ = pageUrl.indexOf('?');
24+
return search === (pageQ === -1 ? '' : pageUrl.slice(pageQ))
25+
? fetchUrl.slice(0, q) + search
26+
: fetchUrl;
27+
}
28+
1229
export type PageComponentModule = {
1330
default: unknown;
1431
};
@@ -809,6 +826,7 @@ export async function createPageNavigationEngine(
809826

810827
if (method !== 'GET') {
811828
deferredRegistry.preserveValuesFrom(_committedDeferredRegistry, pageData);
829+
commitTarget.fetchUrl = withoutActionKey(fetchUrl, pageData.url);
812830
}
813831

814832
const [component, layouts] = await Promise.race([

‎packages/void/src/pages/protocol.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { CloudBindings, CloudContext } from '../runtime/handler.ts';
22
import { isConvertedResponse } from '../runtime/response.ts';
3+
import { hasActionKey, searchWithoutActionKey } from './action-key.ts';
34
import { DEFERRED, separateDeferred, type ResolvedDeferredGroup } from './defer.ts';
45
import {
56
resolveHead,
@@ -611,7 +612,9 @@ export async function handlePageMutation(
611612
props,
612613
params: requestParams(c),
613614
shared,
614-
url: requestPathWithSearch(c),
615+
url: hasActionKey(c.req.raw)
616+
? c.req.path + searchWithoutActionKey(new URL(c.req.url).search)
617+
: requestPathWithSearch(c),
615618
errors,
616619
...(head && { head }),
617620
};

‎packages/void/src/plugin-routing.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ export function routingPlugin(root: string, options: RoutingPluginOptions): Plug
204204
const runtimeEmailPath = resolveFile('#self/email');
205205
const compileOptions: CompileOptions = {
206206
honoPath: resolveFile('hono'),
207+
honoRoutePath: resolveFile('hono/route'),
207208
runtimeEnvPath: resolveFile('#self/_env'),
208209
emailDir,
209210
projectRoot: root,

‎packages/void/src/router/compile.ts‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export type AuthCompileOptions = {
3333
export type CompileOptions = {
3434
/** Absolute path to the hono module entry */
3535
honoPath: string;
36+
/** Absolute path to `hono/route` from the same Hono copy as `honoPath` */
37+
honoRoutePath: string;
3638
/** Absolute path to the void/_env module */
3739
runtimeEnvPath: string;
3840
/** Absolute path to the project's `email/` directory; enables `__emailHandler`. */
@@ -154,7 +156,7 @@ export function compileRouteTable(
154156
lines.push(`import { AsyncLocalStorage } from "node:async_hooks";`);
155157
lines.push(`import { Hono } from ${JSON.stringify(options.honoPath)};`);
156158
lines.push(
157-
`import { createRoutingRuntime as __createRoutingRuntime, createHttpRouteHandler as __createHttpRouteHandler, createErrorRenderer as __createErrorRenderer, createPageGetHandler as __createPageGetHandler, createPageMutationHandler as __createPageMutationHandler, createPagesFallback as __createPagesFallback, createModuleLoader as __createModuleLoader, createClientAssetTags as __createClientAssetTags, createManifestLoader as __createManifestLoader, createSsrFallback as __createSsrFallback, createAssetNotFound as __createAssetNotFound } from ${JSON.stringify(ROUTING_RUNTIME_PATH)};`,
159+
`import { createRoutingRuntime as __createRoutingRuntime, createHttpRouteHandler as __createHttpRouteHandler, createErrorRenderer as __createErrorRenderer, createPageGetHandler as __createPageGetHandler, createPageMutationHandler as __createPageMutationHandler, createNamedActionMarker as __createNamedActionMarker, markRouteHandler as __markRouteHandler, createPagesFallback as __createPagesFallback, createModuleLoader as __createModuleLoader, createClientAssetTags as __createClientAssetTags, createManifestLoader as __createManifestLoader, createSsrFallback as __createSsrFallback, createAssetNotFound as __createAssetNotFound } from ${JSON.stringify(ROUTING_RUNTIME_PATH)};`,
158160
);
159161
if (options.nodeTarget) {
160162
lines.push(`const withRuntimeEnv = (_env, fn) => fn();`);
@@ -261,6 +263,15 @@ export function compileRouteTable(
261263
compileHeaderMiddleware(lines, options.headerRules);
262264
}
263265

266+
// After routing rules, which ignore the action key as platform dispatch does,
267+
// and before auth and user middleware, so their c.redirect() drops it too.
268+
if (pages && scan.pages.pages.some((page) => page.methods.includes('actions'))) {
269+
lines.push(
270+
`import { matchedRoutes as __matchedRoutes } from ${JSON.stringify(options.honoRoutePath)};`,
271+
);
272+
lines.push('app.use(__createNamedActionMarker(__matchedRoutes));');
273+
}
274+
264275
if (auth) {
265276
lines.push('app.use(async (c, next) => {');
266277
lines.push(' return runVoidAuthMiddleware(c, next, __voidAuthOptions, __voidAuthDatabase);');
@@ -296,13 +307,13 @@ export function compileRouteTable(
296307
}
297308

298309
if (auth) {
299-
lines.push('app.all("/api/auth", async (c) => {');
310+
lines.push('app.all("/api/auth", __markRouteHandler(async (c) => {');
300311
lines.push(' return handleVoidAuthRequest(c, __voidAuthOptions, __voidAuthDatabase);');
301-
lines.push('});');
312+
lines.push('}));');
302313
lines.push('');
303-
lines.push('app.all("/api/auth/*", async (c) => {');
314+
lines.push('app.all("/api/auth/*", __markRouteHandler(async (c) => {');
304315
lines.push(' return handleVoidAuthRequest(c, __voidAuthOptions, __voidAuthDatabase);');
305-
lines.push('});');
316+
lines.push('}));');
306317
lines.push('');
307318
}
308319

@@ -390,7 +401,7 @@ export function compileRouteTable(
390401
// Pages handlers win for their methods; upgrades were forwarded above.
391402
for (let i = 0; i < webSockets.length; i++) {
392403
lines.push(
393-
`app.all(${JSON.stringify(webSockets[i].pattern)}, (c) => __forwardWebSocketRequest(c.req.raw, c.env, __voidWebSocketRoutes[${i}]));`,
404+
`app.all(${JSON.stringify(webSockets[i].pattern)}, __markRouteHandler((c) => __forwardWebSocketRequest(c.req.raw, c.env, __voidWebSocketRoutes[${i}])));`,
394405
);
395406
}
396407
if (webSockets.length > 0) {
@@ -491,7 +502,7 @@ export function compileRouteTable(
491502
lines.push(' await scheduled(controller, c.env, c.executionCtx);');
492503
lines.push(' return c.json({ ok: true });');
493504
lines.push('};');
494-
lines.push('app.post("/__void/scheduled", __scheduledHandler);');
505+
lines.push('app.post("/__void/scheduled", __markRouteHandler(__scheduledHandler));');
495506
lines.push('');
496507
}
497508

@@ -559,7 +570,7 @@ export function compileRouteTable(
559570
lines.push(' const result = await __processQueue(payload, c.env);');
560571
lines.push(' return c.json(result);');
561572
lines.push('};');
562-
lines.push('app.post("/__void/queue", __queueHandler);');
573+
lines.push('app.post("/__void/queue", __markRouteHandler(__queueHandler));');
563574
lines.push('');
564575
}
565576

@@ -705,20 +716,20 @@ export function compileRouteTable(
705716
lines.push(' return null;');
706717
lines.push('}');
707718
lines.push(
708-
`app.post("/__void/migrate", (c) => __voidCreateMigrationRequestHandler(${auth ? 'true' : 'false'}, false)(c.req.raw));`,
719+
`app.post("/__void/migrate", __markRouteHandler((c) => __voidCreateMigrationRequestHandler(${auth ? 'true' : 'false'}, false)(c.req.raw)));`,
709720
);
710721
lines.push(
711-
`app.get("/__void/migrate/capabilities", (c) => __voidCreateMigrationRequestHandler(${auth ? 'true' : 'false'}, true)(c.req.raw));`,
722+
`app.get("/__void/migrate/capabilities", __markRouteHandler((c) => __voidCreateMigrationRequestHandler(${auth ? 'true' : 'false'}, true)(c.req.raw)));`,
712723
);
713724
lines.push(
714-
'app.post("/__void/migrate/app", (c) => __voidCreateMigrationRequestHandler(false, true)(c.req.raw));',
725+
'app.post("/__void/migrate/app", __markRouteHandler((c) => __voidCreateMigrationRequestHandler(false, true)(c.req.raw)));',
715726
);
716727
lines.push(
717-
'app.get("/__void/migrate/app/attempt/:attemptId", (c) => __voidCreateMigrationRequestHandler(false, true)(c.req.raw));',
728+
'app.get("/__void/migrate/app/attempt/:attemptId", __markRouteHandler((c) => __voidCreateMigrationRequestHandler(false, true)(c.req.raw)));',
718729
);
719730
if (auth) {
720731
lines.push(
721-
'app.post("/__void/migrate/auth", (c) => __voidCreateMigrationRequestHandler(true, true)(c.req.raw));',
732+
'app.post("/__void/migrate/auth", __markRouteHandler((c) => __voidCreateMigrationRequestHandler(true, true)(c.req.raw)));',
722733
);
723734
}
724735
lines.push('');
@@ -733,14 +744,14 @@ export function compileRouteTable(
733744
`import { createBindingHandler as __createBindingHandler } from ${JSON.stringify(options.bindingHandlerPath)};`,
734745
);
735746
lines.push('let __voidBindingApp;');
736-
lines.push('app.all("/__void/*", async (c) => {');
747+
lines.push('app.all("/__void/*", __markRouteHandler(async (c) => {');
737748
lines.push(
738749
' if (!__voidBindingApp) __voidBindingApp = __createBindingHandler(__getRawEnv());',
739750
);
740751
lines.push(' const url = new URL(c.req.url);');
741752
lines.push(' url.pathname = url.pathname.slice(7);');
742753
lines.push(' return __voidBindingApp.fetch(new Request(url.toString(), c.req.raw));');
743-
lines.push('});');
754+
lines.push('}));');
744755
lines.push('');
745756
}
746757

‎packages/void/src/runtime/routing-pages.ts‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { Context } from 'hono';
1+
import type { Context, MiddlewareHandler } from 'hono';
2+
import { markActionKey } from '../pages/action-key.ts';
23
import { handlePageGet, handlePageMutation } from '../pages/protocol.ts';
34
import type { ClientAssetTags } from './routing-assets.ts';
45
import type { ErrorRenderer } from './routing-errors.ts';
@@ -22,14 +23,39 @@ export type PageRouteOptions = {
2223
renderOptions: PageGetArguments[7];
2324
namedActions?: boolean;
2425
};
25-
// Requests whose first query key selected a named action. That key addresses
26-
// the action, not the page, so c.redirect() must not carry it over.
27-
const actionQueryRequests = new WeakSet<Request>();
28-
export function hasActionQueryKey(request: Request): boolean {
29-
return actionQueryRequests.has(request);
26+
function actionNameOf(url: string): string | undefined {
27+
return new URL(url).searchParams.keys().next().value;
28+
}
29+
const routeHandlers = new WeakSet<Function>();
30+
const namedActionHandlers = new WeakSet<Function>();
31+
/** Records a handler that answers a route, so the marker can tell it from middleware. */
32+
export function markRouteHandler<T extends Function>(handler: T): T {
33+
routeHandlers.add(handler);
34+
return handler;
35+
}
36+
/**
37+
* Runs before auth and user middleware, so a c.redirect() there leaves the
38+
* action key out too. It marks the request only when a page with named actions
39+
* is the route that will answer it, not when a broad page pattern also matches.
40+
*
41+
* `matchedRoutes` comes from 'hono/route' of the same Hono copy as the app: it
42+
* reads the request through a Symbol that each copy defines for itself.
43+
*/
44+
export function createNamedActionMarker(
45+
matchedRoutes: (c: Context) => Array<{ handler: Function }>,
46+
): MiddlewareHandler {
47+
return async (c, next) => {
48+
const answer = matchedRoutes(c)
49+
.slice(c.req.routeIndex + 1)
50+
.find((route) => routeHandlers.has(route.handler));
51+
if (answer && namedActionHandlers.has(answer.handler) && actionNameOf(c.req.url)) {
52+
markActionKey(c.req.raw);
53+
}
54+
await next();
55+
};
3056
}
3157
export function createPageGetHandler(options: PageRouteOptions): (c: Context) => Promise<Response> {
32-
return async (context) => {
58+
return markRouteHandler(async (context: Context) => {
3359
const mod = options.load ? await options.load() : { head: options.head };
3460
return handlePageGet(
3561
mod.loader,
@@ -41,16 +67,14 @@ export function createPageGetHandler(options: PageRouteOptions): (c: Context) =>
4167
options.headConfig ?? undefined,
4268
options.renderOptions,
4369
);
44-
};
70+
});
4571
}
4672
export function createPageMutationHandler(
4773
options: PageRouteOptions,
4874
): (c: Context) => Promise<Response> {
49-
return async (context) => {
75+
const mutationHandler = async (context: Context) => {
5076
const mod = options.load ? await options.load() : {};
51-
const actionName = options.namedActions
52-
? new URL(context.req.url).searchParams.keys().next().value
53-
: undefined;
77+
const actionName = options.namedActions ? actionNameOf(context.req.url) : undefined;
5478
// The name comes from the query string, so only own function properties
5579
// count; `?constructor` or `?valueOf` must not reach Object.prototype.
5680
const name = actionName || 'default';
@@ -67,7 +91,7 @@ export function createPageMutationHandler(
6791
});
6892
}
6993
if (actionName) {
70-
actionQueryRequests.add(context.req.raw);
94+
markActionKey(context.req.raw);
7195
}
7296
return handlePageMutation(
7397
handler,
@@ -81,6 +105,10 @@ export function createPageMutationHandler(
81105
options.renderOptions,
82106
);
83107
};
108+
if (options.namedActions) {
109+
namedActionHandlers.add(mutationHandler);
110+
}
111+
return markRouteHandler(mutationHandler);
84112
}
85113

86114
export function createPagesFallback(options: {

0 commit comments

Comments
 (0)