Using specific locales and calling prefix or jobTitle throws error: Calling faker.helpers.arrayElement() without arguments is no longer supported.

[labithiotis]

Pre-Checks

• Follow our Code of Conduct.
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Faker issue and not related to a combination with another package.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.
• I am willing to provide a PR.

Describe the bug

When using a specific locale and method person.prefix or person.jobTitle it throws an error:

Calling `faker.helpers.arrayElement()` without arguments is no longer supported.

I suspect the issue is the locale data has empty/missing array which is passed into the helper method by prefix or jobTitle.

Expected behaviour

I would expect this not to crash/throw an error.

Effected locales

az
id_ID
ru
zh_CN
zh_TW

Solutions

1. All locales have an empty array set which is passed into arrayElement method
2. Allow arrayElement to be called with undefined

Minimal reproduction code

https://codesandbox.io/s/sad-maxwell-m44x5y?file=/src/index.mjs

Additional Context

There may be other locales and methods that are also effected by this too.

Environment Info

System:
    OS: macOS 14.0
    CPU: (8) arm64 Apple M1
    Memory: 76.72 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.8.1 - ~/.nvm/versions/node/v20.8.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v20.8.1/bin/yarn
    npm: 10.1.0 - ~/.nvm/versions/node/v20.8.1/bin/npm
    pnpm: 8.9.2 - ~/.nvm/versions/node/v20.8.1/bin/pnpm
  Browsers:
    Brave Browser: 118.1.59.122
    Safari: 17.0
  npmPackages:
    @faker-js/faker: ^8.2.0 => 8.2.0

Which module system do you use?

• CJS
• ESM

Used Package Manager

pnpm

[ST-DDT]

Please note that the methods throwing an error when no data are provided is expected behavior although the exact error message is not.
This will be fixed in https://github.com/faker-js/faker/pull/2435/files#diff-b87df353cd0dd3fc93478bf0d3245306b868008a35a98b4c4406260ca2c5c40eR61

See also:

• Change file structure for gendered lists (names, prefixes etc) #1677

Could you please checkout the PR and check whether it matches your expectations?

[xDivisionByZerox]

Can confirm this bug due to the mentioned locale data being null. The problem person.prefix() is that we access the raw definition data which bypasses the usual error handling in those cases.

faker/src/modules/person/index.ts

Line 310 in 74f6bbd

this.faker.rawDefinitions.person ?? {};

[labithiotis]

Checking out that PR it still throws an error if we do the following:

import { fakerAZ } from '@faker-js/faker';
console.log(fakerAZ.person.prefix());

The error is better and more informative though:

[Error]: The locale data for 'person.{prefix, female_prefix, male_prefix}' aren't applicable to this locale.

I feel the library should not be throwing for the built-in locales and methods.

I appreciate that for these locales we don't know what to return but instead of doing null would it be terrible if the locale definition returned an empty string or English with a console warning instead of an error?

For example in my application I dynamically allow a user to select a locale and then generate some data using faker methods. With the current implementation I'm going to need to wrap each method call with try/catch as I still want some partial data instead of returning nothing if only one method fails.

i.e.

import { allFakers } from '@faker-js/faker';

function generateData(locale: keyof allFakers) {
  const faker = allFakers[locale];
  const data: Record<string, string> = {};
  
  try {
    data.name = faker.person.name();
  } catch(e) { /* ignore */ }
  
  try {
    data.prefix = faker.person.prefix();
  } catch(e) { /* ignore */ }  
  
  // ... etc
  
  return data;
}

Vs if faker handling locales

import { allFakers } from '@faker-js/faker';

function generateData(locale: keyof allFakers) {
  const faker = allFakers[locale];
  return {
    name: faker.person.name(),
    prefix: faker.person.prefix(),
    // ... etc
  }  
}  

[matthewmayer]

The methods which currently throw when called with default parameters are as follows:

