Actions: auto-redirect action response to avoid "confirm form resubmi…

…ssion" dialog (#11603)

* feat: redirect with result to avoid resubmission dialog

* fix: use standard POST for react forms

* fix(test): handle redirect responses

* refactor: actionresultbehavior -> disableredirect

* refactor: next() -> throw internal error

* fix(test): bad referer link

* refactor: followRedirect -> followExpectedRedirect

* refactor: remove encryption TODO

* feat: changeset

* chore: whitespace

* feat: laravel note

* refactor: clean up cookie -> action payload

* refactor: actionsinternal -> actionpayload

* refactor: use _astroAction const

* refactor: actionRedirect string as const

* refactor: simplify error check

* chore: remove stray console log

* refactor: only delete cookies on error

* fix: check cookie after handling POST requests

* chore: remove unused tgz

* Revert "fix: check cookie after handling POST requests"

This reverts commit 607f90f.

* Revert "refactor: only delete cookies on error"

This reverts commit 52aab84.

.changeset/rotten-months-report.md

@@ -0,0 +1,27 @@
+---
+'astro': patch
+---
+
+Improves user experience when render an Action result from a form POST request:
+
+- Removes "Confirm post resubmission?" dialog when refreshing a result.
+- Removes the `?_astroAction=NAME` flag when a result is rendered.
+
+Also improves the DX of directing to a new route on success. Actions will now redirect to the route specified in your `action` string on success, and redirect back to the previous page on error. This follows the routing convention of established backend frameworks like Laravel.
+
+For example, say you want to redirect to a `/success` route when `actions.signup` succeeds. You can add `/success` to your `action` string like so:
+
+```astro
+<form method="POST" action={"/success" + actions.signup}>
+```
+
+- On success, Astro will redirect to `/success`. 
+- On error, Astro will redirect back to the current page.
+
+You can retrieve the action result from either page using the `Astro.getActionResult()` function.
+
+### Note on security
+
+This uses a temporary cookie to forward the action result to the next page. The cookie will be deleted when that page is rendered.
+
+⚠ **The action result is not encrypted.** In general, we recommend returning minimal data from an action handler to a) avoid leaking sensitive information, and b) avoid unexpected render issues once the temporary cookie is deleted. For example, a `login` function may return a user's session id to retrieve from your Astro frontmatter, rather than the entire user object.

packages/astro/src/actions/consts.ts

@@ -4,3 +4,9 @@ export const ACTIONS_TYPES_FILE = 'actions.d.ts';
 export const VIRTUAL_INTERNAL_MODULE_ID = 'astro:internal-actions';
 export const RESOLVED_VIRTUAL_INTERNAL_MODULE_ID = '\0astro:internal-actions';
 export const NOOP_ACTIONS = '\0noop-actions';
+
+export const ACTION_QUERY_PARAMS = {
+	actionName: '_astroAction',
+	actionPayload: '_astroActionPayload',
+	actionRedirect: '_astroActionRedirect',
+};

packages/astro/src/actions/runtime/middleware.ts

@@ -10,22 +10,33 @@ import {
 	type SerializedActionResult,
 	serializeActionResult,
 } from './virtual/shared.js';
