IdoPesok / zsa

https://zsa.vercel.app
MIT License
761 stars 23 forks source link

Upgrade from zsa@0.2.3 to 0.3.3 keeps the action loading forever #107

Closed andresgutgon closed 3 months ago

andresgutgon commented 3 months ago

What is the error?

Hi. I'm was working on a project using older versions and I moved to latest to have new ZSAError and the code that's working in older versions now leaves my UI loading

image
-    "zsa": "^0.2.3",
-    "zsa-react": "^0.1.3"
+    "zsa": "^0.3.3",
+    "zsa-react": "^0.1.4"

This is my action

async function checkUsername(username: string, user: SafeUser) {
  const foundUsers = await db
    .select({ id: users.id })
    .from(users)
    .where(and(eq(users.username, username), ne(users.id, user.id)))
    .limit(1)
  return foundUsers.length > 0
}

const input = z.object({
  name: z.string().min(3),
  username: z
    .string()
    .min(3, { message: 'Username must be at least 3 characters long.' })
    .max(20, { message: 'Username cannot be longer than 20 characters.' })
    .regex(/^[a-zA-Z0-9_\-.]*$/, {
      message:
        'Invalid characters in username. Only leters, numbers and "-" without spaces. No accents.',
    }),
})

type Input = z.infer<typeof input>

async function validateUniqueUsername({
  data,
  user,
}: {
  data: Input
  user: SafeUser
}) {
  const asyncInput = input.extend({
    username: input.shape.username.refine(async (username) => {
      return checkUsername(username, user)
    }, { message: 'Username is already taken.' }),
  })

  const uniqueError = await asyncInput.safeParseAsync(data)
  console.log("uniqueError", uniqueError)
  if (uniqueError.success) return

  const flattenedErrors = uniqueError.error.flatten()
  const formattedErrors = uniqueError.error.format()
  throw new ZSAError('INPUT_PARSE_ERROR', uniqueError.error, {
    inputParseErrors: {
      fieldErrors: flattenedErrors?.fieldErrors,
      formErrors: flattenedErrors?.formErrors,
      formattedErrors: formattedErrors,
    },
  })
}

export const onboarding = authProcedure
  .createServerAction()
  .input(input, { type: 'formData' })
  .output(input)
  .handler(async ({ input, ctx: { user } }) => {
    await validateUniqueUsername({ data: input, user })

    const result = await finishOnboarding({ user, data: input })
    const value = result.unwrap()

    // NOTE: This ends in JWT callback in auth/index.ts
    await updateSession({
      user: { name: value.name, username: value.username },
    })

    redirect('/')
  })
andresgutgon commented 3 months ago

I don't know what more details I can give. Basically moving back to older versions the code execute and redirect to /

IdoPesok commented 3 months ago

Hi, happy to help here. Checking out the code now

andresgutgon commented 3 months ago

Wow! awesome response time!

IdoPesok commented 3 months ago

Are you using useServerAction on the client to get the isPending state?

andresgutgon commented 3 months ago

Yes

IdoPesok commented 3 months ago

Are you possibly redirecting to the same page ? IE, is this form on / ?

andresgutgon commented 3 months ago

No, it's on another page: /signin/onboarding. Here is the form

IdoPesok commented 3 months ago

Just for background, the new version uses useTransition to wait for the redirect to finish. Before it would be awkward because redirect was still going on but isPending would go to false. In your case it would mean the sign in button would be enabled but in the browser is still loading. Now, isPending remains true until revalidatePath or redirect are done.

andresgutgon commented 3 months ago

Just for background, the new version uses useTransition to wait for the redirect to finish. So before it would be awkward because redirect was still going on but isPending would go to false. Now, isPending remains true until revalidatePath or redirect are done.

So I can't do the redirect on the server action?

Anyway it fails even if there is an error and the redirect is not called

IdoPesok commented 3 months ago

You can do redirect on the server action, the new updates should have fixed weird behavior with it and the isPending state. There is also #97 that seems to be the same issue, but I haven't been able to replicate locally.

andresgutgon commented 3 months ago

