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

Make errors always be an array #212

Closed
kentcdodds opened this issue Jul 14, 2023 · 5 comments
Closed

Make errors always be an array #212

kentcdodds opened this issue Jul 14, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@kentcdodds
Copy link
Contributor

I have this reusable utility for displaying errors:

function ErrorList({
	id,
	errors,
}: {
	id?: string
	errors?: Array<string> | null
}) {
	return errors?.length ? (
		<ul id={id} className="flex flex-col gap-1">
			{errors.map((error, i) => (
				<li key={i} className="text-foreground-danger text-[10px]">
					{error}
				</li>
			))}
		</ul>
	) : null
}

Unfortunately, because Conform errors can sometimes be a single string, I have to adjust it to accommodate that:

function ErrorList({
	id,
	errors,
}: {
	id?: string
	errors?: Array<string> | string | null
}) {
	if (!errors) return null
	errors = Array.isArray(errors) ? errors : [errors]

	return errors.length ? (
		<ul id={id} className="flex flex-col gap-1">
			{errors.map((error, i) => (
				<li key={i} className="text-foreground-danger text-[10px]">
					{error}
				</li>
			))}
		</ul>
	) : null
}

That just kinda feels weird to me. Can we just have the errors always be an array? If people just want to display the first one they can always [0] to get it.

@kentcdodds
Copy link
Contributor Author

I would add that it would simplify things to remove the .error property as well. It may feel "convenient" because it means people wouldn't have to use field.errors[0] to get the first error, but it adds to confusion because it's another way to do the same thing which adds API surface area and makes people stop and have to make a decision between using field.error and field.errors when there's minimal useful difference.

@edmundhung
Copy link
Owner

Conform errors can sometimes be a single string

This sounds like a bug to me as the errors property should always be string[] or undefined. But I am unable to reproduce a case where errors return a string yet. Any chance you have a repo that reproduce the problem?

@edmundhung edmundhung added the help wanted Extra attention is needed label Jul 15, 2023
@kentcdodds
Copy link
Contributor Author

Ah, the place I was getting hung up is because as one of the steps for the workshop I have this:

	const fieldErrors =
		actionData?.status === 'error' ? actionData.submission.error : null
//...

<ErrorList id={titleErrorId} errors={fieldErrors?.title} />

And that is typed as Record<string, string | string[]>. Any chance that could be changed to Record<string, string[]>?

@edmundhung edmundhung added enhancement New feature or request and removed help wanted Extra attention is needed labels Jul 17, 2023
@edmundhung
Copy link
Owner

Fixed on v0.8.0.

@kentcdodds
Copy link
Contributor Author

Thank you :)

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

No branches or pull requests

2 participants