-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
[Bug]: All server errors get cleared after focus changes or any field value changes #390
Comments
Hi! The current behavior is intentional, but this is definitely somewhere we can make improvements. Definitely open to ideas and discussion here!
With Remix, I've personally found these sorts of server-side validations are pretty easy to simply return from the action and display manually, but I can see the appeal of being able to write these directly into the server-side validator as an async refinement. I already have a few ideas for how to implement either a |
So regarding the Remix action I do see the appeal of assuming the validator will always be exactly the same and that any server errors can be effectively ignored on any change since it greatly simplifies what the library has to deal with, but I don't think it appropriately reflects real world use. Displaying errors directly returned from Remix actions seems like a pretty involved workaround to be honest, since you'd have to basically always have 2 possible sources of errors (server errors and errors managed by RVF) and keep track of field changes to stop displaying server errors for a field when that particular field's value changes (which kind of is the reason we use form libraries instead of rolling our own thing). And yeah I mentioned |
The remix
Having the ability to re-use your client validation on the server is a core value proposition of RVF. There are definitely use-cases where it makes sense to create a separate server validator (I do this a lot with different subactions in a single action function). But validator re-use is the default case. This is also important for progressive enhancement. Imagine a case where a user with a poor internet connection submits a form before JS has loaded. RVF can still handle that because the validator on the server will run and the action will return the validation errors that would normally be surfaced on the client-side. This is why validation behaves the same with server errors as it does with client-side errors. If it suddenly behaved differently in this case, that would be a bug.
This is true. In those cases though, you should be re-running your client-side validations then perform your server-side validations. I do this by validating with the validator, then doing other checks. But you could build a separate validator that contained all the client-side validations + the server validations if you wanted. If you don't run your client-side validations on the server, a tech-savy user can easily get around them by calling So the behavior we have can't really change, but we can add new functionality. One idea I had was something like this. If we went this route, it would be complimented by a // Run validations as before
const result = await validator.validate(await request.formData())
if (result.error) return validationError(result.error, result.submittedData);
// Return custom errors explicitly
const isEmailTaken = await checkIfEmailTaken(result.data.email);
if (isEmailTaken) return customError({ email: "That email is taken" }); |
I don't think that's actually the case though, because on the client all errors don't get cleared when focus changes or a single field value changes (I know this is actually what is implemented since the validator runs on every keystroke, but the resulting behavior is not this, and users don't experience client validation error messages disappearing on focus, which does happen with server errors). This leads to having behaviors like this: rvf.server.validation.movIn this video server validation errors aren't displayed after the first submission (done by pressing Admittedly arguing validation doesn't behave the same with server vs client errors is a bit of a stretch because the implementation is similar. But the final behavior is not at all the same, and I'd say the server validation behavior is not correct (see video above). This behavior assumes no devs will ever want to run any additional validations on submit on the server. What I'm proposing is kind of a compromise where we admit that RVF does accept the fact that some devs could have additional validations on the server that only run when the form is submitted, and that those errors shouldn't go away when a user focuses another field or changes another un-related field. The implementation of this could look like keeping track of server errors and removing them for fields only after they are modified. That doesn't necessarily mean the error will not happen on the server, but I think it's the most reasonable thing (btw it's the same thing that react-hook-form does with it's None of this would break server validations or non-JS users. I'm not sure why you mentioned this. The case I'm making that server validations could have additional validations to the ones the client does (not fewer validations). Right now RVF doesn't support this.
Yeah this is true. You'd be guessing. But like I mentioned I do think it's reasonable to assume that when the field value changes you can clear the server validation error. That enabled what I think is what most devs would expect from a form library. Or more complex behaviors could be implemented by default or be configurable (like not showing the server validation error when the field value is different from what was submitted to the server, not just clearing it when the field value is changed). |
I get the feeling we're talking past each other a bit. 🫤
I'm right here with you. RVF could have better support for this. And I'm open to discussion on API ideas. It's just that changing the way RVF currently treats server errors is off the table. Returning custom server errors needs to be explicit, IMO. |
Ok yeah I get that sentiment. —- I feel like having a whole separate API just for server validation errors that “persist” is a bit too complex, but it’s a valid concern to want to keep it explicit and separated. And you’re thinking that this API would be the one React users have to use when they’re not using Remix and want to show server validation errors on specific fields? |
Those errors would use this hypothetical new API, so the behavior in the video becomes a non-issue.
I definitely feel this. Fewer APIs is usually preferable, so a new API needs a compelling reason to exist. As a reference point, I think if we can come up with something better than returning the custom errors from json manually, then it's worthwhile because that's the approach I usually recommend and use. Worth thinking about if we can fit form-level errors into this too (only a nice-to-have). That's been requested before and I usually give the same recommendation.
It would be for both. |
And what are you thinking for how those server validation errors from the new API get dismissed once the form values start to be modified again on the client? |
That's definitely an option. Any given form likely only needs one or two custom server errors so the burden there is small. It's certainly less work than managing your own state. One potential API design is to tie this into a hypothetical I haven't implemented a |
what do you think about a |
That might be okay. The only thing I might worry about there would be API creep. A rough idea of the server-side api would need to cover at least this. I don't love how verbose it is, but we can optimize from here. return customError({
// We're able to supply formId automatically with `validationError`,
// so maybe there's a way to do that here so you don't have to remember yourself.
formId: validationResult.formId,
errors: {
// for manual validation
email: "That email is taken",
// or with custom options
password: {
password: "Password error",
clearOn: "change"
}
},
submittedData: omitPassword(validationResult.submittedData)
}); |
I am using remix with zod Hope this gets solved 🙏. Ideally, with support for e.g. remix-utils useDebounceSubmit(). |
Which packages are impacted?
@rvf/react
@rvf/remix
@rvf/zod
@rvf/yup
zod-form-data
What version of these packages are you using?
Please provide a link to a minimal reproduction of the issue.
https://stackblitz.com/edit/remix-run-remix-g89vpl?file=app%2Froutes%2F_index.tsx
Steps to Reproduce the Bug or Issue
Go to https://stackblitz.com/edit/remix-run-remix-g89vpl?file=app%2Froutes%2F_index.tsx
Instructions:
Form will load, get errors from backend, but then email field will be focused and server errors will be cleared for all fields.
This happens whenever the following 3 are true:
This can be confirmed by viewing the error history below.
If you press "Create account" without focus being in any field, then errors show up correctly. But again, as soon as you focus another field, focus no field, or change the value in 1 field, all backend are cleared.
Expected behavior
Ideally all server errors would be kept visible until that field is modified.
I think it's support for server validation errors should be as good as client side validation errors.
Related to this: It would be great (and I think kinda necessary if this library is meant to be independent of Remix) if it's also possible to show server validation errors on specific fields when not using remix actions.
When only using
@rvf/react
, I couldn't find any docs mentioning how to support server validation errors (after the form is submitted).I tried using
serverValidationErrors
(which btw gives a type error whenuseForm
is imported from@rvf/remix
, but works when it's imported from@rvf/react
), but it has the same issue described above: all the errors are cleared when focus changes or a field is modified.react-hook-form
has thesetError
API and has the behavior I described above about errors not clearing until a field is changed.Screenshots or Videos
No response
Platform
Additional context
No response
The text was updated successfully, but these errors were encountered: