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

Inconsistencies #13

Open
hickscorp opened this issue Feb 15, 2019 · 0 comments
Open

Inconsistencies #13

hickscorp opened this issue Feb 15, 2019 · 0 comments

Comments

@hickscorp
Copy link

hickscorp commented Feb 15, 2019

From the docs:

image

I'm trying to make impossible states impossible, and I don't seem to be able to.

  • If an unsuccessful result is always null, then what's the point of the successful fields?
  • The typings for the validation messages is something like: (ValidiationMessage | null)[] | null. The docs says that it's empty when there are no errors - but it can also be null. Which on is it?
  • The validation messages are typed in a way that individual messages can be null. What does that mean?

Generally speaking, why not make the return type impossible to represent something impossible?

  • I would enforce that the messages must be either ValidiationMessage[] or ValidiationMessage[] | null. Having individual null messages is just both confusing and adds lots of uncertainty about the meaning of such cases.
  • IMO saying that result will always be null when unsuccessful is a mistake. Sometimes a user posting a form can be partially successful. Having a result with partial changes applied and a list of errors models this already. I think there should be no success flag at all - extracting success from a payload should be as easy success = () => messages || message.length == 0 || !result.

In our case, having to handle these cases has shown to be very laborious, and particularly misleading for the frontend team, especially when using more strong typed languages that aim at eliminating runtime errors (Elm, TS, etc). The fact that an impossible state is made possible makes the frontend developer have to address it, regardless of how absurd it is.

Maybe a bit of love on the base Payload concept would be great before too much adoption?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant