seasonedcc / composable-functions

Types and functions to make composition easy and safe
MIT License
666 stars 14 forks source link

Implement nested errors in errorMessagesForSchema #26

Closed diogob closed 2 years ago

diogob commented 2 years ago

Purpose

When a domain function received nested input data, errorMessagesForSchema should mirror that structure so we do not lose nested error information. This should address #24

diogob commented 2 years ago

@danielweinmann I don't think this will cause any issues for remix-forms, but I haven't done any integration testing. Thoughts?

diogob commented 2 years ago

@gustavoguichard @danielweinmann reading the Zod docs on error handling I realized that we should use those functions and avoid doing our little zod instrospection. However this would probably involve breaking changes, since currently the errorMessagesForSchema takes a SchemaError[] where we already lost all the zod data.

The current implementation of errorMessagesForSchema is broken by design, in the sense that it either breaks the zod abstraction or remains inconsistent with inputErrors.

My suggestion:

We leave errorMessagesForSchema as is, but deprecate it. We add a new property to ErrorResult to retrieve structured errors, we could even borrow the zod naming and call it formatInputErrors().

So interface to get errors for the form would be something like

const errors = result.formatInputErrors()

The result type could be the same as format() or we could tweak it a bit, perhaps making it closer to the NestedErrors type present in the current PR.

gustavoguichard commented 2 years ago

@diogob if you feel like waiting for my opinion here I'd ask you to wait for my return on the 29th bc I need to wrap my head around it to understand the implications.

That said I trust you guys to take that decision regardless bc I know some changes must happen as I could see that request coming when we designed the shallow version ;)

diogob commented 2 years ago

@gustavoguichard I have just now (as I started thinking about the compositions) realized how deep this rabbit hole goes, don't worry we won't rush this. It actually seems that this will be a major release.

diogob commented 2 years ago

31 seems to be a simpler approach that uses only the data contained in SchemaError thus avoiding inspection of the schema to generate the nested structure.