sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.44k stars 1.89k forks source link

Get rid of request in form actions, and transfer type safe data directly #9579

Open jdgamble555 opened 1 year ago

jdgamble555 commented 1 year ago

Describe the problem

It seems like there isn't much difference between form actions and an endpoint when you still have to get the data from formData(). Most importantly, this is not type safe, and a redundant extra step.

Describe the proposed solution

https://start.solidjs.com/core-concepts/actions https://qwik.builder.io/docs/action/

Like Qwik and SolidJS, you should be able to just pass the data directly instead of the excess extra step of using a request object and formData, which is not type safe.

export const actions = {
  login: async (data , { cookies, request }) => {
    const { email } = data;  // automatically typesafe as string type
    const user = await db.getUser(email);
    cookies.set('sessionid', await db.createSession(user));

    return { success: true };
  },
  register: async (event) => {
    // TODO register the user
  }
} satisfies [Actions](https://kit.svelte.dev/docs/types#public-types-actions);

It should also be true for the data and FormData object in use:enhance:

Alternatives considered

Use TRPC to pass typesafe data to endpoints, but type safety should be built-in.

Importance

would make my life easier

Additional Information

No response

Rich-Harris commented 1 year ago

There's no such thing as type safety when dealing with client-server communication — anyone can POST any data at any time. For that reason I'd probably recommend using something like Zod, like this or even this.

As for passing data into the action, this is something we initially planned. But it means calling await request.formData() eagerly, which is undesirable if your form includes files — you want to stream the data rather than buffer it.

jdgamble555 commented 1 year ago

So why not have it pass a request object if it is being passed from a form, and otherwise a JSON object?

Possible New Implementation

This is kind of how I feel it should work...

JSON Object

async function handleSubmit() {

    const todo = todoInputValue;  // perhaps binded to input

    const result: ActionResult = await action.addTodo({ name: todo });

    if (result.success) {
      todos.set(result.value);  // update after, although you could do optimistically before
    }
  }

with:

export const actions {
  addTodo: async (data: Todo) => { // addTodo: (data: Todo, { cookies, request }) => {

    // update in database
    const update = await db.update(data);
    return { 
      data,
      success: true
    };
  }
} satifisfies Actions;

Current Implementation 😬

async function handleSubmit(event) {
    const data = new FormData(this);

    const response = await fetch(this.action, {
      method: 'POST',
      body: data
    });

    const result: ActionResult = deserialize(await response.text());

    if (result.type === 'success') {
      // re-run all `load` functions, following the successful update
      await invalidateAll();
    }

    applyAction(result);
  }

with:

// I just threw some examples together here, but you get the point...
export const load = ((event) => {
  return {
    user: event.locals.user
  };
}) satisfies PageServerLoad;

export const actions = {
  login: async ({ cookies, request }) => {
    const data = await request.formData();
    const email = data.get('email');
    const password = data.get('password');

    const user = await db.getUser(email);
    cookies.set('sessionid', await db.createSession(user));

    return { success: true };
  },
} satisfies Actions;

To me, this seems way more complicated than an endpoint. If it is not an endpoint, it should be extremely easy to implement and not feel like and endpoint. Again, Solid Start and Qwik City make this extremely easy, and do not require you to use fetch, POST, etc. However, you still have access to the event.request for situations where you need them like file uploads.

I would also argue that not returning the data directly, then dealing with a load function is extremely confusing. To me, the load function should be for loading data before the page load, not refreshing data. Now I have to deal with a action function, then somehow refresh a completely different function if I need to get data? I realize this is a different issue.

My point being is that form-actions could be incredibly simplified. I highly appreciate you guys work on this and SvelteKit, but I'm hoping you guys agree that it can be much more simple. The DX of Svelte is amazing, and then you see form actions... and I think: I could write this way quicker with just an endpoint, what is the real advantage here? Form Actions should get rid of complexity, not add it.

SUM

I do believe that most developers will agree with this. If there are specific reasons not to do these things, then make a simple version and a complex version for specific use cases.

Thanks for all you guys do,

J

dummdidumm commented 1 year ago

I think this comparison is not quite right:

What both these frameworks do is hide away some of the work that's done, which is good or bad depending on who you ask. We think it's better to be more explicit about these things so you know better what's going on - the fact that you compared the SolidStart version which doesn't work without JS to form actions shows this (which isn't your fault btw! It's super tricky to see because it looks so easy/similar at first). We also think it's good to not abstract away too much so that people can built their own abstractions on top. Today Zod is the go-to library for parsing stuff from the request object, that may change in the future so we shouldn't build it in for example. You're free to create abstractions on top or use libraries which do this, like https://github.com/ciscoheat/sveltekit-superforms or using TRPC like you mentioned in "alternatives" which you can easily integrate with SvelteKit (https://github.com/icflorescu/trpc-sveltekit).

jdgamble555 commented 1 year ago

@dummdidumm

Solid Start

Hmm. Yes, you're right about Solid Start as I dig deeper; so that is not a good example.

Qwik City

It looks like the file requests are available through the request object, but json data (or variables) are just passed normally with the eager await.

Q1: - Could you not do eager await for variables, and use the request object for files?

TRPC

TRPC was built because these frameworks don't handle these things natively. No one wants to import or use an external library when they don't have to adding more overhead. If it is built in to the framework, should be no problems.

Zod

I'm just talking about passing types, I'm not talking about validating them. This is exactly what you already do with PageData and PageLoad, when you pass data from the server load function to the browser (although it says any in $page, so please fix that). It also seems to work with ActionData, so the issue we are talking about here is browser --> server variable passing.

Fetch

Why use a fetch for passing data in an action. You should make this simple:

const response = await fetch(this.action, {
  method: 'POST',
  body: data
});

const result: ActionResult = deserialize(await response.text());

This could be simplified to:

const result: ActionResult = await action.actionName({ data });

99% of use case are going to be coping and pasting this boilerplate over and over again. If we need fetch for more advanced features, we just use fetch.

Q2: - Could you add the option to just run an action directly without all the fetch boilerplate?


So:

1.) Could you not do eager await for variables, and use the request object for files? - That way we can just pass an object keeping its type (again, validating with ZOD is a different concept).

