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

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:

• #1677

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

from faker.

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.

from faker.

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.

from faker.

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.

from faker.

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.

from faker.

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
  }  
}  

from faker.

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

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(","))
}

from faker.

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());

from faker.

@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?

from faker.

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.

from faker.

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

from faker.

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

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? 🤔

from faker.

@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();

from faker.

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?

from faker.

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)

from faker.

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.

from faker.

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;
}

from faker.