TheEdoRan / next-safe-action

Type safe and validated Server Actions in your Next.js project.
https://next-safe-action.dev
MIT License
2.29k stars 35 forks source link

[BUG] Sending form with javascript disabled not working #189

Closed palmjack closed 4 months ago

palmjack commented 4 months ago

Are you using the latest version of this library?

Is there an existing issue for this?

Describe the bug

Sending form with js disabled does not work when using useAction hook.

Reproduction steps

  1. Go to playground https://next-safe-action-playground.vercel.app/stateless-form
  2. Disable javascript in devtools
  3. Try sending the form

See attached screenshot of the form element CleanShot 2024-07-02 at 12 48 01@2x

Expected behavior

Form should be sent with javascript disabled, at least that's what I understand from looking at the example in the docs https://next-safe-action.dev/docs/recipes/form-actions

Minimal reproduction example

https://next-safe-action-playground.vercel.app/stateless-form

Operating System

macOS

Library version

7.1.3

Next.js version

15.0.0-rc.0

Additional context

No response

TheEdoRan commented 4 months ago

Not sure if I'm missing something, but how can it work with JS disabled if you're using the action via a hook, which is a JavaScript function by definition? Don't think this is related to next-safe-action. I guess progressive enhancement is only available if you're passing the Server Action directly to the form action.

palmjack commented 4 months ago

You may want to take a look on Jack's Harrington example repo and its corresponding yt video I also checked ZSA and its executeFormAction method from useServerAction hook and it works as expected with js disabled. See the example

tenghuan123 commented 4 months ago

First, I tested the executeFormAction exported by zsa's useServerAction, which also does not support progressive enhancement. If I write it like this, I will still get an unsupported error prompt, just like this:

export function UseZsaStateExaple() {
  const { isPending, executeFormAction, data, error } =
    useServerAction(produceNewMessage);

  return (
    <div>
      <form action={executeFormAction}>
        <input name="name" placeholder="Enter your name..." />
        <button type="submit" disabled={isPending}>
          submit me
        </button>
      </form>
      {isPending && <div>Loading...</div>}
      {data && <p>Message: {data}</p>}
      {error && <div>Error: {JSON.stringify(error)}</div>}
    </div>
  );
}

image

In addition, if you want to support progressive enhancement, you need to use the official useActionState of React, and the example of zsa is the same

"use client";

import { useActionState } from "react";
import { produceNewMessage } from "./actions";

export default function UseActionStateExample() {
  let [[data, err], submitAction, isPending] = useActionState(
    produceNewMessage,
    [null, null] // or [initialData, null]
  );

  return (
    <div>
      <form action={submitAction}>
        <input name="name" placeholder="Enter your name..." />
        <button type="submit" disabled={isPending}>
          submit me
        </button>
      </form>
      {isPending && <div>Loading...</div>}
      {data && <p>Message: {data}</p>}
      {error && <div>Error: {JSON.stringify(error)}</div>}
    </div>
  );
}
tenghuan123 commented 4 months ago

Here is an example of using useActionState to support progressive enhancement in next-safe-action

// actions.ts
'use server'

import { createSafeActionClient } from "next-safe-action"
import { zfd } from "zod-form-data";

export const produceNewMessageAction = createSafeActionClient()
  .schema(
    zfd.formData({
      name: zfd.text(),
    })
  )
  .stateAction(async ({ parsedInput }) => {
    await new Promise((resolve) => { setTimeout(resolve, 500) })
    return "Hello, " + parsedInput.name
  })
"use client"

import { useActionState } from "react";
import { produceNewMessageAction } from "./actions";

export function UseActionStateExample() {
  const [state, submitAction, isPending] = useActionState(
    produceNewMessageAction,
    {}
  );

  return (
    <div>
      <form action={submitAction}>
        <input name="name" placeholder="Enter your name..." />
        <button type="submit" disabled={isPending}>submit me</button>
      </form>
      {isPending && <div>Loading...</div>}
      {state.data && <p>Message: {state.data}</p>}
      {state.validationErrors && <div>Error: {JSON.stringify(state.validationErrors)}</div>}
    </div>
  );
}
palmjack commented 4 months ago

Thanks for making it clear. I think it could be a good idea to clearly state it in the docs that if you want progressive enhancement you need to use react native hook.

TheEdoRan commented 4 months ago

As @tenghuan123 said, progressive enhancement is available via useActionState hook from React.

next-safe-action exports a hook called useStateAction from /stateful-hooks path, but I've just tested it and it doesn't work with JavaScript disabled. The execute function returned from that hook is a wrapper of the dispatcher from React's useActionState hook. This is because otherwise useful props (such as status and client input) couldn't be returned from the hook — see the image below. *

Now I'm actually thinking of changing the useStateAction API to just return the [state, action, isPending] original array or remove the stateful hook entirely from the library and tell users to use the React one. If this will be the case, the useStateAction hook will be marked as deprecated for a number of minor versions and then removed.