Yes, that issue looks similar. I have to go. I'll keep investigating. My code is open so feel free to check if you want: https://github.com/andresgutgon/readfort/pull/10

This lib looks awesome. I have a bit more of feedback but first I'll try to fix this one

IdoPesok commented 3 months ago

I think I know the issue, will report back in ~ 10 min

IdoPesok commented 3 months ago

By the way, .input() supports async parsing so in theory you don't need the validateUniqueUsername function and can just do:

[update] I was wrong, user will not be available in this scope. Will think about how to enable this

export const onboarding = authProcedure
  .createServerAction()
  .input(
    input.extend({
        username: input.shape.username.refine(async (username) => {
            return await checkUsername(username, user)
          }, 
          { message: 'Username is already taken.' }
        ),
    }), 
     { type: 'formData' }
   )
  .output(input)
  .handler(async ({ input, ctx: { user } }) => {
    await validateUniqueUsername({ data: input, user })

    const result = await finishOnboarding({ user, data: input })
    const value = result.unwrap()

    // NOTE: This ends in JWT callback in auth/index.ts
    await updateSession({
      user: { name: value.name, username: value.username },
    })

    redirect('/')
  })
IdoPesok commented 3 months ago

I think the issue here is you can't use useServerAction execute method as the action in a form. We have a lot of documentation about forms here. I will add this warning to the docs. Our bad on not already including this and creating confusion.

You have two options:

1) Run execute via the onSubmit handler

Screen Shot 2024-06-09 at 1 48 26 PM

2) If you want to support progressive enhancement, you can use useActionState. We have docs on this as well. Although, to keep your toast showing you will need to create a custom action function.

andresgutgon commented 3 months ago

By the way, .input() supports async parsing so in theory you don't need the validateUniqueUsername function and can just do:

yes! that was another piece of feedback. I can't use your example. my validation use user which is in the context of the action. I didn't found a way to pass context to input

IdoPesok commented 3 months ago

Oh true! Didn't catch that. Good point, will think about how to help in that situation. I guess input can also be a function that takes in the ctx and returns the schema.

andresgutgon commented 3 months ago

I think the issue here is you can't use useServerAction execute method as the action in a form.

Sorry I'm trying to understand why using in this way doesn't work. it worked in the old version. Would be awesome to understand what changed

IdoPesok commented 3 months ago

The change was that execute now internally wraps the action in startTransition from the useTransition hook. I just did some testing with this and its pretty interesting why it isn't working.

Basically, the useServerAction execute function waits for isPending from the transition to be set to false before it resolves. However, when you use <form action={execute} /> weird case arises:

isPending from the transition will only be false once the form's action resolves. However, the form can't resolve until isPending is false. So this creates a state where isPending is stuck on true.

"use client";

import { useCallback, useEffect, useRef, useTransition } from "react";
import { zsaAction } from "./action";

export default function useExecute() {
  const [isPending, startTransition] = useTransition();
  const resolveRef = useRef<any>(undefined);

  const execute = useCallback(async (formData: FormData) => {
    return await new Promise((resolve) => {
      resolveRef.current = resolve;
      startTransition(() => {
        zsaAction(formData);
      });
    });
  }, []);

  useEffect(() => {
    if (isPending) return;

    // resolve once the action is done
    resolveRef.current?.();

    return () => {
      resolveRef.current = undefined;
    };
  }, [isPending]);

  return {
    isPending,
    execute,
  };
}

We can't do zsaAction(formData).then(resolve) because the transition still may be running and the only way we have found to figure out when the transition actually ends is to useEffect #91. This creates a lock state with the form.

Hopefully this makes sense! Going to try to think of a solution so maybe using it with execute is possible.

IdoPesok commented 3 months ago

Hi, wanted to check back in and report an update in the latest version of zsa-react@0.1.5. You can now use useServerAction alongside <form action={...} /> using the new function executeFormAction

Here are the docs

Working on the input functions next, will update when that is done.

andresgutgon commented 3 months ago

executeFormAction it works wonderfully

Looking forward the input context : )