From 8a0bbf5faa03c294d308a13fe210ba6aaeef6968 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 23 Apr 2023 18:51:27 +0200 Subject: [PATCH] feat: introduce locale proxy (#2004) --- docs/guide/upgrading.md | 42 ++++++++ src/definitions/definitions.ts | 2 +- src/faker.ts | 8 +- src/locale-proxy.ts | 91 ++++++++++++++++ src/modules/helpers/index.ts | 2 +- src/modules/location/index.ts | 3 +- src/modules/person/index.ts | 8 +- test/all_functional.spec.ts | 1 + test/faker.spec.ts | 23 +++- test/locale-proxy.spec.ts | 189 +++++++++++++++++++++++++++++++++ test/location.spec.ts | 10 ++ test/person.spec.ts | 24 ++--- 12 files changed, 378 insertions(+), 25 deletions(-) create mode 100644 src/locale-proxy.ts create mode 100644 test/locale-proxy.spec.ts diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 4a5a2ea9322..0572f5f1149 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -97,6 +97,48 @@ for (let user of users) { For more information refer to our [Localization Guide](localization). +### For missing locale data, Faker will now throw instead of returning `undefined` or `a`-`c` + +::: note Note +The following section mostly applies to custom-built Faker instances. +::: + +Previously, for example if `en` didn't have data for `animal.cat`, then `faker.animal.cat()` would have returned one of `a`, `b` or `c` (`arrayElement`'s default value). +These values aren't expected/useful as a fallback and potentially also violate the method's defined return type definitions (in case it doesn't return a `string`). + +We have now addressed this by changing the implementation so that an error is thrown, prompting you to provide/contribute the missing data. +This will also give you detailed information which data are missing. +If you want to check for data you can either use `entry in faker.definitions.category` or use `faker.rawDefinitions.category?.entry` instead. + +```ts +import { Faker, fakerES, es } from '@faker-js/faker'; + +const fakerES_noFallbacks = new Faker({ + locale: [es], +}); +fakerES.music.songName(); // 'I Want to Hold Your Hand' (fallback from en) +// Previously: +//fakerES_noFallbacks.music.songName(); // 'b' +// Now: +fakerES_noFallbacks.music.songName(); // throws a FakerError +``` + +This also has an impact on data that aren't applicable to a locale, for example Chinese doesn't use prefixes in names. + +```ts +import { faker, fakerZH_CN, zh_CN } from '@faker-js/faker'; + +const fakerZH_CN_noFallbacks = new Faker({ + locale: [zh_CN], +}); + +faker.name.prefix(); // 'Mr' +// Previously: +//fakerZH_CN_noFallbacks.person.prefix(); // undefined +// Now: +fakerZH_CN.person.prefix(); // throws a FakerError +``` + ### `faker.mersenne` and `faker.helpers.repeatString` removed `faker.mersenne` and `faker.helpers.repeatString` were only ever intended for internal use, and are no longer available. diff --git a/src/definitions/definitions.ts b/src/definitions/definitions.ts index 413b2bd761b..87399408f7f 100644 --- a/src/definitions/definitions.ts +++ b/src/definitions/definitions.ts @@ -52,4 +52,4 @@ export type LocaleDefinition = { system?: SystemDefinitions; vehicle?: VehicleDefinitions; word?: WordDefinitions; -} & Record>; +} & Record | undefined>; diff --git a/src/faker.ts b/src/faker.ts index a694fcc2c60..e715234d0ec 100644 --- a/src/faker.ts +++ b/src/faker.ts @@ -3,6 +3,8 @@ import { FakerError } from './errors/faker-error'; import { deprecated } from './internal/deprecated'; import type { Mersenne } from './internal/mersenne/mersenne'; import mersenne from './internal/mersenne/mersenne'; +import type { LocaleProxy } from './locale-proxy'; +import { createLocaleProxy } from './locale-proxy'; import { AirlineModule } from './modules/airline'; import { AnimalModule } from './modules/animal'; import { ColorModule } from './modules/color'; @@ -59,7 +61,8 @@ import { mergeLocales } from './utils/merge-locales'; * customFaker.music.genre(); // throws Error as this data is not available in `es` */ export class Faker { - readonly definitions: LocaleDefinition; + readonly rawDefinitions: LocaleDefinition; + readonly definitions: LocaleProxy; private _defaultRefDate: () => Date = () => new Date(); /** @@ -329,7 +332,8 @@ export class Faker { locale = mergeLocales(locale); } - this.definitions = locale as LocaleDefinition; + this.rawDefinitions = locale as LocaleDefinition; + this.definitions = createLocaleProxy(this.rawDefinitions); } /** diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts new file mode 100644 index 00000000000..c5c6aa1b733 --- /dev/null +++ b/src/locale-proxy.ts @@ -0,0 +1,91 @@ +import type { LocaleDefinition } from './definitions'; +import { FakerError } from './errors/faker-error'; + +/** + * A proxy for LocaleDefinitions that marks all properties as required and throws an error when an entry is accessed that is not defined. + */ +export type LocaleProxy = Readonly<{ + [key in keyof LocaleDefinition]-?: Readonly< + Required> + >; +}>; + +const throwReadOnlyError: () => never = () => { + throw new FakerError('You cannot edit the locale data on the faker instance'); +}; + +/** + * Creates a proxy for LocaleDefinition that throws an error if an undefined property is accessed. + * + * @param locale The locale definition to create the proxy for. + */ +export function createLocaleProxy(locale: LocaleDefinition): LocaleProxy { + const proxies = {} as LocaleDefinition; + return new Proxy(locale, { + has(): true { + // Categories are always present (proxied), that's why we return true. + return true; + }, + + get( + target: LocaleDefinition, + categoryName: keyof LocaleDefinition + ): LocaleDefinition[keyof LocaleDefinition] { + if (categoryName in proxies) { + return proxies[categoryName]; + } + + return (proxies[categoryName] = createCategoryProxy( + categoryName, + target[categoryName] + )); + }, + + set: throwReadOnlyError, + deleteProperty: throwReadOnlyError, + }) as LocaleProxy; +} + +/** + * Creates a proxy for a category that throws an error when accessing an undefined property. + * + * @param categoryName The name of the category. + * @param categoryData The module to create the proxy for. + */ +function createCategoryProxy< + CategoryData extends Record +>( + categoryName: string, + categoryData: CategoryData = {} as CategoryData +): Required { + return new Proxy(categoryData, { + has(target: CategoryData, entryName: keyof CategoryData): boolean { + const value = target[entryName]; + return value != null; + }, + + get( + target: CategoryData, + entryName: keyof CategoryData + ): CategoryData[keyof CategoryData] { + const value = target[entryName]; + if (value === null) { + throw new FakerError( + `The locale data for '${categoryName}.${entryName.toString()}' aren't applicable to this locale. + If you think this is a bug, please report it at: https://github.com/faker-js/faker` + ); + } else if (value === undefined) { + throw new FakerError( + `The locale data for '${categoryName}.${entryName.toString()}' are missing in this locale. + Please contribute the missing data to the project or use a locale/Faker instance that has these data. + For more information see https://next.fakerjs.dev/guide/localization.html` + ); + } else { + return value; + } + }, + + set: throwReadOnlyError, + deleteProperty: throwReadOnlyError, + }) as Required; +} diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 48c8cea09c3..0bb54fb631a 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -1160,7 +1160,7 @@ export class HelpersModule { const parts = method.split('.'); let currentModuleOrMethod: unknown = this.faker; - let currentDefinitions: unknown = this.faker.definitions; + let currentDefinitions: unknown = this.faker.rawDefinitions; // Search for the requested method or definition for (const part of parts) { diff --git a/src/modules/location/index.ts b/src/modules/location/index.ts index c39711b786f..26ead8ad5f7 100644 --- a/src/modules/location/index.ts +++ b/src/modules/location/index.ts @@ -73,8 +73,7 @@ export class LocationModule { const { state } = options; if (state) { - const zipRange = - this.faker.definitions.location.postcode_by_state?.[state]; + const zipRange = this.faker.definitions.location.postcode_by_state[state]; if (zipRange) { return String(this.faker.number.int(zipRange)); diff --git a/src/modules/person/index.ts b/src/modules/person/index.ts index 789cd58c549..428b1373446 100644 --- a/src/modules/person/index.ts +++ b/src/modules/person/index.ts @@ -102,7 +102,7 @@ export class PersonModule { */ firstName(sex?: SexType): string { const { first_name, female_first_name, male_first_name } = - this.faker.definitions.person; + this.faker.rawDefinitions.person ?? {}; return selectDefinition(this.faker, this.faker.helpers.arrayElement, sex, { generic: first_name, @@ -132,7 +132,7 @@ export class PersonModule { last_name_pattern, male_last_name_pattern, female_last_name_pattern, - } = this.faker.definitions.person; + } = this.faker.rawDefinitions.person ?? {}; if ( last_name_pattern != null || @@ -174,7 +174,7 @@ export class PersonModule { */ middleName(sex?: SexType): string { const { middle_name, female_middle_name, male_middle_name } = - this.faker.definitions.person; + this.faker.rawDefinitions.person ?? {}; return selectDefinition(this.faker, this.faker.helpers.arrayElement, sex, { generic: middle_name, @@ -315,7 +315,7 @@ export class PersonModule { */ prefix(sex?: SexType): string { const { prefix, female_prefix, male_prefix } = - this.faker.definitions.person; + this.faker.rawDefinitions.person ?? {}; return selectDefinition(this.faker, this.faker.helpers.arrayElement, sex, { generic: prefix, diff --git a/test/all_functional.spec.ts b/test/all_functional.spec.ts index b55012901a2..25619c9e774 100644 --- a/test/all_functional.spec.ts +++ b/test/all_functional.spec.ts @@ -3,6 +3,7 @@ import type { allLocales, Faker, RandomModule } from '../src'; import { allFakers, fakerEN } from '../src'; const IGNORED_MODULES = [ + 'rawDefinitions', 'definitions', 'helpers', '_mersenne', diff --git a/test/faker.spec.ts b/test/faker.spec.ts index 57f291c01be..207dac4a7a3 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -30,16 +30,33 @@ describe('faker', () => { } }); + describe('rawDefinitions', () => { + it('locale rawDefinition accessibility', () => { + // Metadata + expect(faker.rawDefinitions.metadata.title).toBeDefined(); + // Standard modules + expect(faker.rawDefinitions.location?.city_name).toBeDefined(); + // Non-existing module + expect(faker.rawDefinitions.missing).toBeUndefined(); + // Non-existing definition in a non-existing module + expect(faker.rawDefinitions.missing?.missing).toBeUndefined(); + // Non-existing definition in an existing module + expect(faker.rawDefinitions.location?.missing).toBeUndefined(); + }); + }); + describe('definitions', () => { - it('locale definition accessability', () => { + it('locale definition accessibility', () => { // Metadata expect(faker.definitions.metadata.title).toBeDefined(); // Standard modules expect(faker.definitions.location.city_name).toBeDefined(); // Non-existing module - expect(faker.definitions.missing).toBeUndefined(); + expect(faker.definitions.missing).toBeDefined(); + // Non-existing definition in a non-existing module + expect(() => faker.definitions.missing.missing).toThrow(); // Non-existing definition in an existing module - expect(faker.definitions.location.missing).toBeUndefined(); + expect(() => faker.definitions.location.missing).toThrow(); }); }); diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts new file mode 100644 index 00000000000..88ce9555645 --- /dev/null +++ b/test/locale-proxy.spec.ts @@ -0,0 +1,189 @@ +import { describe, expect, it } from 'vitest'; +import type { MetadataDefinitions } from '../src'; +import { en, FakerError } from '../src'; +import { createLocaleProxy } from '../src/locale-proxy'; + +describe('LocaleProxy', () => { + const locale = createLocaleProxy(en); + const enAirline = en.airline ?? { never: 'missing' }; + + describe('category', () => { + it('should be possible to check for a missing category', () => { + expect('category' in locale).toBe(true); + }); + + it('should be possible to check for an existing category', () => { + expect('airline' in locale).toBe(true); + }); + + it('should be possible to access the title', () => { + expect(locale.metadata.title).toBe('English'); + }); + + it('should be possible to access a missing category', () => { + expect(locale.category).toBeDefined(); + }); + + it('should not be possible to add a new category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + locale.category = {}; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to replace a category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + locale.airline = {}; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to delete a missing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + delete locale.category; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to delete an existing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + delete locale.airline; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should be possible to get all categories keys on empty locale', () => { + const empty = createLocaleProxy({ metadata: {} as MetadataDefinitions }); + + expect(Object.keys(empty)).toEqual(['metadata']); + }); + + it('should be possible to get all categories keys on actual locale', () => { + expect(Object.keys(locale).sort()).toEqual(Object.keys(en).sort()); + }); + }); + + describe('entry', () => { + it('should be possible to check for a missing entry in a missing category', () => { + expect('missing' in locale.category).toBe(false); + }); + + it('should be possible to check for a missing entry in a present category', () => { + expect('missing' in locale.airline).toBe(false); + }); + + it('should be possible to check for a present entry', () => { + expect('airline' in locale.airline).toBe(true); + }); + + it('should not be possible to access a missing entry in a missing category', () => { + expect(() => locale.category.missing).toThrowError( + new FakerError( + `The locale data for 'category.missing' are missing in this locale. + Please contribute the missing data to the project or use a locale/Faker instance that has these data. + For more information see https://next.fakerjs.dev/guide/localization.html` + ) + ); + }); + + it('should not be possible to access a missing entry in a present category', () => { + expect(() => locale.airline.missing).toThrowError( + new FakerError( + `The locale data for 'airline.missing' are missing in this locale. + Please contribute the missing data to the project or use a locale/Faker instance that has these data. + For more information see https://next.fakerjs.dev/guide/localization.html` + ) + ); + }); + + it('should be possible to access a present entry', () => { + expect(locale.airline.airline).toBeDefined(); + }); + + it('should not be possible to access an unavailable entry in a present category', () => { + const unavailable = createLocaleProxy({ + metadata: {} as MetadataDefinitions, + airline: { airline: null }, + }); + + expect(() => unavailable.airline.airline).toThrowError( + new FakerError( + `The locale data for 'airline.airline' aren't applicable to this locale. + If you think this is a bug, please report it at: https://github.com/faker-js/faker` + ) + ); + }); + + it('should not be possible to add a new entry in a missing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + locale.category.missing = {}; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to add a new entry in an existing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + locale.airline.missing = {}; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to replace an entry in an existing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + locale.airline.airline = ['dummy']; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to delete a missing entry in a missing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + delete locale.category.missing; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to delete a missing entry in an existing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + delete locale.airline.missing; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should not be possible to delete an existing entry in an existing category', () => { + expect(() => { + // @ts-expect-error: LocaleProxy is read-only. + delete locale.airline.airline; + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); + }); + + it('should be possible to get all keys from missing category', () => { + expect(Object.keys(locale.missing)).toEqual([]); + }); + + it('should be possible to get all keys from existing category', () => { + expect(Object.keys(locale.airline).sort()).toEqual( + Object.keys(enAirline).sort() + ); + }); + }); +}); diff --git a/test/location.spec.ts b/test/location.spec.ts index 78f32b534cb..9ea62cd9bc7 100644 --- a/test/location.spec.ts +++ b/test/location.spec.ts @@ -181,6 +181,16 @@ describe('location', () => { it('should throw when definitions.location.postcode_by_state not set', () => { expect(() => faker.location.zipCode({ state: 'XX' })).toThrow( + new FakerError( + `The locale data for 'location.postcode_by_state' are missing in this locale. + Please contribute the missing data to the project or use a locale/Faker instance that has these data. + For more information see https://next.fakerjs.dev/guide/localization.html` + ) + ); + }); + + it('should throw when definitions.location.postcode_by_state[state] is unknown', () => { + expect(() => fakerEN_US.location.zipCode({ state: 'XX' })).toThrow( new FakerError('No zip code definition found for state "XX"') ); }); diff --git a/test/person.spec.ts b/test/person.spec.ts index dbd2169db73..7465efbdd12 100644 --- a/test/person.spec.ts +++ b/test/person.spec.ts @@ -122,10 +122,10 @@ describe('person', () => { it('should return a female sex-specific name without firstName and lastName', () => { const female_specific = [ - ...fakerMK.definitions.person.female_prefix, - ...fakerMK.definitions.person.female_first_name, - ...fakerMK.definitions.person.female_last_name, - // ...fakerMK.definitions.person.suffix, // Not applicable + ...(fakerMK.rawDefinitions.person?.female_prefix ?? []), + ...(fakerMK.rawDefinitions.person?.female_first_name ?? []), + ...(fakerMK.rawDefinitions.person?.female_last_name ?? []), + // ...(fakerMK.rawDefinitions.person?.suffix ?? []), Not applicable ]; const fullName = fakerMK.person.fullName({ sex: 'female' }); @@ -138,10 +138,10 @@ describe('person', () => { it('should return a male sex-specific name without firstName and lastName', () => { const male_specific = [ - ...fakerMK.definitions.person.male_prefix, - ...fakerMK.definitions.person.male_first_name, - ...fakerMK.definitions.person.male_last_name, - // ...fakerMK.definitions.person.suffix, // Not applicable + ...(fakerMK.rawDefinitions.person?.male_prefix ?? []), + ...(fakerMK.rawDefinitions.person?.male_first_name ?? []), + ...(fakerMK.rawDefinitions.person?.male_last_name ?? []), + // ...(fakerMK.rawDefinitions.person?.suffix ?? []), Not applicable ]; const fullName = fakerMK.person.fullName({ sex: 'male' }); @@ -154,10 +154,10 @@ describe('person', () => { it('should return a female sex-specific name with given firstName and lastName', () => { const male_specific = [ - ...fakerMK.definitions.person.female_prefix, + ...(fakerMK.rawDefinitions.person?.female_prefix ?? []), 'firstName', 'lastName', - // ...fakerMK.definitions.person.suffix, // Not applicable + // ...(fakerMK.rawDefinitions.person?.suffix ?? []), Not applicable ]; const fullName = fakerMK.person.fullName({ @@ -174,10 +174,10 @@ describe('person', () => { it('should return a male sex-specific name with given firstName and lastName', () => { const male_specific = [ - ...fakerMK.definitions.person.male_prefix, + ...(fakerMK.rawDefinitions.person?.male_prefix ?? []), 'firstName', 'lastName', - //...fakerMK.definitions.person.suffix, // Not applicable + // ...(fakerMK.rawDefinitions.person?.suffix ?? []), Not applicable ]; const fullName = fakerMK.person.fullName({