From 84189b6511dc2a14bcfe608696f56a64c2046f39 Mon Sep 17 00:00:00 2001
From: Ben Holmes <hey@bholmes.dev>
Date: Tue, 30 Jul 2024 11:04:10 -0400
Subject: [PATCH] Actions: New fallback behavior with `action={actions.name}`
 (#11570)

* feat: support _astroAction query param

* feat(test): _astroAction query param

* fix: handle _actions requests from legacy fallback

* feat(e2e): new actions pattern on blog test

* feat: update React 19 adapter to use query params

* fix: remove legacy getApiContext()

* feat: ActionQueryStringInvalidError

* fix: update error description

* feat: ActionQueryStringInvalidError

* chore: comment on _actions skip

* feat: .queryString property

* chore: comment on throw new Error

* chore: better guess for "why" on query string

* chore: remove console log

* chore: changeset

* chore: changeset
---
 .changeset/silly-bulldogs-sparkle.md          |  53 ++++++++
 .../src/pages/blog/[...slug].astro            |   5 +-
 .../actions-react-19/src/actions/index.ts     |   5 +-
 .../astro/src/actions/runtime/middleware.ts   | 124 ++++++++++++++----
 .../src/actions/runtime/virtual/server.ts     |   5 +-
 .../src/actions/runtime/virtual/shared.ts     |  19 ++-
 packages/astro/src/core/errors/errors-data.ts |  30 +++++
 packages/astro/templates/actions.mjs          |  27 ++--
 packages/astro/test/actions.test.js           |  48 +++++--
 packages/integrations/react/server.js         |   7 +-
 packages/integrations/react/src/actions.ts    |   5 +-
 11 files changed, 269 insertions(+), 59 deletions(-)
 create mode 100644 .changeset/silly-bulldogs-sparkle.md

diff --git a/.changeset/silly-bulldogs-sparkle.md b/.changeset/silly-bulldogs-sparkle.md
new file mode 100644
index 000000000000..9b23a675fa53
--- /dev/null
+++ b/.changeset/silly-bulldogs-sparkle.md
@@ -0,0 +1,53 @@
+---
+'@astrojs/react': patch
+'astro': patch
+---
+
+**BREAKING CHANGE to the experimental Actions API only.** Install the latest `@astrojs/react` integration as well if you're using React 19 features.
+
+Updates the Astro Actions fallback to support `action={actions.name}` instead of using `getActionProps().` This will submit a form to the server in zero-JS scenarios using a search parameter:
+
+```astro
+---
+import { actions } from 'astro:actions';
+---
+
+<form action={actions.logOut}>
+<!--output: action="?_astroAction=logOut"-->
+  <button>Log Out</button>
+</form>
+```
+
+You may also construct form action URLs using string concatenation, or by using the `URL()` constructor, with the an action's `.queryString` property:
+
+```astro
+---
+import { actions } from 'astro:actions';
+
+const confirmationUrl = new URL('/confirmation', Astro.url);
+confirmationUrl.search = actions.queryString;
+---
+
+<form method="POST" action={confirmationUrl.pathname}>
+  <button>Submit</button>
+</form>
+```
+
+## Migration
+
+`getActionProps()` is now deprecated. To use the new fallback pattern, remove the `getActionProps()` input from your form and pass your action function to the form `action` attribute:
+
+```diff
+---
+import {
+  actions,
+- getActionProps,  
+} from 'astro:actions';
+---
+
++ <form method="POST" action={actions.logOut}>
+- <form method="POST">
+- <input {...getActionProps(actions.logOut)} />
+  <button>Log Out</button>
+</form>
+```
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro b/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
index e864c36101be..64deb7a46b43 100644
--- a/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
+++ b/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
@@ -4,7 +4,7 @@ import BlogPost from '../../layouts/BlogPost.astro';
 import { db, eq, Comment, Likes } from 'astro:db';
 import { Like } from '../../components/Like';
 import { PostComment } from '../../components/PostComment';
-import { actions, getActionProps } from 'astro:actions';
+import { actions } from 'astro:actions';
 import { isInputError } from 'astro:actions';
 
 export const prerender = false;
@@ -55,8 +55,7 @@ const commentPostIdOverride = Astro.url.searchParams.get('commentPostIdOverride'
 			: undefined}
 		client:load
 	/>
-	<form method="POST" data-testid="progressive-fallback">
-		<input {...getActionProps(actions.blog.comment)} />
+	<form method="POST" data-testid="progressive-fallback" action={actions.blog.comment.queryString}>
 		<input type="hidden" name="postId" value={post.id} />
 		<label for="fallback-author">
 			Author
diff --git a/packages/astro/e2e/fixtures/actions-react-19/src/actions/index.ts b/packages/astro/e2e/fixtures/actions-react-19/src/actions/index.ts
index 9cb867603add..39f9dcf9aab8 100644
--- a/packages/astro/e2e/fixtures/actions-react-19/src/actions/index.ts
+++ b/packages/astro/e2e/fixtures/actions-react-19/src/actions/index.ts
@@ -25,11 +25,10 @@ export const server = {
 		likeWithActionState: defineAction({
 			accept: 'form',
 			input: z.object({ postId: z.string() }),
-			handler: async ({ postId }) => {
+			handler: async ({ postId }, ctx) => {
 				await new Promise((r) => setTimeout(r, 200));
 
-				const context = getApiContext();
-				const state = await experimental_getActionState<number>(context);
+				const state = await experimental_getActionState<number>(ctx);
 
 				const { likes } = await db
 					.update(Likes)
diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts
index b70da4f65d25..b6b6ced0e0ff 100644
--- a/packages/astro/src/actions/runtime/middleware.ts
+++ b/packages/astro/src/actions/runtime/middleware.ts
@@ -3,7 +3,12 @@ import type { APIContext, MiddlewareNext } from '../../@types/astro.js';
 import { defineMiddleware } from '../../core/middleware/index.js';
 import { ApiContextStorage } from './store.js';
 import { formContentTypes, getAction, hasContentType } from './utils.js';
-import { callSafely } from './virtual/shared.js';
+import { callSafely, getActionQueryString } from './virtual/shared.js';
+import { AstroError } from '../../core/errors/errors.js';
+import {
+	ActionQueryStringInvalidError,
+	ActionsUsedWithForGetError,
+} from '../../core/errors/errors-data.js';
 
 export type Locals = {
 	_actionsInternal: {
@@ -14,62 +19,129 @@ export type Locals = {
 
 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,
 	// so short circuit if already defined.
 	if (locals._actionsInternal) return ApiContextStorage.run(context, () => next());
-	if (context.request.method === 'GET') {
-		return nextWithLocalsStub(next, context);
-	}
 
 	// Heuristic: If body is null, Astro might've reset this for prerendering.
 	// Stub with warning when `getActionResult()` is used.
-	if (context.request.method === 'POST' && context.request.body === null) {
+	if (request.method === 'POST' && request.body === null) {
 		return nextWithStaticStub(next, context);
 	}
 
-	const { request, url } = context;
-	const contentType = request.headers.get('Content-Type');
+	const actionName = context.url.searchParams.get('_astroAction');
+
+	if (context.request.method === 'POST' && actionName) {
+		return handlePost({ context, next, actionName });
+	}
 
-	// Avoid double-handling with middleware when calling actions directly.
-	if (url.pathname.startsWith('/_actions')) return nextWithLocalsStub(next, context);
+	if (context.request.method === 'GET' && actionName) {
+		throw new AstroError({
+			...ActionsUsedWithForGetError,
+			message: ActionsUsedWithForGetError.message(actionName),
+		});
+	}
 
-	if (!contentType || !hasContentType(contentType, formContentTypes)) {
-		return nextWithLocalsStub(next, context);
+	if (context.request.method === 'POST') {
+		return handlePostLegacy({ context, next });
 	}
 
-	const formData = await request.clone().formData();
-	const actionPath = formData.get('_astroAction');
-	if (typeof actionPath !== 'string') return nextWithLocalsStub(next, context);
+	return nextWithLocalsStub(next, context);
+});
+
+async function handlePost({
+	context,
+	next,
+	actionName,
+}: { context: APIContext; next: MiddlewareNext; actionName: string }) {
+	const { request } = context;
 
-	const action = await getAction(actionPath);
-	if (!action) return nextWithLocalsStub(next, context);
+	const action = await getAction(actionName);
+	if (!action) {
+		throw new AstroError({
+			...ActionQueryStringInvalidError,
+			message: ActionQueryStringInvalidError.message(actionName),
+		});
+	}
+
+	const contentType = request.headers.get('content-type');
+	let formData: FormData | undefined;
+	if (contentType && hasContentType(contentType, formContentTypes)) {
+		formData = await request.clone().formData();
+	}
+	const actionResult = await ApiContextStorage.run(context, () =>
+		callSafely(() => action(formData))
+	);
 
-	const result = await ApiContextStorage.run(context, () => callSafely(() => action(formData)));
+	return handleResult({ context, next, actionName, actionResult });
+}
 
+function handleResult({
+	context,
+	next,
+	actionName,
+	actionResult,
+}: { context: APIContext; next: MiddlewareNext; actionName: string; actionResult: any }) {
 	const actionsInternal: Locals['_actionsInternal'] = {
 		getActionResult: (actionFn) => {
-			if (actionFn.toString() !== actionPath) return Promise.resolve(undefined);
-			// The `action` uses type `unknown` since we can't infer the user's action type.
-			// Cast to `any` to satisfy `getActionResult()` type.
-			return result as any;
+			if (actionFn.toString() !== getActionQueryString(actionName)) {
+				return Promise.resolve(undefined);
+			}
+			return actionResult;
 		},
-		actionResult: result,
+		actionResult,
 	};
+	const locals = context.locals as Locals;
 	Object.defineProperty(locals, '_actionsInternal', { writable: false, value: actionsInternal });
+
 	return ApiContextStorage.run(context, async () => {
 		const response = await next();
-		if (result.error) {
+		if (actionResult.error) {
 			return new Response(response.body, {
-				status: result.error.status,
-				statusText: result.error.name,
+				status: actionResult.error.status,
+				statusText: actionResult.error.type,
 				headers: response.headers,
 			});
 		}
 		return response;
 	});
-});
+}
+
+async function handlePostLegacy({ context, next }: { context: APIContext; next: MiddlewareNext }) {
+	const { request } = context;
+
+	// We should not run a middleware handler for fetch()
+	// requests directly to the /_actions URL.
+	// Otherwise, we may handle the result twice.
+	if (context.url.pathname.startsWith('/_actions')) return nextWithLocalsStub(next, context);
+
+	const contentType = request.headers.get('content-type');
+	let formData: FormData | undefined;
+	if (contentType && hasContentType(contentType, formContentTypes)) {
+		formData = await request.clone().formData();
+	}
+
+	if (!formData) return nextWithLocalsStub(next, context);
+
+	const actionName = formData.get('_astroAction') as string;
+	if (!actionName) return nextWithLocalsStub(next, context);
+
+	const action = await getAction(actionName);
+	if (!action) {
+		throw new AstroError({
+			...ActionQueryStringInvalidError,
+			message: ActionQueryStringInvalidError.message(actionName),
+		});
+	}
+
+	const actionResult = await ApiContextStorage.run(context, () =>
+		callSafely(() => action(formData))
+	);
+	return handleResult({ context, next, actionName, actionResult });
+}
 
 function nextWithStaticStub(next: MiddlewareNext, context: APIContext) {
 	Object.defineProperty(context.locals, '_actionsInternal', {
diff --git a/packages/astro/src/actions/runtime/virtual/server.ts b/packages/astro/src/actions/runtime/virtual/server.ts
index 3efa7ca149a0..93b3f54e1137 100644
--- a/packages/astro/src/actions/runtime/virtual/server.ts
+++ b/packages/astro/src/actions/runtime/virtual/server.ts
@@ -29,6 +29,7 @@ export type ActionClient<
 	? ((
 			input: TAccept extends 'form' ? FormData : z.input<TInputSchema>
 		) => Promise<Awaited<TOutput>>) & {
+			queryString: string;
 			safe: (
 				input: TAccept extends 'form' ? FormData : z.input<TInputSchema>
 			) => Promise<
@@ -59,7 +60,7 @@ export function defineAction<
 	input?: TInputSchema;
 	accept?: TAccept;
 	handler: ActionHandler<TInputSchema, TOutput>;
-}): ActionClient<TOutput, TAccept, TInputSchema> {
+}): ActionClient<TOutput, TAccept, TInputSchema> & string {
 	const serverHandler =
 		accept === 'form'
 			? getFormServerHandler(handler, inputSchema)
@@ -70,7 +71,7 @@ export function defineAction<
 			return callSafely(() => serverHandler(unparsedInput));
 		},
 	});
-	return serverHandler as ActionClient<TOutput, TAccept, TInputSchema>;
+	return serverHandler as ActionClient<TOutput, TAccept, TInputSchema> & string;
 }
 
 function getFormServerHandler<TOutput, TInputSchema extends ActionInputSchema<'form'>>(
diff --git a/packages/astro/src/actions/runtime/virtual/shared.ts b/packages/astro/src/actions/runtime/virtual/shared.ts
index 5c18828a60ac..98f75025a49a 100644
--- a/packages/astro/src/actions/runtime/virtual/shared.ts
+++ b/packages/astro/src/actions/runtime/virtual/shared.ts
@@ -154,10 +154,27 @@ export async function callSafely<TOutput>(
 	}
 }
 
+export function getActionQueryString(name: string) {
+	const searchParams = new URLSearchParams({ _astroAction: name });
+	return `?${searchParams.toString()}`;
+}
+
+/**
+ * @deprecated You can now pass action functions
+ * directly to the `action` attribute on a form.
+ *
+ * Example: `<form action={actions.like} />`
+ */
 export function getActionProps<T extends (args: FormData) => MaybePromise<unknown>>(action: T) {
+	const params = new URLSearchParams(action.toString());
+	const actionName = params.get('_astroAction');
+	if (!actionName) {
+		// No need for AstroError. `getActionProps()` will be removed for stable.
+		throw new Error('Invalid actions function was passed to getActionProps()');
+	}
 	return {
 		type: 'hidden',
 		name: '_astroAction',
-		value: action.toString(),
+		value: actionName,
 	} as const;
 }
diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts
index 8b586e6d2cbe..3da4c312e412 100644
--- a/packages/astro/src/core/errors/errors-data.ts
+++ b/packages/astro/src/core/errors/errors-data.ts
@@ -1617,6 +1617,36 @@ export const ActionsWithoutServerOutputError = {
 	hint: 'Learn about on-demand rendering: https://docs.astro.build/en/basics/rendering-modes/#on-demand-rendered',
 } satisfies ErrorData;
 
+/**
+ * @docs
+ * @see
+ * - [Actions RFC](https://github.com/withastro/roadmap/blob/actions/proposals/0046-actions.md)
+ * @description
+ * Action was called from a form using a GET request, but only POST requests are supported. This often occurs if `method="POST"` is missing on the form.
+ */
+export const ActionsUsedWithForGetError = {
+	name: 'ActionsUsedWithForGetError',
+	title: 'An invalid Action query string was passed by a form.',
+	message: (actionName: string) =>
+		`Action ${actionName} was called from a form using a GET request, but only POST requests are supported. This often occurs if \`method="POST"\` is missing on the form.`,
+	hint: 'Actions are experimental. Visit the RFC for usage instructions: https://github.com/withastro/roadmap/blob/actions/proposals/0046-actions.md',
+} satisfies ErrorData;
+
+/**
+ * @docs
+ * @see
+ * - [Actions RFC](https://github.com/withastro/roadmap/blob/actions/proposals/0046-actions.md)
+ * @description
+ * The server received the query string `?_astroAction=name`, but could not find an action with that name. Use the action function's `.queryString` property to retrieve the form `action` URL.
+ */
+export const ActionQueryStringInvalidError = {
+	name: 'ActionQueryStringInvalidError',
+	title: 'An invalid Action query string was passed by a form.',
+	message: (actionName: string) =>
+		`The server received the query string \`?_astroAction=${actionName}\`, but could not find an action with that name. If you changed an action's name in development, remove this query param from your URL and refresh.`,
+	hint: 'Actions are experimental. Visit the RFC for usage instructions: https://github.com/withastro/roadmap/blob/actions/proposals/0046-actions.md',
+} satisfies ErrorData;
+
 /**
  * @docs
  * @see
diff --git a/packages/astro/templates/actions.mjs b/packages/astro/templates/actions.mjs
index 52906beada19..1101c136181b 100644
--- a/packages/astro/templates/actions.mjs
+++ b/packages/astro/templates/actions.mjs
@@ -1,36 +1,39 @@
-import { ActionError, callSafely } from 'astro:actions';
+import { ActionError, callSafely, getActionQueryString } from 'astro:actions';
 
-function toActionProxy(actionCallback = {}, aggregatedPath = '/_actions/') {
+function toActionProxy(actionCallback = {}, aggregatedPath = '') {
 	return new Proxy(actionCallback, {
 		get(target, objKey) {
-			if (objKey in target) {
+			if (objKey in target || typeof objKey === 'symbol') {
 				return target[objKey];
 			}
 			const path = aggregatedPath + objKey.toString();
 			const action = (param) => actionHandler(param, path);
-			action.toString = () => path;
+
+			action.toString = () => getActionQueryString(path);
+			action.queryString = action.toString();
 			action.safe = (input) => {
 				return callSafely(() => action(input));
 			};
-			action.safe.toString = () => path;
+			action.safe.toString = () => action.toString();
 
 			// Add progressive enhancement info for React.
 			action.$$FORM_ACTION = function () {
-				const data = new FormData();
-				data.set('_astroAction', action.toString());
 				return {
 					method: 'POST',
-					name: action.toString(),
-					data,
+					// `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.safe.$$FORM_ACTION = function () {
 				const data = new FormData();
-				data.set('_astroAction', action.toString());
 				data.set('_astroActionSafe', 'true');
 				return {
 					method: 'POST',
-					name: action.toString(),
+					name: '_astroAction',
+					action: action.toString(),
 					data,
 				};
 			};
@@ -72,7 +75,7 @@ async function actionHandler(param, path) {
 		headers.set('Content-Type', 'application/json');
 		headers.set('Content-Length', body?.length.toString() ?? '0');
 	}
-	const res = await fetch(path, {
+	const res = await fetch(`/_actions/${path}`, {
 		method: 'POST',
 		body,
 		headers,
diff --git a/packages/astro/test/actions.test.js b/packages/astro/test/actions.test.js
index a4322b186d0c..f1ab92202c6d 100644
--- a/packages/astro/test/actions.test.js
+++ b/packages/astro/test/actions.test.js
@@ -174,12 +174,10 @@ describe('Astro Actions', () => {
 			assert.equal(json.isFormData, true, 'Should receive plain FormData');
 		});
 
-		it('Respects user middleware', async () => {
-			const formData = new FormData();
-			formData.append('_astroAction', '/_actions/getUser');
-			const req = new Request('http://example.com/user', {
+		it('Response middleware fallback', async () => {
+			const req = new Request('http://example.com/user?_astroAction=getUser', {
 				method: 'POST',
-				body: formData,
+				body: new FormData(),
 			});
 			const res = await app.render(req);
 			assert.equal(res.ok, true);
@@ -190,11 +188,9 @@ describe('Astro Actions', () => {
 		});
 
 		it('Respects custom errors', async () => {
-			const formData = new FormData();
-			formData.append('_astroAction', '/_actions/getUserOrThrow');
-			const req = new Request('http://example.com/user-or-throw', {
+			const req = new Request('http://example.com/user-or-throw?_astroAction=getUserOrThrow', {
 				method: 'POST',
-				body: formData,
+				body: new FormData(),
 			});
 			const res = await app.render(req);
 			assert.equal(res.ok, false);
@@ -206,6 +202,40 @@ describe('Astro Actions', () => {
 			assert.equal($('#error-code').text(), 'UNAUTHORIZED');
 		});
 
+		describe('legacy', () => {
+			it('Response middleware fallback', async () => {
+				const formData = new FormData();
+				formData.append('_astroAction', 'getUser');
+				const req = new Request('http://example.com/user', {
+					method: 'POST',
+					body: formData,
+				});
+				const res = await app.render(req);
+				assert.equal(res.ok, true);
+
+				const html = await res.text();
+				let $ = cheerio.load(html);
+				assert.equal($('#user').text(), 'Houston');
+			});
+
+			it('Respects custom errors', async () => {
+				const formData = new FormData();
+				formData.append('_astroAction', 'getUserOrThrow');
+				const req = new Request('http://example.com/user-or-throw', {
+					method: 'POST',
+					body: formData,
+				});
+				const res = await app.render(req);
+				assert.equal(res.ok, false);
+				assert.equal(res.status, 401);
+
+				const html = await res.text();
+				let $ = cheerio.load(html);
+				assert.equal($('#error-message').text(), 'Not logged in');
+				assert.equal($('#error-code').text(), 'UNAUTHORIZED');
+			});
+		});
+
 		it('Sets status to 204 when no content', async () => {
 			const req = new Request('http://example.com/_actions/fireAndForget', {
 				method: 'POST',
diff --git a/packages/integrations/react/server.js b/packages/integrations/react/server.js
index 59134a6993b2..6624a5610f00 100644
--- a/packages/integrations/react/server.js
+++ b/packages/integrations/react/server.js
@@ -131,6 +131,7 @@ async function getFormState({ result }) {
 	if (!actionResult) return undefined;
 	if (!isFormRequest(request.headers.get('content-type'))) return undefined;
 
+	const { searchParams } = new URL(request.url);
 	const formData = await request.clone().formData();
 	/**
 	 * The key generated by React to identify each `useActionState()` call.
@@ -142,7 +143,11 @@ async function getFormState({ result }) {
 	 * This matches the endpoint path.
 	 * @example "/_actions/blog.like"
 	 */
-	const actionName = formData.get('_astroAction')?.toString();
+	const actionName =
+		searchParams.get('_astroAction') ??
+		/* Legacy. TODO: remove for stable */ formData
+			.get('_astroAction')
+			?.toString();
 
 	if (!actionKey || !actionName) return undefined;
 
diff --git a/packages/integrations/react/src/actions.ts b/packages/integrations/react/src/actions.ts
index 336d322209ab..bc45e28ee829 100644
--- a/packages/integrations/react/src/actions.ts
+++ b/packages/integrations/react/src/actions.ts
@@ -25,8 +25,9 @@ export function experimental_withState<T>(action: FormFn<T>) {
 	callback.$$FORM_ACTION = action.$$FORM_ACTION;
 	// Called by React when form state is passed from the server.
 	// If the action names match, React returns this state from `useActionState()`.
-	callback.$$IS_SIGNATURE_EQUAL = (actionName: string) => {
-		return action.toString() === actionName;
+	callback.$$IS_SIGNATURE_EQUAL = (incomingActionName: string) => {
+		const actionName = new URLSearchParams(action.toString()).get('_astroAction');
+		return actionName === incomingActionName;
 	};
 
 	// React calls `.bind()` internally to pass the initial state value.