2.) Could you not get rid of the unnecessary fetch boilerplate for form actions, and just add a simple action method like above? If needed more advanced features, people could just use fetch.

J

dummdidumm commented 1 year ago
jdgamble555 commented 1 year ago

1.) I believe there is a way to separate FILE objects from regular input objects. I will get back to you once I fully understand how Qwik plans to handle this.

2.) use:enhance only helps if you're using a form. You don't need to use a form in most cases (except when you have multiple inputs). If we're using a signup button, a pagination button, signout, async load, etc, etc... there is no reason to add more boilerplate with a form element. I realize this could be preferential, but it does add more boilerplate.

So you guys gave another option, which could be used on on:click instead of on:submit, if you want to submit the form programmatically. That way the handleSubmit example could be greatly simplified without needing the fetch to something like this (as I stated above):

const result: ActionResult = await action.actionName({ data });

This is what SolidJS would be doing with createServerAction$ and what Qwik does with routeAction$ programatically.

At the very least, this could be simplified. I know you guys worked hard on form actions here, but there is always room to simplify things as other teams have found 😃


I think we both agree TRPC can add a lot features that SvelteKit doesn't have. I am pro TRPC, just like I'm pro RXJS, but only for advanced use cases, not simple every day use cases.

J

elliott-with-the-longest-name-on-github commented 1 year ago

I tend to work more on the backend than on the frontend, so maybe it's just my ignorance speaking, but this is false, right?:

You don't need to use a form in most cases

The whole point of actions is to provide an interface through which you can progressively enhance forms. Without a form element, JavaScript must work to make any submission to the backend from the frontend. use:enhance just lets you add additional logic when JavaScript works. So in order to use Actions as intended, you'd need a form element in every case, right?

dummdidumm commented 1 year ago

That's correct. If the "no JavaScript/progressive enhancement" case is none of your concern, just skip form actions entirely and use +server.js endpoints with fetch or an abstraction on top of it like TRPC.

jdgamble555 commented 1 year ago

@dummdidumm @tcc-sejohnson - You don't need a <form> html element to have a form input or button. That is the whole reason on:click exists in javascript.

Most examples in Svelte REPL don't even have a form element. In fact, some Svelte UI Libraries won't even give you access to the input values directly, as you would need to submit them programmatically.

The point of form actions is to pass data to the server, but a form CAN exist without a form element.

J

dummdidumm commented 1 year ago

It can, but then it will no longer work without JavaScript - that's the whole point if the progressive enhancement story which form actions are built around

jdgamble555 commented 1 year ago

It can, but then it will no longer work without JavaScript - that's the whole point if the progressive enhancement story which form actions are built around

Ok, I think I understand the misunderstanding now. While form actions are great for when javascript is disabled or unavailable for "progressive enhancements," I don't see that as the ONLY use case or benefit.

That is why form actions ARE available as Solid JS Actions or Qwik Actions$. To me, the other benefit is to get rid of boilerplate code in fetch responses to a server route. Most of the time we're not creating a complex REST or GraphQL endpoint, we just want to get our data in a secure environment without offloading this to the client. We also usually only want our app to access that endpoint directly, not create an endpoint for just anybody.

There are a lot of use cases (IMO most use cases) where you simply want to click a button, and have that button do an action on the server. Yes, you should be able to use a form tag with use:enhance, but you should ALSO be able to submit this programmatically in a post request without all the boilerplate.

If I am doing pagination, for example, I may not be getting all my input from an input element. The page itself may already know what page number it is, etc.

So, instead of having to do this:

const response = await fetch(this.action, {
  method: 'POST',
  body: data
});

const result: ActionResult = deserialize(await response.text());

to connect with my backend form action, I should be able to do just this:

const result: ActionResult = await action.actionName({ data });

Everything else would be the same and work the same way (load function, invalidate, applyAction, backend, etc), we would just be getting rid of less boilerplate, and simplifying things.

J

williamviktorsson commented 1 year ago

I would consider it a significant DX improvement if await request.formData() returned an object that has keys corresponding to the input names of the connected form.

The types per entry could be FormDataEntryValue or string or File and still be nicer to work with than the current form.get(“foobar”); where typos generate errors.

It’s not exactly what the issue asks for, but perhaps a good middle ground between what we have today and what you can achieve by using sveltekit-superforms.

Guandor commented 2 weeks ago

Considering how most frameworks are now including server functions, this seems like the natural next step for SvelteKit.

huseeiin commented 2 weeks ago

i don't think form actions should be removed for this. we could have both form actions and server functions.