+import { ACTION_QUERY_PARAMS } from '../consts.js';
+
+export type ActionPayload = {
+	actionResult: SerializedActionResult;
+	actionName: string;
+};
 
 export type Locals = {
-	_actionsInternal: {
-		actionResult: SerializedActionResult;
-		actionName: string;
-	};
+	_actionPayload: ActionPayload;
 };
 
 export const onRequest = defineMiddleware(async (context, next) => {
 	const locals = context.locals as Locals;
 	const { request } = context;
 	// Actions middleware may have run already after a path rewrite.
 	// See https://github.com/withastro/roadmap/blob/feat/reroute/proposals/0047-rerouting.md#ctxrewrite
-	// `_actionsInternal` is the same for every page,
+	// `_actionPayload` is the same for every page,
 	// so short circuit if already defined.
-	if (locals._actionsInternal) return next();
+	if (locals._actionPayload) return next();
+
+	const actionPayload = context.cookies.get(ACTION_QUERY_PARAMS.actionPayload)?.json();
+	if (actionPayload) {
+		if (!isActionPayload(actionPayload)) {
+			throw new Error('Internal: Invalid action payload in cookie.');
+		}
+		return renderResult({ context, next, ...actionPayload });
+	}
 
 	// Heuristic: If body is null, Astro might've reset this for prerendering.
 	if (import.meta.env.DEV && request.method === 'POST' && request.body === null) {
@@ -37,7 +48,7 @@ export const onRequest = defineMiddleware(async (context, next) => {
 		return next();
 	}
 
-	const actionName = context.url.searchParams.get('_astroAction');
+	const actionName = context.url.searchParams.get(ACTION_QUERY_PARAMS.actionName);
 
 	if (context.request.method === 'POST' && actionName) {
 		return handlePost({ context, next, actionName });
@@ -50,6 +61,33 @@ export const onRequest = defineMiddleware(async (context, next) => {
 	return next();
 });
 
+async function renderResult({
+	context,
+	next,
+	actionResult,
+	actionName,
+}: {
+	context: APIContext;
+	next: MiddlewareNext;
+	actionResult: SerializedActionResult;
+	actionName: string;
+}) {
+	const locals = context.locals as Locals;
+
+	locals._actionPayload = { actionResult, actionName };
+	const response = await next();
+	context.cookies.delete(ACTION_QUERY_PARAMS.actionPayload);
+
+	if (actionResult.type === 'error') {
+		return new Response(response.body, {
+			status: actionResult.status,
+			statusText: actionResult.type,
+			headers: response.headers,
+		});
+	}
+	return response;
+}
+
 async function handlePost({
 	context,
 	next,
@@ -73,35 +111,42 @@ async function handlePost({
 	const action = baseAction.bind(context);
 	const actionResult = await action(formData);
 
-	return handleResult({ context, next, actionName, actionResult });
+	if (context.url.searchParams.get(ACTION_QUERY_PARAMS.actionRedirect) === 'false') {
+		return renderResult({
+			context,
+			next,
+			actionName,
+			actionResult: serializeActionResult(actionResult),
+		});
+	}
+
+	return redirectWithResult({ context, actionName, actionResult });
 }
 
-async function handleResult({
+async function redirectWithResult({
 	context,
-	next,
 	actionName,
 	actionResult,
 }: {
 	context: APIContext;
-	next: MiddlewareNext;
 	actionName: string;
 	actionResult: SafeResult<any, any>;
 }) {
-	const locals = context.locals as Locals;
-	locals._actionsInternal = {
+	context.cookies.set(ACTION_QUERY_PARAMS.actionPayload, {
 		actionName,
 		actionResult: serializeActionResult(actionResult),
-	};
+	});
 
-	const response = await next();
 	if (actionResult.error) {
-		return new Response(response.body, {
-			status: actionResult.error.status,
-			statusText: actionResult.error.type,
-			headers: response.headers,
-		});
+		const referer = context.request.headers.get('Referer');
+		if (!referer) {
+			throw new Error('Internal: Referer unexpectedly missing from Action POST request.');
+		}
+
+		return context.redirect(referer);
 	}
-	return response;
+
+	return context.redirect(context.url.pathname);
 }
 
 async function handlePostLegacy({ context, next }: { context: APIContext; next: MiddlewareNext }) {
@@ -120,7 +165,7 @@ async function handlePostLegacy({ context, next }: { context: APIContext; next:
 
 	if (!formData) return next();
 
-	const actionName = formData.get('_astroAction') as string;
+	const actionName = formData.get(ACTION_QUERY_PARAMS.actionName) as string;
 	if (!actionName) return next();
 
 	const baseAction = await getAction(actionName);
@@ -133,5 +178,13 @@ async function handlePostLegacy({ context, next }: { context: APIContext; next:
 
 	const action = baseAction.bind(context);
 	const actionResult = await action(formData);
-	return handleResult({ context, next, actionName, actionResult });
+	return redirectWithResult({ context, actionName, actionResult });
+}
+
+function isActionPayload(json: unknown): json is ActionPayload {
+	if (typeof json !== 'object' || json == null) return false;
+
+	if (!('actionResult' in json) || typeof json.actionResult !== 'object') return false;
+	if (!('actionName' in json) || typeof json.actionName !== 'string') return false;
+	return true;
 }

packages/astro/src/actions/runtime/virtual/shared.ts

@@ -1,6 +1,9 @@
 import { parse as devalueParse, stringify as devalueStringify } from 'devalue';
 import type { z } from 'zod';
 import type { ErrorInferenceObject, MaybePromise } from '../utils.js';
+import { ACTION_QUERY_PARAMS as _ACTION_QUERY_PARAMS } from '../../consts.js';
+
+export const ACTION_QUERY_PARAMS = _ACTION_QUERY_PARAMS;
 
 export const ACTION_ERROR_CODES = [
 	'BAD_REQUEST',
@@ -166,7 +169,7 @@ export async function callSafely<TOutput>(
 }
 
 export function getActionQueryString(name: string) {
-	const searchParams = new URLSearchParams({ _astroAction: name });
+	const searchParams = new URLSearchParams({ [_ACTION_QUERY_PARAMS.actionName]: name });
 	return `?${searchParams.toString()}`;
 }
 

packages/astro/src/actions/utils.ts

@@ -3,19 +3,19 @@ import type { Locals } from './runtime/middleware.js';
 import { type ActionAPIContext } from './runtime/utils.js';
 import { deserializeActionResult, getActionQueryString } from './runtime/virtual/shared.js';
 
-export function hasActionsInternal(locals: APIContext['locals']): locals is Locals {
-	return '_actionsInternal' in locals;
+export function hasActionPayload(locals: APIContext['locals']): locals is Locals {
+	return '_actionPayload' in locals;
 }
 
 export function createGetActionResult(locals: APIContext['locals']): APIContext['getActionResult'] {
 	return (actionFn): any => {
 		if (
-			!hasActionsInternal(locals) ||
-			actionFn.toString() !== getActionQueryString(locals._actionsInternal.actionName)
+			!hasActionPayload(locals) ||
+			actionFn.toString() !== getActionQueryString(locals._actionPayload.actionName)
 		) {
 			return undefined;
 		}
-		return deserializeActionResult(locals._actionsInternal.actionResult);
+		return deserializeActionResult(locals._actionPayload.actionResult);
 	};
 }
 

packages/astro/src/core/render-context.ts

@@ -11,7 +11,7 @@ import type {
 } from '../@types/astro.js';
 import type { ActionAPIContext } from '../actions/runtime/utils.js';
 import { deserializeActionResult } from '../actions/runtime/virtual/shared.js';
-import { createCallAction, createGetActionResult, hasActionsInternal } from '../actions/utils.js';
+import { createCallAction, createGetActionResult, hasActionPayload } from '../actions/utils.js';
 import {
 	computeCurrentLocale,
 	computePreferredLocale,
@@ -314,8 +314,8 @@ export class RenderContext {
 			},
 		} satisfies AstroGlobal['response'];
 
-		const actionResult = hasActionsInternal(this.locals)
-			? deserializeActionResult(this.locals._actionsInternal.actionResult)
+		const actionResult = hasActionPayload(this.locals)
+			? deserializeActionResult(this.locals._actionPayload.actionResult)
 			: undefined;
 
 		// Create the result object that will be passed into the renderPage function.

packages/astro/templates/actions.mjs

@@ -1,4 +1,9 @@
-import { ActionError, deserializeActionResult, getActionQueryString } from 'astro:actions';
+import {
+	ActionError,
+	deserializeActionResult,
+	getActionQueryString,
+	ACTION_QUERY_PARAMS,
+} from 'astro:actions';
 
 function toActionProxy(actionCallback = {}, aggregatedPath = '') {
 	return new Proxy(actionCallback, {
@@ -16,13 +21,18 @@ function toActionProxy(actionCallback = {}, aggregatedPath = '') {
 				toString: () => action.queryString,
 				// Progressive enhancement info for React.
 				$$FORM_ACTION: function () {
+					const searchParams = new URLSearchParams(action.toString());
+					// Astro will redirect with a GET request by default.
+					// Disable this behavior to preserve form state
+					// for React's progressive enhancement.
+					searchParams.set(ACTION_QUERY_PARAMS.actionRedirect, 'false');
 					return {
 						method: 'POST',
 						// `name` creates a hidden input.
 						// It's unused by Astro, but we can't turn this off.
 						// At least use a name that won't conflict with a user's formData.
 						name: '_astroAction',
-						action: action.toString(),
+						action: '?' + searchParams.toString(),
 					};
 				},
 				// Note: `orThrow` does not have progressive enhancement info.

packages/astro/test/actions.test.js

@@ -176,8 +176,11 @@ describe('Astro Actions', () => {
 			const req = new Request('http://example.com/user?_astroAction=getUser', {
 				method: 'POST',
 				body: new FormData(),
+				headers: {
+					Referer: 'http://example.com/user',
+				},
 			});
-			const res = await app.render(req);
+			const res = await followExpectedRedirect(req, app);
 			assert.equal(res.ok, true);
 
 			const html = await res.text();
@@ -189,9 +192,11 @@ describe('Astro Actions', () => {
 			const req = new Request('http://example.com/user-or-throw?_astroAction=getUserOrThrow', {
 				method: 'POST',
 				body: new FormData(),
+				headers: {
+					Referer: 'http://example.com/user-or-throw',
+				},
 			});
-			const res = await app.render(req);
-			assert.equal(res.ok, false);
+			const res = await followExpectedRedirect(req, app);
 			assert.equal(res.status, 401);
 
 			const html = await res.text();
@@ -219,8 +224,11 @@ describe('Astro Actions', () => {
 				const req = new Request('http://example.com/user', {
 					method: 'POST',
 					body: formData,
+					headers: {
+						Referer: 'http://example.com/user',
+					},
 				});
-				const res = await app.render(req);
+				const res = await followExpectedRedirect(req, app);
 				assert.equal(res.ok, true);
 
 				const html = await res.text();
@@ -234,9 +242,11 @@ describe('Astro Actions', () => {
 				const req = new Request('http://example.com/user-or-throw', {
 					method: 'POST',
 					body: formData,
+					headers: {
+						Referer: 'http://example.com/user-or-throw',
+					},
 				});
-				const res = await app.render(req);
-				assert.equal(res.ok, false);
+				const res = await followExpectedRedirect(req, app);
 				assert.equal(res.status, 401);
 
 				const html = await res.text();
@@ -337,3 +347,28 @@ describe('Astro Actions', () => {
 		});
 	});
 });
+
+const validRedirectStatuses = new Set([301, 302, 303, 304, 307, 308]);
+
+/**
+ * Follow an expected redirect response.
+ *
+ * @param {Request} req
+ * @param {*} app
+ * @returns {Promise<Response>}
+ */
+async function followExpectedRedirect(req, app) {
+	const redirect = await app.render(req, { addCookieHeader: true });
+	assert.ok(
+		validRedirectStatuses.has(redirect.status),
+		`Expected redirect status, got ${redirect.status}`,
+	);
+
+	const redirectUrl = new URL(redirect.headers.get('Location'), req.url);
+	const redirectReq = new Request(redirectUrl, {
+		headers: {
+			Cookie: redirect.headers.get('Set-Cookie'),
+		},
+	});
+	return app.render(redirectReq);
+}