Skip to content

Commit

Permalink
Actions: Make .safe() the default return value (#11571)
Browse files Browse the repository at this point in the history
* feat: new orThrow types

* fix: parens on return type

* feat: switch implementation to orThrow()

* feat(e2e): update PostComment

* fix: remove callSafely from middleware

* fix: toString() for actions

* fix(e2e): more orThrow updates

* feat: remove progressive enhancement from orThrow

* fix: remove _astroActionSafe handler from react

* feat(e2e): update test to use safe calling

* chore: console log

* chore: unused import

* fix: add rewriting: true to test fixture

* fix: correctly throw for server-only actions

* chore: changeset

* fix: update type tests

* fix(test): remove .safe() chain

* docs: use "patch" with BREAKING CHANGE notice

* docs: clarify react integration in changeset
  • Loading branch information
bholmesdev authored Jul 30, 2024
1 parent a77ed84 commit 1c3265a
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 83 deletions.
29 changes: 29 additions & 0 deletions .changeset/light-chairs-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'@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.

Make `.safe()` the default return value for actions. This means `{ data, error }` will be returned when calling an action directly. If you prefer to get the data while allowing errors to throw, chain the `.orThrow()` modifier.

```ts
import { actions } from 'astro:actions';

// Before
const { data, error } = await actions.like.safe();
// After
const { data, error } = await actions.like();

// Before
const newLikes = await actions.like();
// After
const newLikes = await actions.like.orThrow();
```

## Migration

To migrate your existing action calls:

- Remove `.safe` from existing _safe_ action calls
- Add `.orThrow` to existing _unsafe_ action calls
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function Like({ postId, initial }: { postId: string; initial: number }) {
disabled={pending}
onClick={async () => {
setPending(true);
setLikes(await actions.blog.like({ postId }));
setLikes(await actions.blog.like.orThrow({ postId }));
setPending(false);
}}
type="submit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function PostComment({
e.preventDefault();
const form = e.target as HTMLFormElement;
const formData = new FormData(form);
const { data, error } = await actions.blog.comment.safe(formData);
const { data, error } = await actions.blog.comment(formData);
if (isInputError(error)) {
return setBodyError(error.fields.body?.join(' '));
} else if (error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { db, Likes, eq, sql } from 'astro:db';
import { defineAction, getApiContext, z } from 'astro:actions';
import { defineAction, z, type SafeResult } from 'astro:actions';
import { experimental_getActionState } from '@astrojs/react/actions';

export const server = {
Expand Down Expand Up @@ -28,12 +28,13 @@ export const server = {
handler: async ({ postId }, ctx) => {
await new Promise((r) => setTimeout(r, 200));

const state = await experimental_getActionState<number>(ctx);
const state = await experimental_getActionState<SafeResult<any, number>>(ctx);
const previousLikes = state.data ?? 0;

const { likes } = await db
.update(Likes)
.set({
likes: state + 1,
likes: previousLikes + 1,
})
.where(eq(Likes.postId, postId))
.returning()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ export function Like({ postId, label, likes }: { postId: string; label: string;
export function LikeWithActionState({ postId, label, likes: initial }: { postId: string; label: string; likes: number }) {
const [likes, action] = useActionState(
experimental_withState(actions.blog.likeWithActionState),
10,
{ data: initial },
);

return (
<form action={action}>
<input type="hidden" name="postId" value={postId} />
<Button likes={likes} label={label} />
<Button likes={likes.data} label={label} />
</form>
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2845,7 +2845,7 @@ interface AstroSharedContext<
TAction extends ActionClient<unknown, TAccept, TInputSchema>,
>(
action: TAction
) => Awaited<ReturnType<TAction['safe']>> | undefined;
) => Awaited<ReturnType<TAction>> | undefined;
/**
* Route parameters for this request if this is a dynamic route.
*/
Expand Down
10 changes: 3 additions & 7 deletions packages/astro/src/actions/runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { AstroError } from '../../core/errors/errors.js';
import { defineMiddleware } from '../../core/middleware/index.js';
import { ApiContextStorage } from './store.js';
import { formContentTypes, getAction, hasContentType } from './utils.js';
import { callSafely, getActionQueryString } from './virtual/shared.js';
import { getActionQueryString } from './virtual/shared.js';

export type Locals = {
_actionsInternal: {
Expand Down Expand Up @@ -72,9 +72,7 @@ async function handlePost({
if (contentType && hasContentType(contentType, formContentTypes)) {
formData = await request.clone().formData();
}
const actionResult = await ApiContextStorage.run(context, () =>
callSafely(() => action(formData))
);
const actionResult = await ApiContextStorage.run(context, () => action(formData));

return handleResult({ context, next, actionName, actionResult });
}
Expand Down Expand Up @@ -137,9 +135,7 @@ async function handlePostLegacy({ context, next }: { context: APIContext; next:
});
}

const actionResult = await ApiContextStorage.run(context, () =>
callSafely(() => action(formData))
);
const actionResult = await ApiContextStorage.run(context, () => action(formData));
return handleResult({ context, next, actionName, actionResult });
}

Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/actions/runtime/route.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { APIRoute } from '../../@types/astro.js';
import { ApiContextStorage } from './store.js';
import { formContentTypes, getAction, hasContentType } from './utils.js';
import { callSafely } from './virtual/shared.js';

export const POST: APIRoute = async (context) => {
const { request, url } = context;
Expand All @@ -23,7 +22,7 @@ export const POST: APIRoute = async (context) => {
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/415
return new Response(null, { status: 415 });
}
const result = await ApiContextStorage.run(context, () => callSafely(() => action(args)));
const result = await ApiContextStorage.run(context, () => action(args));
if (result.error) {
return new Response(
JSON.stringify({
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/src/actions/runtime/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import type { ZodType } from 'zod';
import type { ActionAccept, ActionClient } from './virtual/server.js';

export const formContentTypes = ['application/x-www-form-urlencoded', 'multipart/form-data'];

export function hasContentType(contentType: string, expected: string[]) {
Expand All @@ -17,7 +20,7 @@ export type MaybePromise<T> = T | Promise<T>;
*/
export async function getAction(
path: string
): Promise<((param: unknown) => MaybePromise<unknown>) | undefined> {
): Promise<ActionClient<unknown, ActionAccept, ZodType> | undefined> {
const pathKeys = path.replace('/_actions/', '').split('.');
// @ts-expect-error virtual module
let { server: actionLookup } = await import('astro:internal-actions');
Expand Down
37 changes: 20 additions & 17 deletions packages/astro/src/actions/runtime/virtual/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,21 @@ export type ActionClient<
> = TInputSchema extends z.ZodType
? ((
input: TAccept extends 'form' ? FormData : z.input<TInputSchema>
) => Promise<Awaited<TOutput>>) & {
) => Promise<
SafeResult<
z.input<TInputSchema> extends ErrorInferenceObject
? z.input<TInputSchema>
: ErrorInferenceObject,
Awaited<TOutput>
>
>) & {
queryString: string;
safe: (
orThrow: (
input: TAccept extends 'form' ? FormData : z.input<TInputSchema>
) => Promise<
SafeResult<
z.input<TInputSchema> extends ErrorInferenceObject
? z.input<TInputSchema>
: ErrorInferenceObject,
Awaited<TOutput>
>
>;
) => Promise<Awaited<TOutput>>;
}
: ((input?: any) => Promise<Awaited<TOutput>>) & {
safe: (input?: any) => Promise<SafeResult<never, Awaited<TOutput>>>;
: (input?: any) => Promise<SafeResult<never, Awaited<TOutput>>> & {
orThrow: (input?: any) => Promise<Awaited<TOutput>>;
};

export function defineAction<
Expand All @@ -66,12 +66,15 @@ export function defineAction<
? getFormServerHandler(handler, inputSchema)
: getJsonServerHandler(handler, inputSchema);

Object.assign(serverHandler, {
safe: async (unparsedInput: unknown) => {
return callSafely(() => serverHandler(unparsedInput));
},
const safeServerHandler = async (unparsedInput: unknown) => {
return callSafely(() => serverHandler(unparsedInput));
};

Object.assign(safeServerHandler, {
orThrow: serverHandler,
});
return serverHandler as ActionClient<TOutput, TAccept, TInputSchema> & string;

return safeServerHandler as ActionClient<TOutput, TAccept, TInputSchema> & string;
}

function getFormServerHandler<TOutput, TInputSchema extends ActionInputSchema<'form'>>(
Expand Down
54 changes: 24 additions & 30 deletions packages/astro/templates/actions.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,30 @@ function toActionProxy(actionCallback = {}, aggregatedPath = '') {
return target[objKey];
}
const path = aggregatedPath + objKey.toString();
const action = (param) => actionHandler(param, path);
const action = (param) => callSafely(() => handleActionOrThrow(param, path));

action.toString = () => getActionQueryString(path);
action.queryString = action.toString();
action.safe = (input) => {
return callSafely(() => action(input));
};
action.safe.toString = () => action.toString();
Object.assign(action, {
queryString: getActionQueryString(path),
toString: () => action.queryString,
// Progressive enhancement info for React.
$$FORM_ACTION: function () {
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(),
};
},
// Note: `orThrow` does not have progressive enhancement info.
// If you want to throw exceptions,
// you must handle those exceptions with client JS.
orThrow: (param) => {
return handleActionOrThrow(param, path);
},
});

// Add progressive enhancement info for React.
action.$$FORM_ACTION = function () {
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.safe.$$FORM_ACTION = function () {
const data = new FormData();
data.set('_astroActionSafe', 'true');
return {
method: 'POST',
name: '_astroAction',
action: action.toString(),
data,
};
};
// recurse to construct queries for nested object paths
// ex. actions.user.admins.auth()
return toActionProxy(action, path + '.');
Expand All @@ -49,14 +43,14 @@ function toActionProxy(actionCallback = {}, aggregatedPath = '') {
* @param {string} path Built path to call action by path name.
* Usage: `actions.[name](param)`.
*/
async function actionHandler(param, path) {
async function handleActionOrThrow(param, path) {
// When running server-side, import the action and call it.
if (import.meta.env.SSR) {
const { getAction } = await import('astro/actions/runtime/utils.js');
const action = await getAction(path);
if (!action) throw new Error(`Action not found: ${path}`);

return action(param);
return action.orThrow(param);
}

// When running client-side, make a fetch request to the action path.
Expand Down
1 change: 1 addition & 0 deletions packages/astro/test/fixtures/actions/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { defineConfig } from 'astro/config';
export default defineConfig({
output: 'server',
experimental: {
rewriting: true,
actions: true,
},
});
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
import { actions } from 'astro:actions';
const { url, channel } = await actions.subscribeFromServer({
const res = await actions.subscribeFromServer.orThrow({
channel: 'bholmesdev',
});
---

<p data-url>{url}</p>
<p data-channel>{channel}</p>
<p data-url>{res.url}</p>
<p data-channel>{res.channel}</p>

11 changes: 9 additions & 2 deletions packages/astro/test/types/action-return-type.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { describe, it } from 'node:test';
import { expectTypeOf } from 'expect-type';
import { type ActionReturnType, defineAction } from '../../dist/actions/runtime/virtual/server.js';
import {
type SafeResult,
type ActionReturnType,
defineAction,
} from '../../dist/actions/runtime/virtual/server.js';
import { z } from '../../zod.mjs';

describe('ActionReturnType', () => {
Expand All @@ -13,6 +17,9 @@ describe('ActionReturnType', () => {
return { name };
},
});
expectTypeOf<ActionReturnType<typeof action>>().toEqualTypeOf<{ name: string }>();
expectTypeOf<ActionReturnType<typeof action>>().toEqualTypeOf<
SafeResult<any, { name: string }>
>();
expectTypeOf<ActionReturnType<typeof action.orThrow>>().toEqualTypeOf<{ name: string }>();
});
});
2 changes: 1 addition & 1 deletion packages/astro/test/types/is-input-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const exampleAction = defineAction({
handler: () => {},
});

const result = await exampleAction.safe({ name: 'Alice' });
const result = await exampleAction({ name: 'Alice' });

describe('isInputError', () => {
it('isInputError narrows unknown error types', async () => {
Expand Down
13 changes: 1 addition & 12 deletions packages/integrations/react/server.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import opts from 'astro:react:opts';
import { AstroError } from 'astro/errors';
import React from 'react';
import ReactDOM from 'react-dom/server';
import { incrementId } from './context.js';
Expand Down Expand Up @@ -151,17 +150,7 @@ async function getFormState({ result }) {

if (!actionKey || !actionName) return undefined;

const isUsingSafe = formData.has('_astroActionSafe');
if (!isUsingSafe && actionResult.error) {
throw new AstroError(
`Unhandled error calling action ${actionName.replace(/^\/_actions\//, '')}:\n[${
actionResult.error.code
}] ${actionResult.error.message}`,
'use `.safe()` to handle from your React component.'
);
}

return [isUsingSafe ? actionResult : actionResult.data, actionKey, actionName];
return [actionResult, actionKey, actionName];
}

async function renderToPipeableStreamAsync(vnode, options) {
Expand Down

0 comments on commit 1c3265a

Please sign in to comment.