Closed rosskevin closed 6 years ago
After a cursory review, it would seem to need the following to make this happen:
mixed.validate
to accept an optional user-provided translate(key)
functionkey
instead of a resolved string message
for errorscreateValidation
to resolve the message using the given translate or fallback to the hardcoded localeForm#handleValidate
would need to be updated to pass through the translate
function provided by the user.Does this look correct? Are you open to a PR on this?
hey there, sorry I don't have a moment right now to really comment, but there is two things here. "yes, extending/altering the existing language is too hard", and "no we don't necessarily need to add a translate
sort of functionality". When I get a moment I'll post more about how we handle i18n with yup/react-formal
Just so I'm clear, I'm not saying implement translate within yup, but allow a translate(key)
function to be provided by the user.
I'm open to anything, we use i18next
for our app, interest to hear your approach.
So yup
doesn't care what shape your messages are. you can just provide a key if you want.
by default react-formal expects the message to a string, but that's not required. We use react-intl
which has message objects that are like { id: 'someKey', defaultMessage: 'foo' }
. We override all the provided messages with these sort of objects as well as for any other messages.
react-formal Message
components have an extract
prop, which maps the yup error json to a string to display, and defaults to: error => error.message || error,
. You can change that to handle translateing the key to a real message like:
export default class FormMessage extends React.Component {
static contextTypes = {
intl: intlShape.isRequired,
};
extract = (error) => {
const key = error.message || error
return this.context.intl.formatMessage(key);
};
render() {
return (
<Form.Message
{...this.props}
extract={this.extract}
/>
);
}
}
what is hard about yup right now tho, is that in order to override the default locales you need to require and mutate the yup one, which is gross. I'd happily accept a PR to give that a proper API.
That's what I'm referring to - the yup defaults. I'd prefer my schema code to be simply yup.string().required()
. Then provide my own message via a user-provided translate instead of falling back to the hardcoded bundle.
In addition, I'd like it to be pluggable using the optionally user-provided translate function so the defaults can be multi-lingual.
So it seems like the plan of action I outlined is still needed to use a pluggable translate + fallback.
Actually, thinking the key
or i18n
property needs to be a part of the validation error, that way the user can still do resolution as they please, similar to what you outlined with the FormMessage
above.
So it seems like the plan of action I outlined is still needed to use a pluggable translate + fallback.
I think we might be on different pages on that part. Yup errors, are not intended to be user facing, they can serve double duty if you have a simple of enough setup, but generally all the i18n should be done in UI bits, such as the form library. The way forward is probably just to allow overriding the default language. Then you can set them to some key, or code, or whatever you want, that can be turned into a localized user message, by your app.
i was trying to say above that this is already possible albeit a bit convoluted.
const locale = require('yup/lib/locale')
locale.string.required = 'isRequiredKey';
but generally all the i18n should be done in UI bits, such as the form library
Agreed, though I still think that providing a key
and message
in the yup error is very easy and inline with yup's concerns.
The last problem is that react-formal addon widgets need access to the user's chosen translate function.
For example, react-formal-material-ui
is a UI library of widgets agnostic of the user's selection of translation engines, and it is responsible for displaying an error message (such as this spec example). This library is taking the information right from the meta
and displaying it.
Using approach with modifying the locale.js to provide keys, now we still need to provide the users' chosen translate
function. Given that this is a UI issue, I can move this to react-formal, but it seems like we at least need Form translate={myTranslateFn}
(or something of the like) to propagate this function perhaps via meta
so that my custom widget can access the user's chosen translate function and use it to determine the message.
Does that make sense?
[...] providing a key and message in the yup error is very easy and inline with yup's concerns.
I'm quite sure why you'd want both a key and message on the yup error, wouldn't you want one or the other? Moreso though, I can't honestly think of a way to do that without making the api really messy. you'd need to provide a key and message for any test that takes a msg
arg not just the defaults. Practically you can already do this, by using an object as the message instead of a string, eg:
let schema = string().required({ key: 'required', message: 'This is required' })
Open to thoughts on that tho. (again we need an api for changing the defaults easier)
[...] least need Form translate={myTranslateFn}
technically you don't need one, the FormMessage example above is a good example of that...that being said it probably is nice to have a some global extract
definition that anything that receives errors can access so you don't need to provide it to each and every input that needs to display errors
I will also add, i'm very wary of having too-specific i18n hooks in react-formal just because each i18n library does it differently. react-intl for instance passes the formatMessage
function down via context, so it isn't accessible outside the component hierarchy, which precludes using the react-formal config stuff for this
I'm quite sure why you'd want both a key and message on the yup error, wouldn't you want one or the other?
You wanted to provide the message
. I just want the key. I'm compromising. I think the core yup code should not be using full text messages, only keys.
Practically you can already do this
I want the brevity of string().required()
instead of the longer syntax spread through the app. We have a lot of forms
technically you don't need one
Yes, we do. The user is using react-formal-material-ui
as a library and it controls the UI display of the field and the error message (see the link above), and it too must be agnostic of chosen translation engines.
I will also add, i'm very wary of having too-specific i18n hooks in react-formal
I'm not looking for specifics, just allow translate to be found. Whatever translate
is. I agree that we don't want to get into the weeds. An extensible plugin on Form
to execute a translation seems feasible.
I think the core yup code should not be using full text messages, only keys.
I see where you are coming from but I don't think that makes sense for all use cases, only the ones using yup for ui form validation. Using yup for server param validation or api parsing, would be pretty bad if the errors only said "error: required"
I still think we are missing each other about what is needed and where to achieve all this. We both agree that this should be easier and not tightly coupled to any i18n choice
I want the brevity of string().required() instead of the longer syntax spread through the app.
Agreed. however this problem isn't related to keys, or message types tho. The issue here (as I tried to note above) is a missing api for changing the defaults to whatever you want, whether it's a message or a key, or both. I think that's really easy to add here.
What is possible now (I know because we do it) is swapping all the default messages out with codes and having react formal properly translate them to messages.
Yes, we do. The user is using react-formal-material-ui as a library and it controls the UI display of the field and the error message (see the link above),
My point was only that you can solve this by having an extract
prop on the base Input component and allow folks to set the default, like we do with Message
component In react-formal, which also displays messages and doesn't have access to some global translate function.
despite that I still agree that maybe some sort of form level extract
prop is a good idea, however I'd be concerned that it wouldn't be sufficient, depending on how the i18n library of choice works. What is available to the consumer at the Form component point in the hierarchy might not be right for translating at the point where the Field is actually rendered
Agreed on swapping locales should be easier in yup.
Also agreed on using an extract
concept overall. I think if the user can provide to the Form
(flow notation):
declare function extract(key: string): string
and that is propagated via meta
, then a pluggable way to translate has been provided. I certainly agree we should not be bound to an i18n solution. We could allow the user to pass the extract
as a prop on the Field
if they required some specifics to be performed on a field-basis which would override the Form extract
.
If something pluggable isn't provided, I'll provide a Form HOC in the react-formal-material-ui
addon library that provides the extract
function via context. That's certainly workable, but I'd rather not be deviating from the core, and simply decorate it with widgets. :)
If something pluggable isn't provided, I'll provide a Form HOC
ya I also think this is a suboptimal solution. lets try to avoid that.
I think for consistency and flexibility its better if the extract
prop matched the signature of the one in Message. It also saves a breaking change.
In terms of what should be done here in yup I think we need two things, the first being more important.
setLanguage()
function that, will override the defaults. maybe something like:
setLanguage({
required: 'foo', // overrides any type that takes required
string: {
max: 'bar', //only change the max message for strings
}
})
@jquense
I see where you are coming from but I don't think that makes sense for all use cases, only the ones using yup for ui form validation. Using yup for server param validation or api parsing, would be pretty bad if the errors only said "error: required"
Sorry to drop my opinion on this: I don't understand this point and I've seen it in a few issues on Joi as well. Even when Yup/Joi are used for server param validation, the consumers of the error messages are still humans, not machines. If you say ""error: required" would be pretty bad for an api error response, I can argue that not being able to translate it is equally bad (I do think that error: required is not pretty bad at all for a developer-targeted error message)
So in my opinion, either you decide to just use keys for messages (which makes more sense to me), or you support human friendly messages in both use cases. As you can map the keys to your custom error messages client-side, so you can server-side.
the consumers of the error messages are still humans, not machines
yes except that in the one case the humans are developers not users of your app. Translating API messages is not something that any api that I know of does. The plain fact is that the errors are never supposed to be surfaced to a user.
Even so, if you want to do that you are definitely free too. Yup errors are intentionally and carefully designed to not care what the messages are. you can strings, numbers, objects, anything. All that is missing (as noted and acknowledged) in this library is a way to easily replace the defaults
Ok, I understand.
"The plain fact is that the errors are never supposed to be surfaced to a user."
See, this is what I found a bit confusing, because you wrote above:
"Using yup for server param validation or api parsing, would be pretty bad if the errors only said "error: required"" If errors are never supposed to surfaced to a user, why would you consider "error: required" as pretty bad? Seems good enough to me. I think this is what confuses most people on Joi too. Having "good enough" messages that are not quite flexible is more confusing (not necessarily "wrong") than a clear: "No, we don't deal with human-friendly error messages, it's up to you to map these error keys to whatever you like".
Anyway, I do get your point that "user" is different than "developer", and thanks for the work on this library.
"error: required" is bad because it doesn't help the developer understand what happened. As you say everyone is still human. The idea is exactly to provide human friendly errors by default. but they are friendly to the developer, not the user. It's the same reason Exceptions have messages. Again, that they are, as a default, a string message, doesn't make them any harder to map to whatever you like.
@jquense could something like message: string | Function
make a sense to provide backward compatible way to handle translations? So for example I could change a locale for yup
like this:
import { setLocale } from 'yup/lib/customLocale';
setLocale({
string: {
min: (params: { min: number }): string => localizeToCzechWithCorrectPluralFormBasedOnMin('string.min', params.min),
trim: 'Toto pole nesmí obsahovat mezery na začátku a na konci.',
},
});
We are using yup
with formik
and the problem is that error.params
are lost between those so we don't have enough information to render proper message (in correct grammar for plurals).
So basically, what if locale could be defined as a string or a function accepting params for validators and returning already localized string?
@michalkvasnicak I believe you can already do this? Have you tried?
@jquense I will try, I was just asking because I'm under the time pressure so just wanted to know if this solution is possible to be merged if I do a PR.
yes i'm fine supporting functions in the locales, it was always intended that that work so if it doesn't happy to take a PR
@jquense I'm sorry, I didn't know that there already is support for functions in locale (it isn't in documentation, or at least I haven't seen that). I found this in code. I'll try it in our project. Sorry for unnecessary comments. Thank you for this great project :)
@michalkvasnicak Did you manage to make it work?
I tried the following
Yup.setLocale({
string: {
matches: ({ path, type, value, originalValue }) => {
console.info("matches", path);
return i18n.t(`signUp.${path}.matches`);
},
required: ({ path, type, value, originalValue }) => {
console.info("required", path);
return i18n.t(`signUp.${path}.required`);
},
min: params => i18n.t("string.min", params.min)
}
});
I can see messages being logged to console for matches
but not for required
...
It doesn't seem to be possible to set the required
message using setLocale
. Either for a particular type, or across all types at once.
Based on the discussion above, I had expected both of these to work:
setLocale({
string: {
required: 'Required',
},
});
or
setLocale({
required: 'Required',
});
I did with the steps above:
/**
* Builds the Yup Locale
*/
export function buildYupLocale(_: unknown, t: TFunction): void {
appYup.setLocale({
mixed: {
required: t("mixed.required.key"),
},
number: {
min: t("number.min.key"),
},
string: {
email: t("string.email.key"),
max: t("string.max.key"),
},
});
}
i18n.use(initReactI18next).init(
{
resources: COMMONS_TRANSLATIONS,
fallbackLng: config.APP_MAIN_LANGUAGE,
keySeparator: ".",
interpolation: {
escapeValue: false,
},
react: {
wait: true,
},
},
buildYupLocale // <-------- the buildYupLocale function is invoked with the second argument being the TFunction
);
const changeLanguage = useCallback(
(newLng: string): void => {
if (i18n.isInitialized && i18n.language !== newLng) {
i18n.changeLanguage(newLng);
setQueryParam(LANG_KEY, newLng, true);
CookieService.getInstance().set(LANG_KEY, newLng, { expires: COOKIE_EXPIRE_DAYS });
setLanguage(newLng);
// Rebuilds the YupLocale so the error messages change the language
buildYupLocale(null, t);
}
},
[t, i18n]
);
buildYupLocale
@rafaelbrier
how do you get the appYup to work? Changing the language on a yup locally does work, but where do you set appYup and import it from?
If you use formik + yup and i18next for localization, then you can do this thing: suppose that your yup validation schema contains value "someField".
so, you can use this syntax for localization:
t(formik.errors.someField as string)
where formik.errors.someField is current error for your field which you add to yup validation scheme:
const validationSchema = yup.object({
someField: yup
.string()
.required('someField is required'),
});
So, in your i18n localization file you can add translation for "someField is required" message and for other formik.errors.someField messages
I see the locale messages in https://github.com/jquense/yup/blob/master/src/locale.js
But there doesn't appear to be any way to alter these messages, nor a way to plug-in other i18n solutions by referring simply to keys and allowing it to resolve independently.
Is this something others are needing too?