feat!: multi locale fallback #858

ST-DDT · 2022-04-20T18:45:21Z

This PR introduces multiple fallback locales using an array of formats.

Old

faker.locale = 'fr_CH';
faker.fallbackLocale = 'en';

New

faker.localeOrder = ['fr_CH', 'fr', 'en'];

I'm not sure about removing support for these to setters:

faker.locale = 'fr_CH';
faker.fallbackLocale = 'en';

and the following constructor parameters

const faker = new Faker({
    locale: 'fr_CH',
    fallbackLocale: 'en',
    locales: { ... }
});

The coverage gets fixed in #849.

Shinigami92 · 2022-04-20T18:54:35Z

src/faker.ts

  locales: UsedLocales;
-  locale: UsableLocale;
-  localeFallback: UsableLocale;
+  localeOrder: UsableLocale[];


I assume we need to make these readonly, so we can control when setting it

There needs to be a way to set them, it may also be useful to get them.
Especially so, if you consider it in combination with seed.

Shinigami92 · 2022-04-20T18:57:21Z

src/faker.ts

+    const parts = locale.split('_');
+    for (let i = parts.length; i > 0; i--) {
+      const subLocale = parts.slice(0, i).join('_');


question: could you explain me whats going on here? if you would like via voice chat 🙂

'en_AU_ocker' -> ['en', 'AU', 'ocker'] // L177

-> ['en', 'AU', 'ocker'] -> 'en_AU_ocker' // L179
-> ['en', 'AU'] -> 'en_AU' // L179
-> ['en'] -> 'en' // L179

Oh... I see, and it will not cause issues, if a in-between locale could not be found cause it's not defined?
Also maybe it should be documented that this exists / handle like this

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'])

Oh... I see, and it will not cause issues, if a in-between locale could not be found cause it's not defined?

faker/src/faker.ts

Line 180 in 75ffa2e

if (this.locales[subLocale] != null) {

I probably have to add some tests explicitly for that case.

Also maybe it should be documented that this exists / handle like this

I will try to do that.

Side note:

IMO my "old school" approach is way easier to read for me.

IMO my "old school" approach is way easier to read for me.

Both fine with me. I just posted this for fun :) Possibly the no split version could be understood better then slice, not sure...

codecov · 2022-04-20T19:07:47Z

Codecov Report

Merging #858 (75ffa2e) into main (b10f00c) will increase coverage by 0.02%.
The diff coverage is 32.71%.

@@            Coverage Diff             @@
##             main     #858      +/-   ##
==========================================
+ Coverage   99.35%   99.38%   +0.02%     
==========================================
  Files        1922     1922              
  Lines      183052   183034      -18     
  Branches      900      906       +6     
==========================================
+ Hits       181878   181900      +22     
+ Misses       1118     1078      -40     
  Partials       56       56

Impacted Files	Coverage Δ
src/locale/af_ZA.ts	`0.00% <0.00%> (ø)`
src/locale/ar.ts	`0.00% <0.00%> (ø)`
src/locale/az.ts	`0.00% <0.00%> (ø)`
src/locale/cz.ts	`0.00% <0.00%> (ø)`
src/locale/de.ts	`0.00% <0.00%> (ø)`
src/locale/de_AT.ts	`0.00% <0.00%> (ø)`
src/locale/de_CH.ts	`0.00% <0.00%> (ø)`
src/locale/el.ts	`0.00% <0.00%> (ø)`
src/locale/en.ts	`0.00% <0.00%> (ø)`
src/locale/en_AU.ts	`0.00% <0.00%> (ø)`
... and 47 more

pkuczynski · 2022-04-20T20:19:53Z

src/locale/ar.ts

@@ -8,8 +8,7 @@ import ar from '../locales/ar';
 import en from '../locales/en';

 const faker = new Faker({
-  locale: 'ar',
-  localeFallback: 'en',
+  localeOrder: ['ar', 'en'],


As en is presented in every locale definition, I think we should skip it and add it as default on usage. Would make the config much simpler.

#642
With that it will someday be possible to create your own Faker instance without 'en'

So instead of configuring it explicitly you want to move it to a default / implicit value?
I explicitly didn't choose that approach to allow users to omit the en locale to avoid certain fallback mechanism issues.

I don't think this should be possible tbh. en is the only full locale and mostly maintained. If you would like to create a new feature which needs locale, we can't block it until its implemented on every locale, right?

The only interface we should offer for locales should be:

const faker = new Faker({ locales: ['de_AT', 'fr'] })

which would mean following fallback line: de_AT > de > fr > en

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.

const faker = new Faker({ locales: ['de_AT', 'fr'] })

locales is already in use and reserved for the locales itself that can be passed

That would resolve to de_AT > de > fr not de_AT > de > fr > en, maybe you don't want en, if you want it, pass it

1 locales is already in use and reserved for the locales itself that can be passed

I am not sure if I understand? This is exactly what we want, initialise faker with locales, right?

I think it should always add en no matter what. Imagine user calls faker.animal.dog() and there is no data for it in neither of the locales he specified. What should faker do then:

return empty string?

throw an error?

or return something from en

I would vote for 3. Better to return English name, than nothing. But maybe we should throw?

1 locales is already in use and reserved for the locales itself that can be passed

I am not sure if I understand? This is exactly what we want, initialise faker with locales, right?

locales -> available data; localeOrder -> active data (+ their order)

I think it should always add en no matter what. Imagine user calls faker.animal.dog() and there is no data for it in neither of the locales he specified. What should faker do then:

We add it by default, to exactly ensure this behavior.
However, to avoid an issue like #547, we have to avoid hardcoded fallbacks.
Sometimes having an undefined/error is easier to detect, then a random english word.

See also #823

locales -> available data; localeOrder -> active data (+ their order)

This is not intuitive at all.

Sometimes having an undefined/error is easier to detect, then a random english word.

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.

Can you come up with sensible example when switching locales on single version of faker is useful?

Because its easier, otherwise we wouldn't do that in our tests.

locales -> available data; localeOrder -> active data (+ their order)

This is not intuitive at all.

For me it is, also I don't want to change the base logic too much with this PR.
(I intend to make this change backwards compatible, switching to ordered locale data instead of keys might make this impossible)

Please note, that this might be due to some thoughts I have regarding the locale loading.
I consider always adding all locale data to the faker instance, but load the actual contents only lazily.
See also #830
Since that's somewhat in the future, I would like to take one step at a time and fix some of our limitations, regarding the use of multiple locales.
In this PR, I would like to mostly stick to existing API. We can replace/improve the existing API in a separate PR to make reviews easier and focused on a single change.

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).

pkuczynski · 2022-04-20T20:20:22Z

src/locale/ar.ts

@@ -8,8 +8,7 @@ import ar from '../locales/ar';
 import en from '../locales/en';

 const faker = new Faker({
-  locale: 'ar',
-  localeFallback: 'en',
+  localeOrder: ['ar', 'en'],
  locales: {


If this would be an array and not object, you would not need the extra prop localeOrder right?

But, then you cannot easily switch between locales.
(strings are easier to replace, then full objects)

See my comment above...

How do you switch between locales?

faker.localeOrder = ['fr'];
faker.setLocale('fr_BE');

pkuczynski · 2022-04-20T20:37:58Z

src/faker.ts

+    const parts = locale.split('_');
+    for (let i = parts.length; i > 0; i--) {
+      const subLocale = parts.slice(0, i).join('_');


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'])

Shinigami92 · 2022-04-20T20:46:16Z

src/faker.ts

-  locale?: UsableLocale;
-  localeFallback?: UsableLocale;


We should somehow deprecate these with since v7 until v8 to make a smooth transition possible for users

src/locale/af_ZA.ts

ST-DDT · 2022-04-21T18:42:50Z

The coverage gets fixed in #849.

ST-DDT · 2022-04-24T11:22:07Z

@pkuczynski: @Shinigami92 also wants to merge localeOrder and locales into one.
However, he doesn't want to just drop locale and localeFallback without deprecating it first.
I'll check whether I can incorporate the backwards compatibility in this PR, otherwise this would be moved to v8 (as a hard break), so we don't have to many breaking changes in a single version.

ST-DDT · 2022-07-02T20:42:54Z

We will use a different approach.

feat: multi locale fallback

75ffa2e

ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent labels Apr 20, 2022

ST-DDT added this to the v7 - Next Major milestone Apr 20, 2022

ST-DDT self-assigned this Apr 20, 2022

ST-DDT requested a review from a team as a code owner April 20, 2022 18:45

Shinigami92 changed the title ~~feat: multi locale fallback~~ feat!: multi locale fallback Apr 20, 2022

Shinigami92 self-requested a review April 20, 2022 18:49

ST-DDT requested a review from a team April 20, 2022 18:57

Shinigami92 reviewed Apr 20, 2022

View reviewed changes

pkuczynski reviewed Apr 20, 2022

View reviewed changes

Shinigami92 requested changes Apr 20, 2022

View reviewed changes

ST-DDT commented Apr 20, 2022

View reviewed changes

src/locale/af_ZA.ts Show resolved Hide resolved

ST-DDT linked an issue Apr 20, 2022 that may be closed by this pull request

Incorrect behavior of name.firstName() function #547

Closed

ST-DDT mentioned this pull request Apr 20, 2022

Improve locale loading and global locale data #823

Closed

ST-DDT added s: on hold Blocked by something or frozen to avoid conflicts do NOT merge yet Do not merge this PR into the target branch yet and removed s: on hold Blocked by something or frozen to avoid conflicts labels Apr 21, 2022

ST-DDT mentioned this pull request Apr 30, 2022

QUESTION : cascading locality fallbacks #265

Closed

Shinigami92 modified the milestones: v7 - Current Major, v7.0 - Next Minor May 3, 2022

Shinigami92 added the needs rebase There is a merge conflict label May 19, 2022

Shinigami92 modified the milestones: v7.0 - Structural Breaking Changes, v7 - Current Major May 24, 2022

ST-DDT closed this Jul 2, 2022

ST-DDT deleted the feature/multi-locale-fallback branch July 2, 2022 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: multi locale fallback #858

feat!: multi locale fallback #858

ST-DDT commented Apr 20, 2022 •

edited

Loading

Shinigami92 Apr 20, 2022

ST-DDT Apr 20, 2022

Shinigami92 Apr 20, 2022

ST-DDT Apr 20, 2022

Shinigami92 Apr 20, 2022

pkuczynski Apr 20, 2022

ST-DDT Apr 20, 2022 •

edited

Loading

pkuczynski Apr 20, 2022

codecov bot commented Apr 20, 2022

pkuczynski Apr 20, 2022

Shinigami92 Apr 20, 2022 •

edited

Loading

ST-DDT Apr 20, 2022

pkuczynski Apr 20, 2022

Shinigami92 Apr 20, 2022

pkuczynski Apr 20, 2022

ST-DDT Apr 20, 2022

pkuczynski Apr 22, 2022 •

edited

Loading

ST-DDT Apr 22, 2022

pkuczynski Apr 20, 2022

ST-DDT Apr 20, 2022 •

edited

Loading

pkuczynski Apr 20, 2022

pkuczynski Apr 20, 2022

ST-DDT Apr 20, 2022

pkuczynski Apr 20, 2022

Shinigami92 Apr 20, 2022

ST-DDT commented Apr 21, 2022

ST-DDT commented Apr 24, 2022

ST-DDT commented Jul 2, 2022

feat!: multi locale fallback #858

feat!: multi locale fallback #858

Conversation

ST-DDT commented Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 20, 2022

Codecov Report

Choose a reason for hiding this comment

Shinigami92 Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkuczynski Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT commented Apr 21, 2022

ST-DDT commented Apr 24, 2022

ST-DDT commented Jul 2, 2022

ST-DDT commented Apr 20, 2022 •

edited

Loading

ST-DDT Apr 20, 2022 •

edited

Loading

Shinigami92 Apr 20, 2022 •

edited

Loading

pkuczynski Apr 22, 2022 •

edited

Loading

ST-DDT Apr 20, 2022 •

edited

Loading