withastro / roadmap

Ideas, suggestions, and formal RFC proposals for the Astro project.
320 stars 30 forks source link

Actions #898

Closed bholmesdev closed 6 months ago

bholmesdev commented 7 months ago

Body

Summary

Let's make handling form submissions easy and type-safe both server-side and client-side.

Goals

Non-goals

Background & Motivation

Form submissions are a core building block of the web that Astro has yet to form an opinion on (pun intended).

So far, Astro has been rewarded for waiting on the platform and the Astro community to mature before designing a primitive. By waiting on view transitions, we found a SPA-like routing solution grounded in native APIs. By waiting on libSQL, we found a data storage solution for content sites and apps alike. Now, we've waited on other major frameworks to forge new paths with form actions. This includes Remix actions, SvelteKit actions, React server actions, and more.

At the same time, Astro just launched its database primitive: Astro DB. This is propelling the Astro community from static content to more dynamic use cases:

To meet our community where it's heading, Astro needs a form submission story.

The problem with existing solutions

Astro presents two solutions to handling form submissions with today's primitives. Though capable, these tools are either too primitive or present unacceptable tradeoffs for common use cases.

JSON API routes

JSON API routes allow developers to handle POST requests and return a JSON response to the client. Astro suggests this approach with a documentation recipe, demonstrating how to create an API route and handle the result from a Preact component.

However, REST endpoints are overly primitive for basic use cases. The developer is left to handle parse errors and API contracts themselves, without type safety on either side. To properly handle all error cases, this grows complex for even the simplest of forms:

REST boilerplate example ```ts // src/api/board/[board]/card/[card].ts import { db, Card, eq, and, desc } from "astro:db"; import type { APIContext } from "astro"; const POST = async ({params, request}: APIContext) => { if (!params.card || !params.board) { return new Response( JSON.stringify({ success: false, error: "Invalid board or card ID" }), { status: 400 } ); } if (!request.headers.get("content-type")?.includes("application/x-www-form-urlencoded")) { return new Response( JSON.stringify({ success: false, error: "Must be a form request" }), { status: 400 } ); } const body = await request.formData(); const name = body.get("name"); const description = body.get("description"); if (typeof name !== "string" || typeof description !== "string") { return new Response( JSON.stringify({ success: false, error: "Invalid form data" }), { status: 400 } ); } // Actual app logic starts here! const res = await db .update(Card) .set({ name, description }) .where( and(eq(Card.boardId, params.board), eq(Card.id, params.card)) ); return new Response( JSON.stringify({ success: res.rowsAffected > 0 }), { status: res.rowsAffected > 0 ? 200 : 404 } ); } ```

The client should also guard against malformed response values. This is accomplished through runtime validation with Zod, or a type cast to the response the client expects. Managing this contract in both places leaves room for types to fall out-of-sync. The manual work of defining and casting types is also added complexity that the Astro docs avoid for beginner use cases.

What's more, there is no guidance to progressively enhance this form. By default, a browser will send the form data to the action field specified on the <form> element, and rerender the page with the action response. This default behavior is important to consider when a user submits a form before client JS has finished parsing, a common concern for poor internet connections.

However, we cannot apply our API route as the action. Since our API route returns JSON, the user would be greeted by a stringified JSON blob rather than the refreshed contents of the page. The developer would need to duplicate this API handler into the page frontmatter to return HTML with the refreshed content. This is added complexity that our docs understandably don't discuss.

View transition forms

View transitions for forms allow developers to handle a submission from Astro frontmatter and re-render the page with a SPA-like refresh.

---
const formData = await Astro.request.formData();
const likes = handleSubmission(formData);
---

<form method="POST">
  <button>Likes {likes}</button>
</form>

This avoids common pitfalls with MPA form submissions, including the "Confirm resubmission?" dialog a user may receive attempting to reload the page. This solution also progressively enhances based on the default form action handler.

However, handling submissions from the page's frontmatter is prohibitive for static sites that cannot afford to server-render every route. It also triggers unnecessary work when client-side update is contained. For example, clicking "Likes" in this example will re-render the blog post and remount all client components without the transition:persist decorator.

Last, the user is left to figure out common needs like optimistic UI updates and loading states. The user can attach event listeners for the view transition lifecycle, though we lack documentation on how to do so from popular client frameworks like React.

Princesseuh commented 7 months ago

Moved this from the Stage 1, posted this right before it moved to Stage 2:

