IdoPesok / zsa

https://zsa.vercel.app
MIT License
436 stars 13 forks source link

Keep errors while submiting to the server #121

Closed andresgutgon closed 1 week ago

andresgutgon commented 2 weeks ago

What?

Hi, I'm using useServerAction to save some data into my server. It's working fine but I see a glitch while isPending

The errors in the server are reset and the UI jumbs.

This is my form:

  const { data, executeFormAction, isPending, error, ...rest } = useServerAction(
    onboarding,
    {
      initialData: { name, username },
    },
  )
  const errorData = error?.data
  const fieldErrors = error?.fieldErrors
  return (
      <Input
        label='Your username'
        type='text'
        name='username'
        errors={fieldErrors?.username}
        defaultValue={data?.username ?? errorData?.username}
        onChange={(e) => setUserName(e.target.value)}
        placeholder='Pick a username'
        description={<UsernameDescription username={currentUsername} />}
      />
  )

This is my action:

export const onboarding = authProcedure
  .createServerAction()
  .input(input, { type: 'formData' })
  .output(input)
  .experimental_shapeError(({ err, typedData }) => {
    if (err instanceof ZSAError && err.code !== 'INPUT_PARSE_ERROR') {
      return {
        code: err.code,
        message: err.message,
      }
    }

    return {
      data: typedData?.inputRaw,
      fieldErrors: typedData?.inputParseErrors?.formattedErrors,
    }
  })
  .handler(async ({ input, ctx: { user } }) => {
    await validateUniqueUsername({ data: input, user })

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

    redirect('/')
  })

This is my UI. Check the disappearing red errors when submitting

https://github.com/IdoPesok/zsa/assets/49499/b17d4941-0091-4758-a184-23fd6c254177

andresgutgon commented 2 weeks ago

Am I using it ok? Is there a way of avoiding this glitch?

andresgutgon commented 2 weeks ago

Also, to keep previous data, I had to use shapeError and get typedData?.inputRaw. How hard would always be to return data already present in the form?

I say this because otherwise, all inputs are empty when one of them have a validation error no?

IdoPesok commented 2 weeks ago

Hi, thanks for bringing this up!

I understand the issue. Going to compare with other libraries and report back shortly with next steps.

In the meantime -- if you want to try -- you should be able to get this persistence with useActionState. This also supports progressive enhancement.

always be to return data already present in the form?

Do you mean to return inputRaw in a default TZSAError without needing to do shape error?

andresgutgon commented 2 weeks ago

Do you mean to return inputRaw in a default TZSAError without needing to do shape error?

Yes!

andresgutgon commented 2 weeks ago

Using state gives me this TS error

I'm in latest zsa. In fact I'm debugging it locally with npm link

image
IdoPesok commented 2 weeks ago

What version of React is this?

IdoPesok commented 2 weeks ago

Also if you are using npm link, do you want to give the input schema functions a shot? Going to merge soon would be awesome if you can test that branch in your project and give feedback.

andresgutgon commented 2 weeks ago

What version of React is this?

I'm living on the edge xD

{
    "next": "14.3.0-canary.59",
    "next-auth": "5.0.0-beta.19",
    "react": "19.0.0-beta-26f2496093-20240514",
    "react-dom": "19.0.0-beta-26f2496093-20240514"
}

Maybe is because of types?

    "@types/react": "^18.2.61",
    "@types/react-dom": "^18.2.19",
andresgutgon commented 2 weeks ago

Also if you are using npm link, do you want to give the input schema functions a shot?

I can try that. But would be ideal to use useServerAction

And out of the box get:

  1. data persisted between attempts
  2. Do not erase errors until the response come from the server (action)

But I think that would be ideal without needing to customize errors or other parts of the lib. It would make a straightforward understand of the model

IdoPesok commented 2 weeks ago

Ah yes, I meant outside of this issue and related to #120 so you don't need to do validate unique username in the handler.

andresgutgon commented 2 weeks ago

Ah!! yes yes I didn't understand you. Let me check that

IdoPesok commented 2 weeks ago

Hi, the formatted errors come from Zod formatted errors. Same thing with field errors, and form errors. These are standard Zod error schemas that people use. We can be more clear in our docs to explain this.

andresgutgon commented 2 weeks ago

I see, but my question is. For having formatted errors do I need to use shapeError. I'll try with stable branch but i think it's working without that method

It does indeed. And also in your PR. All was my fault https://github.com/IdoPesok/zsa/issues/121#issuecomment-2171686993

andresgutgon commented 2 weeks ago

Playing with this https://github.com/IdoPesok/zsa/pull/120 is failing for me

Basically I'm not sure what I have to pass in the input I'm passing a z.object but is failing

input(async ({ ctx }) => z.object(...), { type: 'formData'})
image

I see this in Chrome Dev tools

image

I think that PR could have a regression

The same code in my app gets the error on error key from useServerAction hook

image image

But with that PR I get the error about my ZodObject not being an instance of z.ZodType. No idea what could be wrong. My ZodObject is indeed an instance of zodType

andresgutgon commented 2 weeks ago

Doing these 2 changes it works. No need for custom shapeError

Looks like something related with instanceof but no idea why these 2 return false

image
andresgutgon commented 2 weeks ago

Maybe is related https://github.com/colinhacks/zod/issues/2241 or this one https://github.com/colinhacks/zod/issues/2746

andresgutgon commented 2 weeks ago

ALL_MY_FAULT šŸ˜‚

Oh man, NodeJS is hard. What happened is your monorepo use NPM and I was linking from my monorepo with pnpm link ~/code/opensource/zsa/packages/zsa and it was taking Zod from pnpm and so the instance was different when checking instanceof

What I did instead is

{
  "zsa": "file:../../../../opensource/zsa/packages/zsa",
  "zsa-react": "file:../../../../opensource/zsa/packages/zsa-react",
}

Fuck, this was a good lesson.

Your PR works great input(function) does what it promise šŸŽ‰

image

I'll move now to investigate the problems in this issue

OFF-TOPIC: What's the use case for retry? Reading the code it makes everything more complicated. I'm curious to know what made you implement retries in this package. I can't imagine any good cases

IdoPesok commented 2 weeks ago

Sorry for late response, was traveling this wknd!

Oh man, NodeJS is hard. What happened is your monorepo use NPM and I was linking from my monorepo with pnpm link ~/code/opensource/zsa/packages/zsa and it was taking Zod from pnpm and so the instance was different when checking instanceof

Ah this is super tricky! I was so confused why the instance of wasn't getting the ZodObject in your first message. Good job debugging this.

Your PR works great input(function) does what it promise

Awesome! Happy to hear! Still writing some more tests. Excited to merge it.

What's the use case for retry?

Sometimes if there are rate limits in effect it could be useful to retry. This is also available in libraries such as react-query. I agree it does make things more complicated šŸ˜„ I had a branch with many more features such as polling and caching but figured people just should use react-query with zsa since its already so good.

Okay, I am going to move this conversation to #124 as the code is there.

andresgutgon commented 2 weeks ago

Polling and caching

Yep, these are frontend concerns and because zsa already integrates with react-query there's no need to bring that complexity to this code.

Okay, I am going to move this conversation to https://github.com/IdoPesok/zsa/pull/124 as the code is there.

Nice, happy to change anything