diff options
| -rw-r--r-- | .changeset/rotten-months-report.md | 27 | ||||
| -rw-r--r-- | packages/astro/astro-4.13.1.tgz | bin | 4623929 -> 0 bytes | |||
| -rw-r--r-- | packages/astro/src/actions/consts.ts | 6 | ||||
| -rw-r--r-- | packages/astro/src/actions/runtime/middleware.ts | 99 | ||||
| -rw-r--r-- | packages/astro/src/actions/runtime/virtual/shared.ts | 5 | ||||
| -rw-r--r-- | packages/astro/src/actions/utils.ts | 10 | ||||
| -rw-r--r-- | packages/astro/src/core/render-context.ts | 6 | ||||
| -rw-r--r-- | packages/astro/templates/actions.mjs | 14 | ||||
| -rw-r--r-- | packages/astro/test/actions.test.js | 47 |
9 files changed, 174 insertions, 40 deletions
diff --git a/.changeset/rotten-months-report.md b/.changeset/rotten-months-report.md new file mode 100644 index 000000000..c55fc6171 --- /dev/null +++ b/.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. diff --git a/packages/astro/astro-4.13.1.tgz b/packages/astro/astro-4.13.1.tgz Binary files differdeleted file mode 100644 index ec456aa56..000000000 --- a/packages/astro/astro-4.13.1.tgz +++ /dev/null diff --git a/packages/astro/src/actions/consts.ts b/packages/astro/src/actions/consts.ts index e1324f248..6a55386d8 100644 --- a/packages/astro/src/actions/consts.ts +++ b/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', +}; diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts index f589382de..5e13cfdc9 100644 --- a/packages/astro/src/actions/runtime/middleware.ts +++ b/packages/astro/src/actions/runtime/middleware.ts @@ -10,12 +10,15 @@ 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) => { @@ -23,9 +26,17 @@ export const onRequest = defineMiddleware(async (context, next) => { 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; } diff --git a/packages/astro/src/actions/runtime/virtual/shared.ts b/packages/astro/src/actions/runtime/virtual/shared.ts index 962a0bec8..1bbeed670 100644 --- a/packages/astro/src/actions/runtime/virtual/shared.ts +++ b/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()}`; } diff --git a/packages/astro/src/actions/utils.ts b/packages/astro/src/actions/utils.ts index b08146e8a..5cf786266 100644 --- a/packages/astro/src/actions/utils.ts +++ b/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); }; } diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 97e1b25d9..a57221574 100644 --- a/packages/astro/src/core/render-context.ts +++ b/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. diff --git a/packages/astro/templates/actions.mjs b/packages/astro/templates/actions.mjs index f38ba3fa9..16e94291e 100644 --- a/packages/astro/templates/actions.mjs +++ b/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. diff --git a/packages/astro/test/actions.test.js b/packages/astro/test/actions.test.js index 3623d86be..341e7c8d6 100644 --- a/packages/astro/test/actions.test.js +++ b/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); +} |