When this is deliberate I think it would be good to mark these methods with @throws with a more detailed explanation as to why it throws, e.g.
@throws in some locales which do not have ZIP codes
or
@throws in some locales which do not not use name prefixes

Presumably that would also allow tooling to indicate when a try-catch is needed?

company.companySuffix: az
company.suffixes: az
location.state: az,ro_MD,sk
location.stateAbbr: cs_CZ,ro_MD,sk
location.zipCode: en_HK
location.zipCodeByState: en_HK
person.jobArea: ar,fr,fr_BE,fr_CA,fr_CH,fr_LU
person.jobDescriptor: ar,fr,fr_BE,fr_CA,fr_CH,fr_LU
person.jobTitle: ar,fr,fr_BE,fr_CA,fr_CH,fr_LU,ur
person.jobType: ur
person.prefix: az,id_ID,ru,zh_CN,zh_TW
person.suffix: az,it,mk,pt_PT,ro_MD,ru

Quick and dirty script for finding these

const {
  allFakers,
  faker
} = require("@faker-js/faker")
const keys = Object.keys(allFakers)
const missing = {}
for (let key of keys) {
  if (!["base"].includes(key)) {
    const localFaker = allFakers[key]
    let modules = Object.keys(faker)
    for (let module of modules) {
      if (!["rawDefinitions", "definitions", "helpers", "image", "string", "random"].includes(module)) {
        let methods = Object.keys(faker[module])
        for (let method of methods) {
          if (!["faker"].includes(method)) {
            try {
              const result = localFaker[module][method]()
            } catch (e) {
              if (!missing[`${module}.${method}`]) {
                missing[`${module}.${method}`] = []
              }
              missing[`${module}.${method}`].push(key)
            }
          }
        }
      }
    }
  }
}
for (let key2 of Object.keys(missing)) {
  console.log(key2 + ": " + missing[key2].join(","))
}

[ST-DDT]

Well, then we would have to put the throws label on every method that uses locale data.
Because technically you could use an empty locale.

Returning undefined can be just as bad for those that enable strictNotNull checks.

[ST-DDT]

Workaround:

/*
 * Defaults to undefined if the method throws an error.
 */
function du(fn: () => string) : string | undefined {
  try {
    return fn();
  } catch {
    return undefined;
  }
}

Usage:

const jobTitle = du(() => faker.person.jobTitle());

[matthewmayer]

Well, then we would have to put the throws label on every method that uses locale data.

Because technically you could use an empty locale.

Returning undefined can be just as bad for those that enable strictNotNull checks.

We don't have to do that. We should document things that will likely trip up developers in built in methods and locales.

[labithiotis]

@ST-DDT that workaround is okay and maybe acceptable for methods you know that will throw. However, as this could be indeterminate from consumers' point of view, you are asking us to wrap every method just in case it will throw.

I can easily see others having failing tests where the test uses a random locale and some methods throw during the test suite. The PR to improve the error message will help isolate the cause.

I do agree that returning undefined isn't an acceptable solution.

Would it be terrible to fall back to the en locale definitions for these locales?

[xDivisionByZerox]

We don't have to do that. We should document things that will likely trip up developers in built in methods and locales.

Have to disagree with you here. Would be super confusing for me as a dev if some functions have a specific throw description because they are "more likely" to throw, but others don't yet they (could) throw will throw.

[xDivisionByZerox]

@ST-DDT that workaround is okay and maybe acceptable for methods you know that will throw. However, as this could be indeterminate from consumers' point of view, you are asking us to wrap every method just in case it will throw.

Yes and no. What we really want is the community to get active to provide missing locale data. That is the reason why we explicitly decided on throwing for missing locale data. Nevertheless, I can understand that this might be frustrating, but you can always choose to use the default faker jnstance to ensure that all methods work.

I do agree that returning undefined isn't an acceptable solution.

Good that we agree on that.

Would it be terrible to fall back to the en locale definitions for these locales?

