feat!: multi locale fallback #858
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,14 +38,12 @@ export type UsedLocales = Partial<Record<UsableLocale, LocaleDefinition>>; | |
|
|
||
| export interface FakerOptions { | ||
| locales: UsedLocales; | ||
| localeOrder?: UsableLocale[]; | ||
| } | ||
|
|
||
| export class Faker { | ||
| locales: UsedLocales; | ||
| localeOrder: UsableLocale[]; | ||
|
Comment on lines
45
to
+46
There was a problem hiding this comment. I assume we need to make these readonly, so we can control when setting it There was a problem hiding this comment. There needs to be a way to set them, it may also be useful to get them. |
||
|
|
||
| readonly definitions: LocaleDefinition = this.initDefinitions(); | ||
|
|
||
|
|
@@ -95,25 +93,35 @@ export class Faker { | |
| } | ||
|
|
||
| this.locales = opts.locales; | ||
| this.localeOrder = opts.localeOrder ?? ['en']; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a Proxy based LocaleDefinition that virtually merges the locales. | ||
| */ | ||
| private initDefinitions(): LocaleDefinition { | ||
| // Returns the first resolved locale data that aren't undefined | ||
| const findFirst = <T>( | ||
| resolver: (data: LocaleDefinition) => T | undefined | ||
| ): T | undefined => { | ||
| for (const locale of this.localeOrder) { | ||
| const baseData = resolver(this.locales[locale]); | ||
| if (baseData != null) { | ||
| return baseData; | ||
| } | ||
| } | ||
| return undefined; | ||
| }; | ||
|
|
||
| // Returns the first LocaleDefinition[key] in any locale | ||
| const resolveBaseData = (key: keyof LocaleDefinition): unknown => | ||
| findFirst((data) => data[key]); | ||
|
|
||
| // Returns the first LocaleDefinition[module][entry] in any locale | ||
| const resolveModuleData = ( | ||
| module: keyof LocaleDefinition, | ||
| entry: string | ||
| ): unknown => findFirst((data) => data[module]?.[entry]); | ||
|
|
||
| // Returns a proxy that can return the entries for a module (if it exists) | ||
| const moduleLoader = ( | ||
|
|
@@ -159,11 +167,25 @@ export class Faker { | |
| } | ||
|
|
||
| /** | ||
| * Set Faker's locale using the default fallback strategy. | ||
| * | ||
| * @param locale The locale to set (e.g. `en` or `en_AU`, `en_AU_ocker`). | ||
| * @param fallbacks The fixed fallbacks to use. Defaults to `['en']`. | ||
| */ | ||
| setLocale(locale: UsableLocale, fallbacks: UsableLocale[] = ['en']): void { | ||
| const localeOrder = []; | ||
| const parts = locale.split('_'); | ||
| for (let i = parts.length; i > 0; i--) { | ||
| const subLocale = parts.slice(0, i).join('_'); | ||
|
Comment on lines
+177
to
+179
There was a problem hiding this comment. question: could you explain me whats going on here? if you would like via voice chat 🙂 There was a problem hiding this comment.
-> There was a problem hiding this comment. Oh... I see, and it will not cause issues, if a in-between locale could not be found cause it's not defined? There was a problem hiding this comment. Side note: ES6 way: 'en_AU_ocker'.split('_').reduce((result, _, i, parts) => {
return result.concat(parts.slice(0, parts.length - i).join('_'))
}, [])No split: 'en_AU_ocker'.split('_').reduce((result, _, i, parts) => {
parts.pop()
return result.concat(parts.join('_'))
}, ['en_AU_ocker'])There was a problem hiding this comment.
Line 180 in 75ffa2e I probably have to add some tests explicitly for that case.
I will try to do that.
IMO my "old school" approach is way easier to read for me. There was a problem hiding this comment.
Both fine with me. I just posted this for fun :) Possibly the no split version could be understood better then |
||
| if (this.locales[subLocale] != null) { | ||
| localeOrder.push(subLocale); | ||
| } | ||
| } | ||
| for (const fallback of fallbacks) { | ||
| if (!localeOrder.includes(fallback) && this.locales[fallback] != null) { | ||
| localeOrder.push(fallback); | ||
| } | ||
| } | ||
| this.localeOrder = localeOrder; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,8 +8,7 @@ import ar from '../locales/ar'; | |
| import en from '../locales/en'; | ||
|
|
||
| const faker = new Faker({ | ||
| localeOrder: ['ar', 'en'], | ||
|
There was a problem hiding this comment. As There was a problem hiding this comment. #642 There was a problem hiding this comment. So instead of configuring it explicitly you want to move it to a default / implicit value? There was a problem hiding this comment. I don't think this should be possible tbh. The only interface we should offer for locales should be: const faker = new Faker({ locales: ['de_AT', 'fr'] })which would mean following fallback line: I can't imagine someone would prefer not working feature over getting a string in English when its not supported in a locale selected by him. There was a problem hiding this comment.
There was a problem hiding this comment.
I am not sure if I understand? This is exactly what we want, initialise faker with locales, right? I think it should always add
I would vote for 3. Better to return English name, than nothing. But maybe we should throw? There was a problem hiding this comment.
We add it by default, to exactly ensure this behavior. See also #823 There was a problem hiding this comment.
This is not intuitive at all.
That's true, but since faker is meant to provide fake data for tests and maybe some demoes, I would be surprised if someone would prefer error to be thrown instead of seeing the value in English... If the error is thrown he would be a bit blocked vs when English is returned and he might be annoyed a bit and report the issue to us, which we would encourage him to fix by providing localisation... I spent some time thinking about it and I wonder why would someone want to switch locales if he can simply have to separate instances of faker for every locale he needs? Can you come up with sensible example when switching locales on single version of faker is useful? I would rather see something like this on the user side: import { Faker, pl, de_AT } from '@faker-js/faker'
// or for someone who does not have tree-shaking bundler
import { Faker } from '@faker-js/faker'
import { pl } from '@faker-js/faker/locale/pl'
import { de_DE } from '@faker-js/faker/locale/de_DE'
const faker = {
pl: new Faker({ locales: [pl] })
de: new Faker({ locales: [de_AT] })
}
console.log(faker.pl.animal.bird())
console.log(faker.de.animal.bird())That's clean. There was a problem hiding this comment.
Because its easier, otherwise we wouldn't do that in our tests.
For me it is, also I don't want to change the base logic too much with this PR. Please note, that this might be due to some thoughts I have regarding the locale loading. I'm not really happy of how big of a change this fix actually is and think about ways to simplify it (e.g. don't replace fallbackLocale with localeOrder and just make locale a single key or array). |
||
| locales: { | ||
|
There was a problem hiding this comment. If this would be an array and not object, you would not need the extra prop There was a problem hiding this comment. But, then you cannot easily switch between locales. There was a problem hiding this comment. How do you switch between locales? There was a problem hiding this comment. faker.localeOrder = ['fr']; |
||
| ar, | ||
| en, | ||
|
|
||
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.
We should somehow deprecate these with since v7 until v8 to make a smooth transition possible for users