-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Fix validation errors from resolvers are not translated #9191
Fix validation errors from resolvers are not translated #9191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is missing some additional tests. Also, as this changes the behavior of the resolvers validation, I'd be more comfortable if this was released in a minor version. Would you mind targeting the next
branch instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, but we really need extensive tests showing that error message translation occurs once and only once in the 3 following cases:
- per input validation
- form-wide validation with
validate
- form-wide validation with
resolvers
These tests may be scattered around several components, I'm not sure. A central test for this matter would be welcome.
Added to |
return <>{translate(message, { _: message, ...args })}</>; | ||
return <>{translate(message, args)}</>; | ||
} | ||
|
||
return <>{translate(errorMessage as string, { _: errorMessage })}</>; | ||
return <>{translate(errorMessage as string)}</>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To allow warnings on missing translations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then it's a regression, as missing translation warnings will now get logged in the console while they were not in master
.
If this is only for the purpose of tests, you can enable warning on missing message in the parameters of the polyglotI18nProvider options (https://airbnb.io/polyglot.js/#options-overview). And if it doesn't work, you'll have to use a spy i18nProvider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that it silenced the missing translations warnings was actually a reggresion and was contradictory to the expected and documented behavior.
Fixes #9084