-
-
Notifications
You must be signed in to change notification settings - Fork 222
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
feat: Add i18n, global config and improve error messages #397
Conversation
First off there is a bunch of stuff I really like:
The global message store + side-effect modules for loading work quite well & seem like the right design choice: Questions:What would be the impact of switching to references for message-keys?import * as v from "valibot"
v.setLocalMessage(
v.minLength, //instead of "min_length"
(issue) => `Invalid length: Expected ${issue.expected} but received ${issue.received}`,
'en'
); Potential Pros
Cons
Are "global" and "local" messages the best names?If I understand correctly "global" messages are generic messages that can be used when no more specific message is avaliable. "local" messages are very specific messages that apply to only one validator. I find this terminology a bit confusing. "local" seems really close to "locale", which could lead to misunderstandings. Perhaps Default message values could be compressedLooking at the default messages, most seem to follow this pattern: (issue) =>`Ungültiger 48 bit MAC: ${issue.expected} erwartet aber ${issue.received} erhalten` "48 bit MAC" is the expected format for this validator. Most messages differ only in that part of the string. The rest is the same, so re-declaring it is wasteful. With a bit of currying this could be compressed: // i18n/de/genericMessage
const createInvalidFormatMsg = (expectedFormat: string) => (issue) =>`Ungültiger ${expectedFormat}: ${issue.expected} erwartet aber ${issue.received} erhalten`
// i18n/de/minValue
import { setLocalMessage } from 'valibot';
setLocalMessage(
'min_value',
createInvalidFormatMsg("Wert"),
'de'
); This would minify way better. Since this would be a non-breaking change it could always be done later, so no rush. |
Thank you @LorisSigrist for your detailed feedback! I really appreciate it.
Great idea! I will investigate the difference and share the results with you in the comments of this PR.
I assume that the memory usage is higher this way if only a few translations are imported. However, I have not tested this and the difference is probably negligible.
I am not 100% happy with the names. I tried to map both with
Since one is just called Important: If we change the message keys to references, this procedure no longer works. That's why it may be better to stick with the current approach and separate them using two different functions.
Technically this works. However, I would first want to check whether it actually reduces the bundle size after gzip compression. |
Ok this may sound controversial but I think i18n should not be the job of a validation library. Don't get me wrong, if you want the burden of keeping the translations up to date and the extra maintenance cost I will use What do you think about this? |
Thank you @paoloricciuti for your feedback! I agree that integrated translations do not have to be part of Valibot. I included it in this draft because several users requested it. I would be happy to get more feedback on this from other users.
What do you like about the API? What do you think about the names |
It might make sense to move the internal translations (if we decide to provide them) into a separate package, e.g. |
I refer to this API import * as v from 'valibot';
import * as m from "./paraglide/messages.js"
const Schema = v.string([
v.email((issue) => m.invalidEmail(issue)), // Can also be written as `v.email(m.invalidEmail)`
]); Regarding |
Thanks for your feedback! Based on it I changed a few things. I renamed import * as v from 'valibot';
z.setSpecificMessage(
v.minLength,
(issue) => `Invalid length: Expected ${issue.expected} but received ${issue.received}`,
'en'
); I also added import * as v from 'valibot';
v.setSchemaMessage(
(issue) => `Invalid type: Expected ${issue.expected} but received ${issue.received}`,
'en'
); The last change was that I moved the official translations into a separate package called import { Language } from './types';
// prettier-ignore
const language: Language = {
code: 'en',
schema: (issue) => `Invalid type: Expected ${issue.expected} but received ${issue.received}`,
specific: {
bic: (issue) => `Invalid BIC: Received ${issue.received}`,
bytes: (issue) => `Invalid bytes: Expected ${issue.expected} but received ${issue.received}`,
creditCard: (issue) => `Invalid credit card: Received ${issue.received}`,
// ...
}
};
export default language; import '@valibot/i18n'; // Import any translation
import '@valibot/i18n/de'; // Import specific language
import '@valibot/i18n/de/email'; // Import specific language message |
I plan to merge my changes in the next few days and release a new version next week. |
I like how Valibot follows modular principles and separates i18n into its own package, along with the code generation that can make things simpler. There are a couple of things I'd like to point out. 1. Default English LanguageDespite having en.ts file in i18n package, I noticed the following comment in the file: // Create languages array
// Note: The language file `en` does not need to be added as the default
// messages of Valibot are already in English Am I correct in understanding that Valibot will come with all English translations right away? The thing is, not all applications use English, so including it by default could increase the bundle size unnecessarily. Why not leave the language choice to the user? Moreover, this will help reduce Valibot bundle size and maintain the modular design. 2. Single LanguageCurrently, we can only specify one language: const output = v.parse(LoginSchema, input, {
lang: 'de',
}); However, it seems to me that as Valibot evolves, the number of supported languages will only increase, and the community might not always keep up with translating all languages immediately after a new feature is released. This means there might be situations where some languages lack complete translations, especially for languages with fewer speakers. To handle such situations and make the user experience as convenient as possible, why not allow specifying multiple languages in a prioritized order? const output = v.parse(LoginSchema, input, {
lang: ['de', 'fr', 'en'],
}); Now, if a translation is not found for the This could be beneficial for many countries. 3. Expected/Received in Every TranslationCurrently, each translation includes information about the received and expected values. I think this information can often be excessive and make it challenging to use. Why not make the display of this information optional? I believe it won't significantly impact the bundle size. (issue) => `Invalid email${issue.inDetail ? `: Expected ${issue.expected} but received ${issue.received}`: ``}` Moreover, with utility functions, you can further reduce the amount of code: function util(message: string, issue: SchemaIssue) {
return `${message}${issue.inDetail ? `: Expected ${issue.expected} but received ${issue.received}` : ``}`;
} 4. DocumentationSimilarly to how a bundle is generated for the i18n package, we can generate such a table for the documentation page to help the community maintain complete translations:
This way, it becomes easier for contributors to see which languages might need attention in terms of translations. This can also inspire users to contribute translations in their language 🙂 |
Thank you very much for your feedback and contribution @gmaxlev!
Yes and no. Valibot's default messages are in English, but they are not included as a translation pack. Instead, they are backed into the source code. This allows us to implement the default messages with better performance and smaller bundle size. The default messages add about 60 kB initially to the core bundle, while an entire language pack adds about 700 bytes (you can also reduce the bytes of a language pack by importing only the messages you need). We could outsource all the messages to If we outsource all the messages and part of the implementation to In the end, it is always a trade-off between bundle size, performance, and developer experience. For example, in some places in the library, we deliberately do not use the spread operator because it is a performance bottleneck. This decision increases the initial bundle size by about 100 to 200 bytes. I have not made a final decision on this yet, so I am looking forward to your feedback.
With the current implementation, the default messages in English are always the fallback if a translation is missing. But I understand your point and see the benefit of this implementation. Again, it's a tradeoff between bundle size, performance, and developer experience. If we decide to outsource a large part of the i18n implementation to
I like the idea of this being configurable. Especially in combination with your util function.
This is also a really good idea! I plan to implement it. |
The initial additional bundle size due to the 3 new features (global config, detailed error messages, and i18n functionality) is approximately 350 kB. However, by improving the existing code, the additional bundle size of this PR is expected to be about 310 kB. The global config is about 50 kB, the detailed error messages are about 200 kB, and the i18n functionality is about 120 kB. Due to compression, the total bundle size is slightly less than that. |
I checked the implementation and the bundle size. This feature would cost an additional 40 kB. Because of gzip compression, it makes no difference if we implement it with or without a util function. I am not sure if we should add this feature now or wait until multiple users request it and then decide. We could also research if users of Zod have ever requested this feature. |
I also want to mention that it is really easy to overwrite the error message and remove the details using import * as v from 'valibot';
v.setGlobalMessage((issue) =>
issue.message.slice(0, issue.message.indexOf(':'))
); |
I expect to merge this PR tomorrow. However, I am still happy to receive feedback. All feedback is heard and influences the long-term development of the library. Major changes are still possible until v1. |
Better error messages will be very welcome! How did you calculate the 150 bytes increase? |
I calculated it by bundling a schema with and without the respective code. The byte amount is the minified and gzipped version. |
v0.28.0 with i18n is available: https://valibot.dev/guides/internationalization/ |
Overview
This PR is a first draft of our i18n feature discussed in #36. It also improves the built-in error messages and adds global configuration settings.
Global config
When calling
parse
orsafeParse
to validate unknown data with a schema, configurations can be passed as the third argument.These configurations can now also be defined globally with
setConfig
. The configurations passed directly toparse
andsafeParse
have a higher priority and are merged with the global configurations.Error messages
Previously, the error messages were extremely minimalistic. For example, if the data type was incorrect,
Invalid type
was always returned. With the increasing popularity of Valibot, I see it as a benefit to DX and UX if the default messages are more informative.Inspired by Zod, I added the
expected
andreceived
properties to each issue. These are language-neutral strings that follow a defined format. They are language-neutral because they use symbols like "!" and "&" instead of words like "not" or "and".The special thing about our implementation is that it only requires about 150 additional bytes after minification and compression.
i18n feature
Issue #36 has resulted in the following three requirements:
More languages
When implementing i18n for Valibot, it was important to keep the bundle size to a minimum. For example, if only
string
andemail
are needed, it should be possible that the final bundle contains only these translations. This is done by providing each translation as a separate submodule and importing them as needed, as proposed by @gmaxlev.Globally overridable
The submodules write the translations to a global storage. The API used for this is accessible and can also be used by developers to globally customize error messages and add own translations.
setLocalMessage
sets the message of a local function likeminLength
and has a higher priority thansetGlobalMessage
which can be used to override the default messages globally.Dynamically customizable
Instead of a string, you can also pass a function to
setLocalMessage
, as well as to schema and validation functions, which has access to all issue information such asexpected
and configurations such aslang
to dynamically generate error messages. This enables integration with i18n libraries such as Paraglide JS.The language setting can be defined globally with
setConfig
or locally when callingparse
andsafeParse
as described above.Feedback
I think it is possible that Valibot will be downloaded more than 1 million times a month by the end of the year, making it one of the most important schema libraries. I would like to lay the foundation for this together with you, the early adopters. I look forward to receiving feedback on naming and implementation of this PR draft.
If you are interested, you can clone the repo with the
feat-i18n
branch and then runpnpm i && cd library && pnpm build
to build the library. Then you can either use the playground inlibrary/playground.ts
withpnpm playground
or pack the library withpnpm pack
to install the bundle into another project withpnpm i ./valibot-0.26.0.tgz
.