-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
Custom faker instance with en locale cannot generate system variables #3405
Comments
Realized that I need to extend |
I wonder if there is a way we could emit a warning if someone makes a custom faker without extending base? As I feel this is likely to be not intentional most of the time. |
This comment has been minimized.
This comment has been minimized.
Maybe we should add a hint to the constructor like /**
* ...
* Please make sure that all required locales including parent locales are present e.g. `[de_AT, de, en, base]`
* ...
*/ And extend the error message a bit: throw new FakerError(
`The locale data for '${path.join('.')}' are missing in this locale.
If this is a custom Faker instance, please make sure all required locales are used e.g. '[de_AT, de, en, base]'.
Please contribute missing data to the project or use a locale/Faker instance that has these data.
For more information see https://fakerjs.dev/guide/localization.html`
); ? |
@ST-DDT That's a great idea. I have to admit though the error I encountered is a user error coming from myself because I totally missed the part of extending off |
Still we can always improve error messages etc to reduce human error 😀 By the way why did you feel you had to make a custom faker instance - if you just need a custom seed you don't need one? Or was this a simplified example just to explain the bug and you actually need a custom Faker instance? |
@matthewmayer Your second guess is correct: we needed to create a custom faker instance with additional fake data, and the example I shown is simply a contrived one but minimal enough to reproduce the "issue" (which turns out to be a user error, hehe) |
@ST-DDT what do you think about adding a check like this to the constructor? if (locale && (typeof locale === 'string' || locale.length === 1)) {
// eslint-disable-next-line no-undef
console.warn(
'[@faker-js/faker] only a single locale was provided. Was this intentional? In general, pass an array of locales ending with the `base` locale, for example [customLocale, de_CH, de, en, base]. See https://fakerjs.dev/guide/localization.html#custom-locales-and-fallbacks'
);
} |
If you really want to save some bundle size, you might want to create a locale instance that only contains the data you need prepackaged. |
I created PR to improve the error message: #3406 |
Pre-Checks
Describe the bug
I am using a custom faker instance in my app, i.e.:
However, I get an error in the console with this message:
However,
en
is the locale with the widest / largest dataset and I would expect it to havesystem.mine_type
implemented. Using the defaultfaker
instance does not give me any issues, but I assume that the default faker is also using the "en" locale.Minimal reproduction code
See example here: https://stackblitz.com/edit/vitejs-vite-iwf3husr
To reproduce this code independently, you can do this:
Additional Context
No response
Environment Info
System: OS: Linux 5.0 undefined CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Memory: 0 Bytes / 0 Bytes Shell: 1.0 - /bin/jsh Binaries: Node: 18.20.3 - /usr/local/bin/node Yarn: 1.22.19 - /usr/local/bin/yarn npm: 10.2.3 - /usr/local/bin/npm pnpm: 8.15.6 - /usr/local/bin/pnpm npmPackages: @faker-js/faker: ^9.5.0 => 9.5.0
Which module system do you use?
Used Package Manager
npm
The text was updated successfully, but these errors were encountered: