Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[✨] complex form should not flatten errors #5463

Closed
tzdesign opened this issue Nov 20, 2023 · 3 comments · Fixed by #6568
Closed

[✨] complex form should not flatten errors #5463

tzdesign opened this issue Nov 20, 2023 · 3 comments · Fixed by #6568
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request

Comments

@tzdesign
Copy link
Contributor

tzdesign commented Nov 20, 2023

Is your feature request related to a problem?

We have a complex Zod Type for cart.

If an error occurs in one of the fields at the lowest level like:

cart.invoiceAddress.firstName

The errors are flatten within cart:

invoiceAddress: string[]

Example multiple fields:

{
  "invoiceAddress": [
    "String must contain at least 2 character(s)",
    "String must contain at least 2 character(s)",
    "String must contain at least 2 character(s)",
    "String must contain at least 1 character(s)",
    "String must contain at least 2 character(s)",
    "String must contain at least 2 character(s)",
  ]
}[]
 {
    code: 'too_small',
    minimum: 2,
    type: 'string',
    inclusive: true,
    exact: false,
    message: 'String must contain at least 2 character(s)',
    path: [ 'invoiceAddress', 'street' ]
  },
  {
    code: 'too_small',
    minimum: 1,
    type: 'string',
    inclusive: true,
    exact: false,
    message: 'String must contain at least 1 character(s)',
    path: [ 'invoiceAddress', 'streetNumber' ]
  },
  {
    code: 'too_small',
    minimum: 2,
    type: 'string',
    inclusive: true,
    exact: false,
    message: 'String must contain at least 2 character(s)',
    path: [ 'invoiceAddress', 'zip' ]
  },

SEE: #4634

Describe the solution you'd like

We need to access the specific fields with the path.

action.fieldErrors.cart.invoiceAddress.firstName 
// or
action.fieldErrors.get('cart.invoiceAddress.firstName)

Describe alternatives you've considered

We either have to overwrite the types so that the errors are not flattened for complex forms or we send the errors unflattened in a second field.

Additional context

No response

@tzdesign tzdesign added STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request labels Nov 20, 2023
@tzdesign
Copy link
Contributor Author

@ulic75 I created an issue so maybe someone can check this out.

@mhevery
Copy link
Contributor

mhevery commented Dec 10, 2023

@wmertens do you have an opinion on this?

@tzdesign
Copy link
Contributor Author

@mhevery @wmertens

What we are doing is skipping zod and doing this:

export const useMyCoolAction = routeAction$(async (rawData, event) => {
  try {
    const data = someSchema.parse(rawData);
  } catch (error) {
    if (error instanceof z.ZodError) {
      event.fail(400, flattenZodIssues(error.issues));
    }
  }
});

The flattenZodIssue:

export default function flattenZodIssues(issues: z.ZodIssue | z.ZodIssue[]) {
  issues = Array.isArray(issues) ? issues : [issues];

  return issues.reduce((acc, issue) => {
    acc[issue.path.join('.')] = issue.message;
    return acc;
  }, {} as Record<string, string>);
}

export type FlattenZodIssues = ReturnType<typeof flattenZodIssues>;

tzdesign pushed a commit to tzdesign/qwik that referenced this issue Jun 18, 2024
On very complex nested types, you loose the location or the field the error belongs to. Closes QwikDev#5463

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

closes QwikDev#5463
gioboa added a commit that referenced this issue Jul 9, 2024
* feat(qwikcity/actions): dotnotation field-errors

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

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

closes #5463

* fix types and adding test

* make the type partial

* introduce arrays to the type and flattenErrors

* run api.update

* chore: run api.update

* chore: run api.update

* adding array to the test

---------

Co-authored-by: Tobias Zimmermann <ich@tzdesign.de>
Co-authored-by: gioboa <giorgiob.boa@gmail.com>
Co-authored-by: PatrickJS <github@patrickjs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants