Introduce a way to get details about missing locale data when using them

[ST-DDT]

Clear and concise description of the problem

Currently many methods look like this:

foobar(): string {
    return this.faker.random.arrayElement(this.faker.definitions.foo.bar);
}

However, the current locale does not have a value for it, then it will return 'a' | 'b' | 'c'.
It would be nice, if the method would throw an error, with details about missing data.
It should still be possible to check whether the data exists without throwing and catching an error.

This kind of blocks the removal of the default from random.arrayElement()

Suggested solution

Change the faker.definitions to throw an error, if the data are absent.
Introduce faker.extensions that would return undefined (the current faker.definitions behavior).

(Not sure about the name)

Alternative

Check the data before using it each time. This reduces the code readability a lot.

Additional context

IMO faker.definitions should change its behavior, instead of adding a new field for it, because I assume, the default expectation is that the data are present. Most of our methods wouldn't change either. Only some of them would, where the data are explicitly optional.

[Shinigami92]

Just to prioritize some issues, I would see

• Improve locale loading and global locale data #823

should be handled before this issue

[ST-DDT]

We might also need a way to mark/handle explicitly missing data.
#1665 (comment) introduces empty arrays for that purpose.

[ST-DDT]

Team decision

If faker.definitions.category.entry is not defined we will throw an error.

[xDivisionByZerox]

What blocks us from "simply" making all definitions required? If we currently don't have any they could be filled with an empty array. Then the next step would be to throw an error in helpers.arrayElement when an empty array is passed.

Is that the desired behavior or did I misunderstood the issue?

[matthewmayer]

The current behavior is to fallback to the fallback locale ie English en which is defined for nearly everything.

In which cases would we want to explicitly throw an error?

[xDivisionByZerox]

The current behavior is to fallback to the fallback locale ie English en which is defined for nearly everything.

Yes, that is correct. But this happens when accessing the actual locale data via the Proxy:

faker/src/faker.ts

Lines 191 to 196 in f9e5ba7

	const resolveModuleData = (
	module: keyof LocaleDefinition,
	entry: string
	): unknown =>
	this.locales[this.locale][module]?.[entry] ??
	this.locales[this.localeFallback][module]?.[entry];

Which could simply be replaced by checking against the entries length instead of its existence.

In which cases would we want to explicitly throw an error?

I'm talking about two different things here (locale data accessing and helpers.arrayElement behavior). I'm currently more focused on throwing an error in helpers.arrayElement if an empty array is passed as a parameter.

[ST-DDT]

What blocks us from "simply" making all definitions required?

Currently only:

• faker/src/modules/location/index.ts

  Line 95 in f9e5ba7

  const zipRange = this.faker.definitions.location.postcode_by_state?.[state];

  (to be made required in refactor(location)!: throw error if state is unknown #1760)

But we have a PR that adds more:

• #1819 fix(person): avoid repeated double last name via new last_name_patterns

If we currently don't have any they could be filled with an empty array.

There are three problems with that.

1. We would have to check that in each method and would have to throw an easy to understand error message.
   • Thus reducing the readability of the implementation
   • Or loosing DX if we just let that empty array wander over to helpers.arrayElement
2. We are likely to miss adding [] for new/other locales for these required values
   • We will likely bloat our locales with "empty" files
   • We cannot safely add this via Proxy because of 3
   • We loose the ability to distinguish between missing/not yet provided data and explicitly unsupported/unavailable data
3. Not all our locale data are arrays, some are objects (e.g. location.postcode_by_state), some might be singular data (locale title, locale code, ...)

Then the next step would be to throw an error in helpers.arrayElement when an empty array is passed.

This issue is explictly about improving the DX by exposing which data are missing:

new FakerError("This faker instance does not have any data for ``location.postcode_by_state``. Please make sure you configured the correct fallbacks or create a PR that adds these data to Faker for your locale.");

instead of:

new FakerError("Cannot invoke helpers.arrayElement() with an empty array")

[ST-DDT]

The current behavior is to fallback to the fallback locale ie English en which is defined for nearly everything.

In which cases would we want to explicitly throw an error?

In the case where you don't want the English fallback and you explicitly want only data in your selected locale.
(You will then get an explicit error, requesting you to provide the data, instead of silently falling back to English values)

[ST-DDT]

The current behavior is to fallback to the fallback locale ie English en which is defined for nearly everything.

Yes, that is correct. But this happens when accessing the actual locale data via the Proxy:

Which IMO is likely to change in implementation, but will still be some kind of Proxy (to throw useful error messages).

Which could simply be replaced by checking against the entries length instead of its existence.

See #893 (comment) (1. argument)

I'm currently more focused on throwing an error in helpers.arrayElement if an empty array is passed as a parameter.

Which is a different issue altogether: #219

[matthewmayer]

Ok so this this mostly a internal issue

For most consumers of the library nothing much changes unless you are explicitly not falling back to en (which requires a non-default setup) or using one of the rare methods which don't have a en fallback?

[ST-DDT]

Note to self: Do it.

[ST-DDT]

I have to create a PR for moving definitions.title to definitions.metadata.title first.

[ST-DDT]

Blocked by #1978.

• refactor(locale)!: move title to metadata #1978

[matthewmayer]

I've been thinking about this, and perhaps this could be an explicit option on the Faker instance, because in some cases you might want an error, and in some cases e.g. if you are directly using calls to Faker in templates, you would rather just have undefined returned.

Suppose you have a method which is only defined in certain locales (say faker.location.county() because not all countries have 2nd lebel admin areas, or faker.person.prefix(), because not all languages use prefixes in names.

Then you could do

fakerXX.location.county() // undefined
fakerXX.throwsErrorOnMissingData = true
fakerXX.location.county() // throws '"This faker instance does not have any data for `location.county`. Please make sure you configured the correct fallbacks or create a PR that adds these data to Faker for your locale.'

[ST-DDT]

Team Decision

fakerXX.throwsErrorOnMissingData = true

While this is an interesting feature, it is not possible to implement this in a type safe way.

If we add | undefined to every method then all our users would have to use ! when generating values, which will likely cause lint warnings.
If we don't add it, then we violate TS types.

There might be ways to handle that using some kind of Faker type overwrite.
However that will probably break with our tree shakign efforts in v8.2.
So we won't implement this for now.

[matthewmayer]

Team Decision

fakerXX.throwsErrorOnMissingData = true

While this is an interesting feature, it is not possible to implement this in a type safe way.

If we add | undefined to every method then all our users would have to use ! when generating values, which will likely cause lint warnings.

If we don't add it, then we violate TS types.

There might be ways to handle that using some kind of Faker type overwrite.

However that will probably break with our tree shakign efforts in v8.2.

So we won't implement this for now.

That's why JavaScript is better than typescript 😂😂😂

[ST-DDT]

Team Decision

We want this for v8.0.