What do you think?

* useStateAction return object: useStateAction return object

redBaron23 commented 4 months ago

Hey, BTW, do you guys can use the bindArgsSchemas ? I'm new on this so that's the reason I didn't open a new issue, and it could be an error on my side. This is the error that I get

The 'this' context of type 'SafeActionFn<string, ZodObject<{ username: ZodString; }, "strip", ZodTypeAny, { username: string; }, { username: string; }>, [userId: ZodString, age: ZodNumber], any, any, { ...; }>' is not assignable to method's 'this' of type '(this: null, args_0: string, args_1: number, ...args_2: any[]) => Promise<SafeActionResult<string, ZodObject<{ username: ZodString; }, "strip", ZodTypeAny, { username: string; }, { username: string; }>, ... 4 more ..., unknown> | undefined>'.
  Types of parameters 'bindArgsInputs' and 'args_0' are incompatible.
    Type '[string, number, ...any[]]' is not assignable to type '[...bindArgsInputs: any[], input: any]'.
      Source provides no match for required element at position 1 in target.ts(2684)
(alias) const onboardUser: SafeActionFn<string, z.ZodObject<{
    username: z.ZodString;
}, "strip", z.ZodTypeAny, {
    username: string;
}, {
    username: string;
}>, [userId: z.ZodString, age: z.ZodNumber], any, any, {
    ...;
}>
import onboardUser
TheEdoRan commented 4 months ago

@redBaron23 are you using TS 5.5?

tenghuan123 commented 4 months ago

As @tenghuan123 said, progressive enhancement is available via useActionState hook from React.

next-safe-action exports a hook called useStateAction from /stateful-hooks path, but I've just tested it and it doesn't work with JavaScript disabled. The execute function returned from that hook is a wrapper of the dispatcher from React's useActionState hook. This is because otherwise useful props (such as status and client input) couldn't be returned from the hook — see the image below. *

Now I'm actually thinking of changing the useStateAction API to just return the [state, action, isPending] original array or remove the stateful hook entirely from the library and tell users to use the React one. If this will be the case, the useStateAction hook will be marked as deprecated for a number of minor versions and then removed.

What do you think?

  • useStateAction return object: useStateAction return object

First of all, thank you for your outstanding work, next-safe-action is great.

Secondly, I think it is very difficult for a website to fully support progressive enhancement and have good UX, because it has nothing to do with the implementation of the code, but requires many compromises in product design.

Why is this being said? Following the previous example, the user did call the server action in a non-JS state and display the result, but it did not display the effect of isPending. The reason is that isPending is always a state and cannot update itself in non-JS. Therefore, progressive enhancement can only return values through different server actions to obtain successful and failed states, without intermediate transition states.

There is no intermediate state like isPending, which feels very bad for UX, because when the network situation is bad, users may not receive page feedback for a long time, and users may fall into self-doubt, whether it is a software bug or not submitted.

Therefore, I believe that if developers want more states to provide better UX and good DX, they need to use useStateAction provided by next-safe-action. If developers want to support progressive enhancement, they need to use useActionState. In short, useStateAction is an enhanced version of useActionState. Perhaps useStateAction should export the dispatch of useActionState used internally to the outside, because in this way useStateAction only extends the original functionality of useActionState without losing the ability to support progressive enhancement

TheEdoRan commented 4 months ago

@tenghuan123 thank you for your feedback. This issue has also been discussed here:

It would be great to return the dispatcher from useActionState hook too, and it would also be really simple to implement. The problem, as explained in the discussion, is this:

Problem is that if users use dispatcher instead of execute, then input and status properties go out of sync [...] Returning dispatcher from useStateAction would allow writing buggy code, and I already can see the number of issues that would be opened if I implemented that. So, not sure if there's a good solution. If you want progressive enhancement, it's probably best to use the useActionState hook from React.

TheEdoRan commented 4 months ago

Added a section about progressive enhancement on the website: https://next-safe-action.dev/docs/execution/hooks/usestateaction#progressive-enhancement

redBaron23 commented 4 months ago

@redBaron23 are you using TS 5.5?

Yes 5.5.3

TheEdoRan commented 4 months ago

@redBaron23 check out

redBaron23 commented 4 months ago

@redBaron23 check out

Thank you so much. Awesome lib BTW

MrOxMasTer commented 3 months ago

https://github.com/TheEdoRan/next-safe-action/discussions/190#discussioncomment-10098371

MrOxMasTer commented 3 months ago

Here is an example of using useActionState to support progressive enhancement in next-safe-action

do you have any typing errors with this approach? image

https://typescript.tv/errors/#ts2769

MrOxMasTer commented 3 months ago

MrOxMasTer commented Jul 20, 2024 • I understood the reason. In the 2nd argument of useActionState