It's not terrible, but we explicitly decided against that (I can provide a reference link later, currently on my phone). The reason is that if you use a specific locale, you do that on purpose to omit English locale data. If that wouldn't be the case, you would simply use the English locale.

---

Another workaround would be to create your mergeLocales function and build a faker instance with a custom locale consisting of locale you want, including en as fallback.

[ST-DDT]

Would it be terrible to fall back to the en locale definitions for these locales?

It's not terrible, but we explicitly decided against that (I can provide a reference link later, currently on my phone). The reason is that if you use a specific locale, you do that on purpose to omit English locale data. If that wouldn't be the case, you would simply use the English locale.

The prebuilt instances actually do fallback to en for missing data. Non applicable data on the other hand don't fall back. Unfortunately, that is the error you are running into.
The thing is, if that locale doesn't have the concept of prefixes, then the user's data model has to support that and since we cannot/don't want to return undefined, all we can do is throw to raise awareness of the issue.

As @xDivisionByZerox mentioned, you can always create your own locale to circumvent the issue, or just omit locales, that don't work for your usecase.

EDIT

See also: https://fakerjs.dev/api/utils.html#mergelocales

You can eliminate non applicable data by replacing

faker/src/utils/merge-locales.ts

Line 22 in 30557b9

if (merged[key] === undefined) {

with

if (merged[key] == null) {

in your copy of that method.

@xDivisionByZerox Should we add an option (disabled by default) to our method for that? 🤔

[labithiotis]

@xDivisionByZerox @ST-DDT Ok, thanks for your rapid response thus far.

I have a few work arounds:

1. wrap each with try/catch or function handler
2. omit those locales from my options
3. use mergeLocales

For 3 I tried this, but it still throws an error am I missing something?

function fakerFallback(faker: typeof Faker): typeof Faker {
  for (const locale in faker.allFakers) {
    Object.defineProperty(
      faker.allFakers,
      locale,
      faker.mergeLocales([
        faker.allLocales[locale as keyof typeof faker.allFakers],
        faker.allLocales.en,
      ])
    );
  }

  return faker;
}

// I would expect calling the following to fallback to english?
faker.allFakers.az.person.prefix();

[matthewmayer]

I think adding @throws to every method that potentially throws with missing locale data would also raise awareness of the issue. Is there any downside to doing this?

[ST-DDT]

For 3 I tried this, but it still throws an error am I missing something?

@labithiotis You have to use your own variant of mergeLocales that has the changes I outlined in the edit here: #2503 (comment)

[labithiotis]

Current mergeLocales is only merging on top-level definitions, i.e. if person is null in the locale then it will merge the next locale. But in my case person is never null it's the nested value inside person called a prefix that is sometimes null. Even after doing a custom merge to have locales merged as expected (person.prefix for az falls back to en), I'm still getting an error as the faker methods are using the rawDiffintions as mentioned by @xDivisionByZerox #2503 (comment)

Also happens on the PR to improve the error messaging too.

[ST-DDT]

Yeah, I think I might have mad a mistake with the suggested mergeLocales changes.

Here the fixed workaround:

export function mergeLocales(locales: LocaleDefinition[]): LocaleDefinition {
  const merged: LocaleDefinition = {};

  for (const locale of locales) {
    for (const key in locale) {
      const value = locale[key];
      if (merged[key] === undefined) {
        merged[key] = { ...value };
      } else {
        merged[key] = { ...value, ...merged[key] };
      }
      // Remove explicitly absent values
      const module = merged[key];
      for (const entryKey in module) {
        if (module[entryKey] == null) {
          delete module[entryKey];
        }
      }
    }
  }

  return merged;
}

[ST-DDT]

Team Decision

• We will not add a @throws to every method that uses localized data.
• We will extend our localization docs to include a paragraph about explicitly absent data. There we might add a hint how to overwrite the merge behavior.
• For now, we will not extend our mergeLocales method with a parameter/option to the change the merge strategy. If there is interest in that please create a separate feature request issue.