I am not super familiar with the subject at hand (I think in the past 10 years, I've only written a <form> once, and there definitely wasn't any sort of typed actions), but I quite like the named actions, seems pretty neat to use.

One thing I'll just mention is that I saw people float the idea of having two files with the same names but a different extension (like page.astro, page.ts) but I'd approach this with caution because TypeScript might not like it. Much like you cannot have a .ts and .tsx with the same name in the same folder, the same may (I'm not 100% sure) apply to Astro files.

bholmesdev commented 7 months ago

@Princesseuh I agree with this! During goal setting, we also found a more fundamental issue following SvelteKit's spec for look-behind files: all pages with a page.ts defined would be server rendered.

A common example: a blog with a like button. Each post in your blog would include the like button, and submitting a "click" would trigger a form action to update your database counter. Using the SvelteKit convention, you would duplicate your [...slug].astro with a [...slug].ts to contain your action definitions:

// src/pages/[...slug].ts

export const actions = {
  like: defineAction(...),
}

However, this now means [...slug] is dynamic and will not be prerendered. This means your getStaticPaths() call must be refactored to a server entrypoint with a cache() header to get the same effect. We could also find a workaround in Astro to prerender GET request only, though this may cause confusion for the developer when using the hybrid output; do I set export const prerender from the [...slug].ts? From the [...slug].astro? Both?

You might think "well, form actions do require server handling. Maybe that refactor is justified." This is fair! However, there are alternative APIs we can explore to keep refactors to a minimum for the developer.

One solution would be to keep action declarations to standalone endpoint files, like a src/pages/like-api.ts in our previous example. A call to this separate endpoint could return a JSON result when called from a client script, or redirect back to the referring page when called from an HTML form. This offers similar benefits to a look-behind file while keeping action handling separate from content rendering.

bholmesdev commented 7 months ago

Where should actions live?

I want to outline a few places where actions may be defined:

  1. In the .astro component itself. We explored this early on since it felt the most streamlined. However, we quickly saw the limitations of export-ing from Astro frontmatter. Frontmatter creates a function closure, which can lead to misuse of variables within an action handler. This challenge is shared by getStaticPaths() and it would be best to avoid repeating this pattern in future APIs.
  2. In a TypeScript file of the same name as your page. This follows SvelteKit's convention of "look-behind files" and addresses our frontmatter closure problem. However, using this convention would lock your .astro route into dynamic rendering, which forces refactors onto the developer. See this comment for some more complete thoughts.
  3. In a separate endpoint file using an actions export. This allows similar colocation with routes to (2) while keeping dynamic handlers separate from static content. This solution may let you switch from REST exports like POST to a single actions export containing one or several handlers:
src/pages/blog/api.ts

export const actions = {
  like: defineAction(...),
}

To call these actions from a form or from client code, we would need some convention to infer actions available on a route. We may do this with code generation by route name (magicFetch('/blog/api', 'like')), direct imports with server action bundling, or by using object syntax as described by Arsh's typed-api.

  1. In a global src/actions directory. This ditches pages/ colocation with routes for a new directory name. Content Collections and Astro DB offer some parallels with src/content/ and db/, and tRPC uses a centralized file for action declaration as well. I expect this pattern to feel predictable given the prior art. This would also simplify how actions are called compared to (3). Since there's one place for action declarations, we can expose all actions from a virtual module like astro:actions. However, there are concerns with scalability and colocation to discuss.

Right now, I actually lean towards (4). I admittedly wanted to like (3) in this list, since I value colocation with my pages. However, I quickly found myself centralizing actions to a src/pages/api.ts or some equivalent when I was free to define multiple actions, which wasn't much different from a src/actions/index.ts.

I also realized colocation isn't self-explanatory. We want colocation... with what? After working on Astro Studio, I realized we didn't use colocation with presentational UI in pages/; we organized our API by domain models in our database. Using tRPC, we built nested routers for User, Project, Workspace, and a few other concepts. We fell into a structure like this:

// src/procedures/trpc.ts
app({
  user: userRouter, // from ./users.ts
  github: githubRouter, // from ./github.ts
  project: projectRouter, // from ./projects/.ts
});

// in your app
trpc.user.auth()
trpc.github.createTemplate()
trpc.project.create()

We could create a similar organization with src/actions/ using nested objects for each domain:

actions/
  user.ts
  github.ts
  project.ts
  index.ts
import { userActions } from './user';
import { githubActions } from './github';
import { projectActions } from './project';

export const actions = {
  user: userActions,
  github: githubActions,
  project: projectActions,
}
// in your app
actions.user.auth()
actions.github.createTemplate()
actions.project.create()

This pattern mirrors REST-ful APIs as well, which I hope feels intuitive for existing Astro users. I glossed over specifics on calling these actions from a form or a client script, but I'll detail soon!

pilcrowonpaper commented 7 months ago

You can send either a JSON object or form data to a form action

I would not recommend this. POST requests with form data are considered "simple requests" and ignored by browsers' same origin policy. Even if Astro implements a CSRF protection by default, some devs are going to disable it to enable cross-origin and cross-site requests and not implement CSRF protection since they assume their actions only accept JSON (which is subjected to SOP).

bholmesdev commented 7 months ago

Thanks for sharing that nuance @pilcrowOnPaper. I know Astro recently added built-in CSRF protection across requests, but I see what you mean about the payloads having different behavior.

It sounds like we shouldn't implicitly let you pass form data or json to the same action. How would you feel if the action explicitly defined the payload it is willing to accept? Assuming a Zod setup for validation, it may look like the following:

const like = defineAction({
  input: formData(), // accepts form data
  handler: () => {}
});

const comment = defineAction({
  input: z.object(), // accepts json
  handler: () => {}
});
robertvanhoesel commented 7 months ago

TL;DR: Make forData opt-in by exporting additional utilities or mapping formAction(defineAction()) as a way to parse a schema to a formData compatible format.

How would you feel if the action explicitly defined the payload it is willing to accept? Assuming a Zod setup for validation, it may look like the following:

@bholmesdev I just recently spend some effort using @conform-to/zod's parseWithZod() to allow me to create and validate form schema's in FrontMatter, and I would say it would be amazing if we can define expected server action's payload using Zod while still using a formSubmission. It's definitely not possible to map deeply nested Zod schemas to form input, but you can get really far. @conform-to has support for most field and Zod Types.

Safely parsing form data, and maybe validating it against a schema sounds like a good goal to me.

Here's how I currently use forms in Astro:

// admin/discounts/create.astro
---
import { createForm } from 'lib/forms.ts'
const discountSchema = z.object({
  description: z.string().optional().default(""),
  limit: z.number().int().min(1).default(1),
  code: z.string().optional(),
})

const { data, fields, isValid, errors } = await createForm(discountSchema, Astro.request)
// createForm builds fields with required attributes from schema, checks for `request.method=='POST'
// and parses with Zod to data if valid, or errors otherwise

if(isValid) {
  try { await storeOnServer(data) } catch {}
}
---
<form method="post">
   <label>Code</label>
   <input {...fields.code} /> /* type="text" name="code" */
   <small>{errors.code?.message}</small>

    <label>Limit</label>
   <input {...fields.limit} /> /* type="number" name="limit" min="1" */
      <small>{errors.limit?.message}</small>

   <label>Description</label>
   <textarea {...fields.description} /> /* name="description" */
   <small>{errors.description?.message}</small>

   <button type="submit">Create</button>
</form>

Exported action

The below is very much my interopretation/stab at an API I personally would prefer using, and not so much what I think makes sense from a technical perspective.

I think locating actions inside an actions folder is fine, especially if the resulting endpoints don't necessarily match the names or folder structure. That would allow collocation multiple related actions togethers

// src/actions/cart.ts

export const sizes = ["S", "M", "L", "XL"];
const cartItem = z.object({
       sku: z.string(),
       size: z.enum(sizes),
       amount: z.number()
    })

export addToCart = createAction({
    schema: cartItem,
    handler: (item: z.infer<typeof cartItem>) => addToCart(item)
})

export updateCartItem = createAction({
    schema: cartItem,
    handler: (item: z.infer<typeof cartItem>) => updateCart(item)
})

// etc

createAction() could export some useful utilities like:

Usage in forms

On the consumer side, I would use it like the following, perhaps:

// pages/store/[product].astro
import { addToCart } from '../../actions/cart.ts'

const product = Astro.props
const { form, inputs } = addToCart
---
<form {...form.attrs}> /* action="/_actions/cart/addToCart" method="post" */
   <label>Amount</label>
   <input {...form.inputs.amount.attrs } />
   <select {...form.inputs.size.attrs }>
    {form.inputs.size.options.map(size => <option value={size}>{size}</option>)}
    </select>
   <input {...form.inputs.sku.attrs} type="hidden" value={product.sku} />
   <button type="submit">Add to cart</button>
</form>

If I would like to use the action as a full page form submission instead, maybe an other API could be something like: await formAction(action, Astro.request)

Which creates the above attributes and a form handler on this specific route. It would submit to the action/call its handler, with validated formData and additionally output errors/success state.

// pages/store/[product].astro
import { addToCart } from '../../actions/cart.ts'
import { formAction } from 'astro:actions'

const product = Astro.props
const { form, inputs, success, errors } = await formAction(addToCart, Astro.request)
---
<form {...form.attrs}> /* action="" method="post" */
    {success && <p>Added to cart</p>}
   <label>Amount</label>
   <input {...form.inputs.amount.attrs } />
   {errors.amount && <small>{error.amount.message}</small>}

   <select {...form.inputs.size.attrs }>
    {form.inputs.size.options.map(size => <option value={size}>{size}</option>)}
    </select>
    {errors.size && <small>{error.size.message}</small>}

   <input {...form.inputs.sku.attrs} type="hidden" value={product.sku} />
   <button type="submit">Add to cart</button>
</form>

Usage inside Framework Components

You won't touch the exported form property at all and instead directly use the schema, url etc

// components/addToCartButton.tsx
import { addToCart, sizes } from '../../actions/cart.ts'

export const AddToCartButton = (props) => {
   const [amount, setAmount] = createSignal<z.infer<typeof addToCart.schema>['amount']>(1)
   const [size, setSize] = createSignal<z.infer<typeof addToCart.schema>['size']>(sizes[0])
   const [success, setSuccess] = createSignal(false)
   const [errors, setErrors] = createSignal(null)

   const submit = () => {
      try {
         await someHttpLib.post(addToCart.url, { amount: amount(), size: size(), sku: props.sku })
         setSuccess(true)
      } catch (e) {
         setErrors(e)
      }
   }

   return (<div>
      <NumberField value={amount} onChange={setAmount} />
      <Dropdown options={size} selected={size} onSelect={setSize} />
      <Button onClick={submit} />
   </div>)
}

Could even take it up a notch and let submission and handling of the server action result be an exported function

// components/addToCartButton.tsx
import { addToCart, sizes } from '../../actions/cart.ts'

export const AddToCartButton = (props) => {
   const [amount, setAmount] = createSignal<z.infer<typeof addToCart.schema>['amount']>(1)
   const [size, setSize] = createSignal<z.infer<typeof addToCart.schema>['size']>(sizes[0])
   const [success, setSuccess] = createSignal(false)
   const [errors, setErrors] = createSignal(null)

   const submit = () => {
      const { success, errors } = await addToCart.submit({ amount: amount(), size: size(), sku: props.sku })
      setSuccess(success)
      setErrors(errors)
   }

   return (<div>
      <NumberField value={amount} onChange={setAmount} />
      <Dropdown options={size} selected={size} onSelect={setSize} />
      <Button onClick={submit} />
   </div>)
}
bholmesdev commented 7 months ago

Thanks for that @robertvanhoesel! Good to see thorough overviews like this. I think we're pretty aligned on how we want form actions to feel. Pulling out a few pieces here:

robertvanhoesel commented 7 months ago

I think if actions export enough data/schemas/utilities the very concrete implementation I showed above could be more of a library implementing the actions API rather than something that should be packed by default. Could still be part of the core offering but ultimately it starts becoming prescriptive in how to implement actions.

In the end this is a pure DOM spec implementation to have some client side validation based on an action schema. It makes sense to live as import { createForm } from 'astro-actions-dom' or import { Form } from 'astro:actions

Perhaps this could be an Astro component feature exclusively using a directive. Do you have thoughts on this?

I have not considered a directive. Would I assume you mean something like:

<input action:attributes={action.schema.myProperty} />

I'm not sure if I'm a fan, but perhaps that's because directives are sparsely used in Astro at the moment.

I have considered building it into a component, but my main argument against it is that it would prevent you from using UI Framework components which enhance standard HTML Inputs with validation states or UX. I want to be able tp spread the validation attributes and field name onto a framework component, which has my preference over using some custom <Field> component. Even for pure Dom it feels better to have an non-abstracted <input {....attrs} over a Field where I don't know what to expect will be rendered.

Is the below syntax possible in Astro (maybe it already is!), a

component that can take a function as contents/default slot where the first param is the generated inputs/validation?

---
import { Form } from 'astro:actions'
import { postComment } from '../actions/comments'
---

<Form action={postComment}>{(inputs) => (
   <div>
      <input {...inputs.name.attrs} />
      <textarea {inputs.content.attrs} />
    </div>
 )}
</Form>

Beyond this DOM topic:

  1. What would be the expectation that an action? It will return resulting data as JSON or throw an exception? Will the action API format the Response object? If not,
  2. As a framework component, how would I call it and then subsequently handle the error? Is there some contract or 'ClientSafeError' type of error?
  3. I'm not sure if it makes sense that the vanilla implementation of actions take formData vs JSON.

    You can call an action using a standard HTML form element with the action property.

    Parsing formData into a richer object schema is a messy process. I'm not aware of frameworks that prefer sending formData over pure JSON.

  4. Is there any opportunity here to combine Astro ViewTransitions and Partial Page routes?

Partial Page Action / Multiple Forms

My biggest frustration currently with native forms and handling forms in Astro is whenever you start having more than one form/action on a page. I currently make sure components that include a form, and can be on the same page as other form components, will check for some unique <button type="submit" name="scope" value="sometinyform"> combination to be present before handling the formData.

I even have a helper in my createForm for creating that scope on the submit and then also validating the scope value is set whenever a POST request on that route is triggered.

const { fields, submitButton } = await createForm(schema, Astro.request, { scope: 'someIdentifier' })
---
<form>
  ...
  <button {...submitButton} >
 </form>

I was initially excited for (Form) Actions in Astro because I hoped it would offer a fairly native solution to this without the need of client side UI components.

So, that makes me wonder, can we adopt a baseline HTMX feature set for this. Combining Partial Route Components (so they can be routed to) with ViewTransitions.

// pages/partials/like.astro
---
import { likeAction } from '../../actions/like.ts'

export const partial = true;

interface Props {
   count: number
   slug: string
 }

let count = Astro.props.count

if(Astro.request.method==="POST") {
 try {
    count = await likeAction(Astro.request.formData()) // count++
    } catch (e) { .... }
 }
---
<form transition:replace action="partials/like" method="post">
   {count} likes
   <button type="submit"> Like</button>
   <input type="hidden" name="slug" value={Astro.props.slug} />
</form>

So we could do something like the below on routes that have ViewTransitions enabled.

// pages/blogs/[slug].astro
---
import Like from '../../partials/like.astro'

const blog = Astro.props

---
<h1>{blog.title}</h1>

<LikeCount slug={blog.slug} count={blog.likes} />

If not the above, I'm really puzzled as to why to support formData out of the box in actions. In that case astro:actions would be more utilities for registering API routes that return API data, which is a fine feature.

I'm a fan of Astro because I can sprinkle in frameworks and libraries where needed. It would be super amazing if we could have basic self replacing forms out of the box.

bholmesdev commented 7 months ago

Appreciate the input @robertvanhoesel. I'm hearing a few pieces in your feedback:

hfournier commented 7 months ago

@bholmesdev your last bullet sounds a bit htmx-ish.

NuroDev commented 7 months ago

If we support FormData as an input object alongside JSON, we need a strong reason for it.

Just to spit ball & throw it out there: I think the majority of the time a JSON payload is all anyone will ever need. The one / main use cases I can think of for when FormData may be needed is for for if your action has a payload that cannot be serialised.

The main example being a user account settings page where the user can upload an avatar / image. The options in this scenario in my mind would be:

The other main scenario I could possibly wonder when FormData may be needed instead of a JSON payload would be for Dates / BigInts, but that could potentially be worked around by implementing Superjson support.

bholmesdev commented 7 months ago

Alright, we've been iterating on form actions with a lot through feedback from core, maintainers, and the general community. It seems the thoughts that resonated most were:

I'm excited to share a demo video for a proof-of-concept. We're already reconsidering some ideas noted here, namely removing the need for an enhance flag. Give it a watch! Thanks y'all.

https://www.loom.com/share/81a46fa487b64ee98cb4954680f3646e?sid=717eb237-4dd8-41ce-8612-1274b471c2ca

bholmesdev commented 7 months ago

Good feedback @NuroDev! I agree JSON is the best default, and we'll likely lead with that in our documentation examples. Still, the form action property supported in React 19 does show the value of form data payloads. You can control state updates from the client, but get zero-JS progressive enhancement for free. It's a tighter requirement to store all request information in form inputs instead of JS. But for forms fit the mold, it's a great practice to support.

That's also a good callout on file uploads. It sounds like the tRPC team has encoded file blobs to JSON strings with reasonable success, but it's probably not the approach for everyone. At the very least, it would be nice to access the raw request body from a context variable if you need to go deep parsing inputs.

pilcrowonpaper commented 7 months ago

Wait, how do you implement progressive enhancement with JSON?

Re: input validation - I don't really see the benefit of integrating it into the core of form actions. Why can't it be a wrapper over defineAction() like defineActionWithValidation()?

NuroDev commented 7 months ago

I don't really see the benefit of integrating it into the core of form actions.

As a counter point to this: Astro already offers input validation for stuff like content collections out of the box with defineCollection, so would it not make sense to continue the trend?

Though that did pose me the question: If someone doesn't want any kind of validation, or wishes to use a different validation library (Valibot, Typebox, etc) would the 1st parameter of the handler just return the un-validated request body I assume?

bholmesdev commented 7 months ago

Alright, thanks again for the quality input everyone. We've finally hit a point that we can draft an RFC. It includes complete docs of all expected features, and describes any drawbacks or alternatives we've discussed here. Give it a read when you get the chance. Open to feedback as always.

https://github.com/withastro/roadmap/pull/912

mayank99 commented 7 months ago

I took a brief look at #912 and it feels a tad overcomplicated and restrictive. I don't want to use zod or anything, I just want to call a function that executes on the server. (This is not to meant to be dismissive users who want zod, but I feel that it shouldn't be required)

The ideal API would allow defining functions in the same astro file as the form:

---
const doStuff = async (formData) => {
  // do stuff with formData
}
---

<form method="POST" action={doStuff}>…</form>

I see that this is listed as a non-goal, which is a bit unfortunate. What is the main problem with this? (i.e. what is meant by "misuse of variables"?) Today we can already handle form submissions in frontmatter, so this kinda feels natural to me? (at least when using SSR)

As a compromise, it would be nice if actions were regular functions and allowed to be exported from anywhere (not just in src/actions/):

// in src/wherever.js
export const doStuff = async (formData) => {
  // do stuff with formData
}

and imported anywhere, probably with import attributes:

---
import { doStuff } from "../wherever.js" with { type: "action" };
---

<form method="POST" action={doStuff}>…</form>

The RFC says the main argument against this syntax is that users might forget to set the import attribute. (I feel like users deserve more credit than that, but anyway…)

Maybe there should be runtime restriction around where the action is invoked from. e.g. <form action> works out-of-the-box in .astro, but anything else could perhaps require using a helper (and throw otherwise)?

Example ```js export const doStuff = async () => {}; ``` ```jsx import { doStuff } from "./doStuff" with { type: "action" }; import { invokeAction } from "astro:actions"; const handleFormSubmit = async (e) => { e.preventDefault(); // throws if not used in .astro file // doStuff(); // must wrap in invokeAction const { stuff } = await invokeAction(doStuff)(); } ```

Edit: Is it possible to implement it such that it throws if imported without the attribute? I feel like it should be possible using a helper at the place where the action is defined (rather than at the call-site).

import { defineAction } from "astro:actions";
export const doStuff = defineAction(async () => {});
import { doStuff } from "./doStuff";
await doStuff(); // ❌ throws
import { doStuff } from "./doStuff" with { type: "action" };
await doStuff(); // 👍 all good
bholmesdev commented 7 months ago

Hey @mayank99! Appreciate the detailed input here. It sounds like you see a bit too much overhead with defineAction() compared to a straightforward function. A primary motivation for Zod was to simplify parsing formData properties without type casting or validation error tracking. Just curious, how do you normally parse FormData objects in your projects?

I'll also admit defineAction() are a superset of action handlers, this happens to work in our early prototype:


export const server = {
  comment: async (formData: FormData) => {...}
}

Once we have an experimental release, it may be worth trying this alongside defineAction() to see what sticks.

It also seems like you value colocation of actions within your Astro components. We agree, this seems intuitive. However, we had a few qualms with actions inside Astro frontmatter beyond what was listed in the non-goal:

Your import attributes idea would solve this issue. defineAction() could raise exceptions somehow to catch mis-use. There were just a few issues we were unsure about:

mayank99 commented 7 months ago

@bholmesdev I appreciate you taking the time to dive deeper into my feedback! I enjoy this kind of nuanced discussion 💜

Just curious, how do you normally parse FormData objects in your projects?

Normally I extract the fields like this:

const { postId, title, body } = Object.fromEntries(formData);

and validate them in varying ways:

Manually ```js if (typeof postId !== 'string') throw "Expected postId to be a string"; // … ```
Using tiny-invariant ```js invariant(typeof postId === 'string'); invariant(typeof title === 'string'); invariant(typeof body === 'string'); ```

The neat thing about this is that zod can be very easily added to this step.

const { postId, title, body } = PostSchema.parse(Object.fromEntries(formData));

Users may expect variables outside of their action to be accessible within an action. They may also expect to mutate outside variables based on the result of an action.

Yes, that's part of why I want the colocation in the frontmatter in the first place 😅

I think in RSC, this works sorta as expected. You can define server actions in the same file as a server component, and even use variables from the surrounding scope.

It's unclear how to use the result of an action. For instance, say we wanted to show an error banner if doStuff fails. How could we do that?

The way I normally do this is either directly setting the error variable in frontmatter, or if I'm redirecting to a different page, then I use an ephemeral cookie (I believe this is called "session flashing" in some backend frameworks).

In any case, I think the same approach could be used as the current RFC (i.e. Astro.getActionResult) if something is returned from the action? I don't see how/why it would need to be too different for frontmatter actions.


Should import attributes be used for transformations like this?

I think yes? On the web, there are two cases supported in different browsers: type: "css" and type: "json". In both cases, they take the raw file, and transform it into something that makes more sense in the JS file where it is imported (e.g. a CSSStyleSheet instance and a regular JS object respectively).

It's a stage 3 proposal with differing opinions on how they should be held. They are also not (yet) implemented in Vite. Betting on attributes would be a slight gamble.

This is a great point. It might be worth talking to the maintainers of Node.js and Vite to see how they feel about it.


How do you typically organize your backend handlers in your codebase?

I think people should be free to organize it however they want, rather than being restricted to using the src/actions folder. For instance, I typically like to colocate the actions in the same folder with the component using it.

With your suggestion, we introduce the need for a defineAction() wrapper and an import attribute at each call-site.

My impression was that defineAction() was already required in the current RFC.

The import attribute is indeed more verbose. I wonder if typescript support for import attributes could make it so that auto-import adds the import attribute too. 🤔

This is a bit more syntax than importing directly from a virtual module like astro:actions. Perhaps that complexity is worth it, just want to hear your perspective.

Virtual modules add an extra level of overhead imo, since there is an intermediate layer between where the action is defined vs used. I've already had some intermittent issues with the generated astro:db modules.

With import attributes, we'd be able to get rid of the intermediate layer and import the action directly.

okikio commented 7 months ago

Should import attributes be used for transformations like this?

I think yes? On the web, there are two cases supported in different browsers: type: "css" and type: "json". In both cases, they take the raw file, and transform it into something that makes more sense in the JS file where it is imported (e.g. a CSSStyleSheet instance and a regular JS object respectively).

It's a stage 3 proposal with differing opinions on how they should be held. They are also not (yet) implemented in Vite. Betting on attributes would be a slight gamble.

This is a great point. It might be worth talking to the maintainers of Node.js and Vite to see how they feel about it.

@mayank99 Building on your comment on import attributes, you're right that they're not quite standardized yet, another possible, less elegant, but ready solution might be using import query params, e.g. ...?action at least until Vite.js and Node.js are ready for import attributes, IMO predefined folders apply a number of restrictions that aren't really necessary for what actions normally do

Note: I do like the actions/ folder as a convention but not necessarily a rule, instead I think using import query params ...?action would be a more appropriate solution at least until import attributes are standardized.

E.g.

// src/components/Like.tsx
import { actions } from "~/actions/index.ts?action";
import { useState } from "preact/hooks";

export function Like({ postId }: { postId: string }) {
  const [likes, setLikes] = useState(0);
  return (
    <button
      onClick={async () => {
        const newLikes = await actions.like({ postId });
        setLikes(newLikes);
      }}
    >
      {likes} likes
    </button>
  );
}
bholmesdev commented 6 months ago

Thanks for the input @mayank99 and @okikio! Let me speak to a few bits of feedback I'm hearing:

First, it sounds like there's desire to handle validation yourself without an input wrapper. Taking Mayank's example, something like:

handler: (formData) => {
  const { postId, title, body } = PostSchema.parse(Object.fromEntries(formData));
}

I spent some time prototyping this, since I did like the simplified onboarding experience. However, there's a few problems with the approach:

const result = await actions.myAction.safe(formData);
if (isInputError(result.error)) {
  result.error.fields.postId // autocompletion!
}

In the end, it felt like a first-party input validator offered enough utility to keep. I've also updated the handler's default input type to be a FormData object. This makes validation optional. So if you want to DIY validation, it's still fairly straightforward:

action: defineAction({
  accept: 'form',
  handler: (formData) => {
    formData.get('property')
   }
})

Now, about the astro:actions module...

First, I'm pretty confident we can avoid an intermittent issues with virtual module types using this model (see astro:db). Types are inferred directly from the server object, so you do not need astro dev to be running for types to stay up-to-date. This should hopefully prevent the need for server reloads as types update.

It also seems like colocation with individual components is valuable to y'all. I understand that. Still, I want to offer some thoughts:

First, frontmatter closures are dangerous for reasons not discussed in the above examples. I can't forget a discussion with the Clerk CEO about an authentication issue their users were seeing. They expected the following to gate their action correctly:

function Page() {
  const user = auth();
  function login() {
    "use server";
    if (user.session === 'expected') { /* do something */ }
  }
  return (<></>);
}

Not only did this not check the user object from the action, but it encoded the user as a hidden form input to "resume" that state on the server. This is because components do not rerun when an action is called; only the action does. Unless Astro wants to adopt HTML diffing over client-side calls, we would have the same problem.

Putting closures aside, there is a world using import attributes from a separate file. But there are a few troubles that make this difficult:

I'll also address Okikio's suggestion for a ?flag using Vite bundling. This is supported today, though it's tough to preserve types using the model. When implementing in userland, @bluwy needed to implement a TypeScript plugin to properly regex the flag away. It's tough to justify the maintenance burden, especially if this API is a compromise.

In the end, an actions file is simpler and well-proven from a technical standpoint. We just can't justify a gamble on import assertions. What's more, starting with the simpler path keeps this door open; start with an actions file, move to actions anywhere :tm: if there's enough demand.

Hope this makes sense! Our goal was to entertain every possible direction before landing on a release, so this is all appreciated.

cbasje commented 6 months ago

First of all, I love that you are adding this feature! I use SvelteKit in my job and the Superforms library has been a gamechanger so I am glad that Astro is adding something similar. Fun fact about the above conversation: the Superforms library just had a major rewrite to allow for more custom validation but that is besides my point haha.

I was playing around with the feature on a project that I want to use this. On this project I need to set a config.base because it shares a domain with other projects. However, when I try to use an Action, it gives me an error Action not found because it still tries to call Actions at /_actions/... instead of /base/_actions/.... I don't see a way of adapting the request anywhere and if it is possible without middleware? I am also not sure if this is a good place to report it but I wanted to do it somewhere.

Anyway, thanks for all this great work!

bholmesdev commented 6 months ago

Hey @cbasje! Glad you're enjoying the feature. You're right that the base config option should be respected from client code. I can take a look at that.

ciw1973 commented 6 months ago

Wait, how do you implement progressive enhancement with JSON?

You can't, that's why actions being able to handle FormData is so important.

Accessibility requirements mean that many large organisations and government departments mandate that their public facing (and they strongly recommend that their internal) apps and sites are fully functional without JS.

Try using an SPA with screen reader software, and you'll quickly see why this isn't just about catering for people who've decided for whatever reason to disable JS in their browser.

third774 commented 6 months ago

@ciw1973 @pilcrowOnPaper Adam Silver has a great blog post on this topic: https://adamsilver.io/blog/javascript-isnt-always-available-and-its-not-the-users-fault/

robertvanhoesel commented 6 months ago

@bholmesdev Finally got to play around with Actions today.

Excellent job on the implementation and API, error handling and progressive enhancement (I like how getActionResult works a lot). The proxy object definition is really useful as I can mimic different parts of my app and ensure certain middleware is run on matching action routes.

One question I have: I can export an action from a route pages/myRoute.astro like below, and everything works (as I expected). I know this is a discouraged pattern because people may not be aware of the context around the function not being available in the action. But it is a very nice way to collocate my actions. Is the current thinking still the same, or can I safely introduce the below pattern to others?

export const myAction = () => defineAction({ ... }
bholmesdev commented 6 months ago

Glad you like how actions are working @robertvanhoesel! And uh... I'm very surprised this works actually. Can you give a more complete example? 😅

florian-lefebvre commented 6 months ago

Stage 3 RFC available in #912

robertvanhoesel commented 6 months ago

Glad you like how actions are working @robertvanhoesel! And uh... I'm very surprised this works actually. Can you give a more complete example? 😅

@bholmesdev See below, maybe it works because the action is imported in the Githubissues.

  • Githubissues is a development platform for aggregating issues.