Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions: Make .safe() the default return value #11571

Merged
merged 19 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved

```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
Loading