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

Error should be allowed to be any #2134

Open
adamscybot opened this issue Dec 18, 2019 · 16 comments
Open

Error should be allowed to be any #2134

adamscybot opened this issue Dec 18, 2019 · 16 comments

Comments

@adamscybot
Copy link

🐛 Bug report

Current Behavior

Currently, errors must be strings.

Expected behavior

Errors can be any object/array/primitive

Reproducible example

The types complain if you attempt to do anything but string.

Suggested solution(s)

Just change the types to allow any on errors.

Additional context

Only strings as errors is very limiting. Sometimes I want to render a special interactive error (like with an actionable button). For that to be possible, I want to set some keys and props for example into the error state.

It's also quite limiting for an internationalisation perspective. You'd usually want to set an object with the keys/message arguments so you can then plug that into the intl lib on render.

I think the base JS is actually fine, its just setting stuff in state. Its potentially only the TS types preventing the use case.

@rafalp
Copy link

rafalp commented Jan 5, 2020

To offer extra context, I'm currently integrating with GraphQL server that returns errors in this shape:

{
  location: Array<string>
  message: string
  type: string
}

My error can look like this:

{
  location: ["name"],
  type: "value_error.username.not_available",
  message: "Username is not available."
}

I would like to display pre-defined error in the UI for some types, and fallback message for other types.

@jaredpalmer
Copy link
Owner

Can you transform the errors into key value that matches Formik’s shape?

@DalerAsrorov
Copy link

@rafalp I also use GraphQL for hydrating the Formik fields for initial render (on edit) and I am just normalizing the records before passing them on to Formik error props. Not sure if the library-wise changing is needed. However, making Error of type any makes sense @jaredpalmer

@jaredpalmer
Copy link
Owner

Formik makes assumptions about shape of errors for both getFieldProps, useField and to check validity. getFieldProps/useField/Field may not work if you don’t adhere to to this contract

@rafalp
Copy link

rafalp commented Jan 6, 2020

Ok, more on my usecase then:

My API returns errors as described above. Some of those errors are familiar to me (because they came from my code) and some aren't (because they came from upstream API that my server proxies to, and one I have no control over because those are 3rd party extensions). I have simple component called FieldError that takes error code and list of predefined error messages for recognised codes:

                <FieldError
                  error={errors.name}
                  messages={{
                    "value_error.missing": <Trans>Enter your name.</Trans>,
                    "value_error.any_str.min_length": (
                      <Plural
                        value={settings.usernameMinLength}
                        one={
                          <Trans>
                            Name should be at least # character long (it has {values.name.length}).
                          </Trans>
                        }
                        other={
                          <Trans>
                            Name should be at least # characters long (it has {values.name.length}).
                          </Trans>
                        }
                      />
                    ),
                    "value_error.any_str.max_length": (
                      <Plural
                        value={settings.usernameMaxLength}
                        one={
                          <Trans>
                            Name should be at least # character long (it has {values.name.length}).
                          </Trans>
                        }
                        other={
                          <Trans>
                            Name should be at least # characters long (it has {values.name.length}).
                          </Trans>
                        }
                      />
                    ),
                  }}
                />

My plan was that if error type was not recognised by client, I would fallback to server-provided message as it is. I could duplicate list of recognised error types just for GraphQL hydrating step but this seems error prone to me: I will have to make sure that both lists are in sync when I'm adding new error code to the form (and when speaking about dashboard, I'll likely have dozen of forms to take care of).

As fall-back I've been contemplating something quick and dirty like converting errors to type:message strings, and doing splitting logic in FormError.

@jaredpalmer
Copy link
Owner

I think this is a solid solution IMHO

@adamscybot
Copy link
Author

adamscybot commented Jan 7, 2020

Thanks for reopening :).

The solution above is sensible if you want to render some dynamic non-string content in simple use cases -- i.e. internationlisation with known message parameters. I'm definitely not suggesting we should allow, say, already rendered components to be set in the state. I have actually ended up with something similar, but I've still needed to pass an object in the error state rather than a plain string.

