qwik icon indicating copy to clipboard operation
qwik copied to clipboard

feat(qwikcity/actions): dotnotation field-errors

Open tzdesign opened this issue 1 year ago • 5 comments

Overview

On very complex nested types, you loose the location or the field the error belongs to.

BREAKING CHANGE:

The fieldErrors type on the action changed from Record<string,string[]> (simplified) to Record<string,string | string[]> where the key is now dot notation

Closes #5463

More context

@ulic75 added the possibility for complex form with the PR #4634 and we were trying to use it in our cart. As you might imagine, the cart type is very complex and has more than one level. Example action with comparable type here:

export const useAddressFormAction = routeAction$(
  () => {},
  zod$((z) =>
    z.object({
      email: z.string(),
      address: z.object({
        street: z.string(),
        city: z.string(),
        zip: z.string(),
      }),
    })
  )
);

export default component$(() => {
  const addressAction = useAddressFormAction();
  return (
    <Form action={addressAction}>
      <input
        type="email"
        name="email"
        class={{
          error: addressAction.value?.fieldErrors?.email?.length ?? 0 > 0,
        }}
      />
      <br />
      <input
        type="text"
        name="address.street"
        class={{
          error: addressAction.value?.fieldErrors?.['address']?.length ?? 0 > 0,
        }}
      />
      <br />
      <input
        type="text"
        name="address.city"
        class={{
          error: addressAction.value?.fieldErrors?.['address']?.length ?? 0 > 0,
        }}
      />
      <br />
      <input
        type="text"
        name="address.zip"
        class={{
          error: addressAction.value?.fieldErrors?.['address']?.length ?? 0 > 0,
        }}
      />
      <br />

      <button>Send</button>
    </Form>
  );
});

As you can see. You can't determine if address.street is failing exactly, as the errors on address are just an array of strings.

With my approach the errors are the following type:

type FieldErrors = Partial<
    Record<
      | "email"
      | "address.street"
      | "address.city"
      | "address.zip",
      string
    >
  >

With this in mind, the name of the input and the field in fieldErrors will be exactly the same. And what's impossible now, becomes possible:

<input
        type="email"
        name="email"
        class={{
          error: addressAction.value?.fieldErrors?.email
        }}
      />
      <br />
      <input
        type="text"
        name="address.street"
        class={{
          error: addressAction.value?.fieldErrors?.['address.street'] !== undefined,
        }}
      />
      <br />
      <input
        type="text"
        name="address.city"
        class={{
          error: addressAction.value?.fieldErrors?.['address.city'] !== undefined,
        }}
      />
      <br />
      <input
        type="text"
        name="address.zip"
        class={{
          error: addressAction.value?.fieldErrors?.['address.zip'] !== undefined,
        }}
      />

Arrays

Since @ulic75 also made something like person.0.name possible, I had to change the type for fieldErrors to string | string[]. This means if you have arrays in your validation there are two things which can happen.

The array is missing in the provided data:

fieldErrors will have the property arrayName[] with an array as type.

Example output:

{
   "arrayName[]": ["Required"]
}

The array is NOT missing, but data does not validate:

For example you have addresses like {street: string, zip: number}[] as validation type. You pass address street, but forgot about zip. You will get this:

{
   "addresses[].zip": ["Required"]
}

What is it?

  • [x] Feature / enhancement
  • [ ] Bug
  • [x] Docs / tests / types / typos

Checklist:

  • [x] My code follows the developer guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have made corresponding changes to the documentation
  • [x] Added new tests to cover the fix / functionality

tzdesign avatar Jun 18 '24 10:06 tzdesign

Deploy request for qwik-insights rejected.

Name Link
Latest commit 9ec0a3250ab84df1beceb385a04465af687a0f9e

netlify[bot] avatar Jun 18 '24 10:06 netlify[bot]

@gioboa this is my idea of fixing #5463

Let me know what you think. Added the infos to the docs. As @mhevery and @wmertens was mention in #5463. Maybe you have an opinion on that?

We are now in production without passing validation and using this flattenZodMap locally in the action body. Not perfect, but with this pr, we could refactor passing validation again.

Did a draft, as I had issues linking this. Some dirty task for spa_init. Not sure if I am the cause.

tzdesign avatar Jun 18 '24 10:06 tzdesign

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

commit: 9ec0a32

@builder.io/qwik

npm i https://pkg.pr.new/@builder.io/qwik@6568
@builder.io/qwik-city

npm i https://pkg.pr.new/@builder.io/qwik-city@6568
eslint-plugin-qwik

npm i https://pkg.pr.new/eslint-plugin-qwik@6568
create-qwik

npm i https://pkg.pr.new/create-qwik@6568

templates

pkg-pr-new[bot] avatar Jun 18 '24 13:06 pkg-pr-new[bot]

@gioboa I added the array field.

I was reading the code and thought about arrays in general.

My change is helping a lot with deep nested objects, but it's the same kind of unhelpful with complex forms having arrays with objects and arrays.

I guess we could make it even more complex, but I don't think it's necessary for 99% of the people out there.

image

tzdesign avatar Jun 20 '24 09:06 tzdesign

Thanks @tzdesign :rocket: for this great piece of code

gioboa avatar Jun 20 '24 09:06 gioboa