The reason is just a string doesn't work if you have any kind of parameterisation needed to render the error -- unless you resort to stuffing the string and splitting it out again . For example, I may need to pass down some ID that is useful for the error rendering. Like rendering a button to open a link to a specific entity in the error. There are many other examples where there needs to be some dynamic data associated with the error so it can be rendered in a certain way and have certain interactions. A string isnt a rich enough type to represent these. Instead I'd want something like:

{ 
    code: 'LINKED_ENTITY_BROKEN',
    id: '2'
}

Then I'd do something like this in my field impl:

{error.code === 'LINKED_ENTITY_BROKEN' && <Link=`/link/to/thing/${error.id}`}>Thing is broken. Click here to address it. </Link>}

One workaround right now is to override the types. It still works. Yes Formik rightly makes some assumptions about what's in there. But the developer can work with those assumptions since (as far as I know?) it assumed no error if its nullish. So the dev can remove his object and set null when he/she wants to remove the error. This is what I'm doing right now (with type overrides). We only need these extra parameters when you actually have an error to render anyway.

Another solution is to shove a JSON serialised string down there but that's obviously hacky.

Most of the solution can be in userland, its just Formik needs to allow a richer description of the error than a string can support because in advanced use cases theres metadata attached to that error you need to access. Those cases are often (always?) the ones where the error is also represented by more complex controls than a string and so can not be precomputed before being passed into formik.

@adamscybot
Copy link
Author

Adding another use case: You want to have an array of errors and display them with more complex separation than string concatenation. I.e. a bulleted list.

In this instance the error can be an array and the consumer decides what to do with that.

@stale stale bot added the stale label Apr 5, 2020
@adamscybot
Copy link
Author

@jaredpalmer Digging up an old issue, but given above would you be open to a PR to provide this? Appreciate there may need to be more discussion.

@adamscybot
Copy link
Author

adamscybot commented Feb 11, 2021

@jaredpalmer I hate to cause a ping here, I know how annoying it can be. I'm avidly following Formik 3 development and I think it's great. I've been really pleased with the advancements in the latest version; and I've already benefited from those from your contributions and I'm truly grateful for it. I ping this because I think that it is a perfect time to consider this issue. Fundamentally I think "errors as objects" is a good cause which could positively affect the architecture.

@github-actions github-actions bot removed the stale label Aug 19, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 60 days

@github-actions github-actions bot added the stale label Sep 18, 2021
@artursvonda
Copy link

Adding another use case: You want to have an array of errors and display them with more complex separation than string concatenation. I.e. a bulleted list.

In this instance the error can be an array and the consumer decides what to do with that.

We actually have similar use-case where we want to display multiple errors per field. This would be greatly appreciated.

@github-actions github-actions bot removed the stale label Oct 27, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 60 days

@github-actions github-actions bot added the stale label Nov 26, 2021
@adamscybot
Copy link
Author

adamscybot commented Nov 26, 2021 via email

@johnrom
Copy link
Collaborator

johnrom commented Nov 29, 2021

Errors should be configurable, possibly unknown, definitely not any; preferably like:

interface YourErrorShape {
  message: string,
}

const customFormik = createFormik().withErrors<YourErrorShape>(
  // configure the default error mapping from validateField, validateForm, etc
  (error, name) => ({ message: `OOPS: ${error}` })
);

const myForm = () => <customFormik.Formik initialErrors={{ yourField: { message: "Initial error" }}}>
  <Etc />
</customFormik.Formik>

@sergei-maertens
Copy link

I also think the approach of a generic here would be great, and for backwards compatibility you can leave the default be string.

We have a use case where a field can have multiple validators, spitting out validation messages with a hierarchy - so we want to order by this hierarchy in the error display component.

Another example is a field with a pattern that would only allow digits and has a max length of 9 characters (like a Dutch SSN) - you want to display both "you must have a least 9 digits" and "all characters must be digits" instead of letting the user type an alphanumeric string up to 9 chars only to the display that they need to be numbers.

I'm thinking now that the only sensible workaround would be to come up with our own error structure, throw that into JSON.stringify to set it in Formik and then in the display component JSON.parse it again. Which is... far from ideal.

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

No branches or pull requests

7 participants