From f688434e207af26a3ce83617d6680ad3ce104369 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 1 Apr 2023 23:54:17 +0200 Subject: [PATCH 01/29] feat: introduce locale access proxy --- src/definitions/definitions.ts | 2 +- src/faker.ts | 8 +- src/locale-proxy.ts | 98 ++++++++++++++++++++++++ src/modules/helpers/index.ts | 2 +- src/modules/location/index.ts | 3 +- src/modules/person/index.ts | 19 +++-- test/__snapshots__/location.spec.ts.snap | 6 -- test/all_functional.spec.ts | 43 +++++++++++ test/faker.spec.ts | 17 +++- test/locale-proxy.spec.ts | 93 ++++++++++++++++++++++ test/location.spec.ts | 5 +- test/person.spec.ts | 24 +++--- 12 files changed, 284 insertions(+), 36 deletions(-) create mode 100644 src/locale-proxy.ts create mode 100644 test/locale-proxy.spec.ts diff --git a/src/definitions/definitions.ts b/src/definitions/definitions.ts index 112c2c1aa64..55bf51fe5df 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 b799418950c..9dffa41f78e 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 { LocaleAccess } from './locale-proxy'; +import { createLocaleAccess } from './locale-proxy'; import { AirlineModule } from './modules/airline'; import { AnimalModule } from './modules/animal'; import { ColorModule } from './modules/color'; @@ -60,7 +62,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: LocaleAccess; private _defaultRefDate: () => Date = () => new Date(); /** @@ -330,7 +333,8 @@ export class Faker { locale = mergeLocales(locale); } - this.definitions = locale as LocaleDefinition; + this.rawDefinitions = locale as LocaleDefinition; + this.definitions = createLocaleAccess(this.rawDefinitions); } /** diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts new file mode 100644 index 00000000000..ea82597591c --- /dev/null +++ b/src/locale-proxy.ts @@ -0,0 +1,98 @@ +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 LocaleAccess = Readonly<{ + [key in keyof LocaleDefinition]-?: Readonly< + Required> + >; +}>; + +/** + * Creates a proxy for LocaleDefinition that throws an error when a property is accessed that is not defined. + * + * @param locale The locale definition to create the proxy for. + */ +export function createLocaleAccess(locale: LocaleDefinition): LocaleAccess { + return new Proxy({} as LocaleDefinition, { + has(): true { + return true; + }, + + get( + target, + categoryName: keyof LocaleDefinition + ): LocaleDefinition[keyof LocaleDefinition] { + if (categoryName in target) { + return target[categoryName]; + } + + return (target[categoryName] = createCategoryProxy( + categoryName, + locale[categoryName] + )); + }, + + set(): never { + throw new FakerError('LocaleAccess is read-only.'); + }, + + deleteProperty(): never { + throw new FakerError('LocaleAccess is read-only.'); + }, + + ownKeys(): Array { + return Object.keys(locale); + }, + }) as LocaleAccess; +} + +/** + * Creates a proxy for a category that throws an error when a property is accessed that is not defined. + * + * @param categoryName The name of the category. + * @param categoryData The module to create the proxy for. + */ +function createCategoryProxy>( + categoryName: string, + categoryData: T = {} as T +): Required { + return new Proxy({} as Required, { + has(_, entryName: keyof T): boolean { + const value = categoryData[entryName]; + return value != null && (!Array.isArray(value) || value.length !== 0); + }, + + get(_, entryName: keyof T): T[keyof T] { + const value = categoryData[entryName]; + if (value == null) { + 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 if (Array.isArray(value) && value.length === 0) { + 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 { + return value; + } + }, + + set(): never { + throw new FakerError('LocaleAccess is read-only.'); + }, + + deleteProperty(): never { + throw new FakerError('LocaleAccess is read-only.'); + }, + + ownKeys(): Array { + return Object.keys(categoryData); + }, + }); +} diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index a5805af8030..82eca4b8cd8 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -1139,7 +1139,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 ab158ecbc88..a9e6ecf9a34 100644 --- a/src/modules/location/index.ts +++ b/src/modules/location/index.ts @@ -102,7 +102,8 @@ export class LocationModule { const { state } = options; - const zipRange = this.faker.definitions.location.postcode_by_state?.[state]; + const zipRange = + this.faker.rawDefinitions.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 b1722bc0027..9347656c699 100644 --- a/src/modules/person/index.ts +++ b/src/modules/person/index.ts @@ -57,21 +57,20 @@ function selectDefinition( * Module to generate people's personal information such as names and job titles. Prior to Faker 8.0.0, this module was known as `faker.name`. * * ### Overview - * + * * To generate a full name, use [`fullName`](https://next.fakerjs.dev/api/person.html#fullname). Note that this is not the same as simply concatenating [`firstName`](https://next.fakerjs.dev/api/person.html#firstname) and [`lastName`](https://next.fakerjs.dev/api/person.html#lastname), as the full name may contain a prefix, suffix, or both. Additionally, different supported locales will have differing name patterns. For example, the last name may appear before the first name, or there may be a double or hyphenated first or last name. - * + * * You can also generate the parts of a name separately, using [`prefix`](https://next.fakerjs.dev/api/person.html#prefix), [`firstName`](https://next.fakerjs.dev/api/person.html#firstname), [`middleName`](https://next.fakerjs.dev/api/person.html#middlename), [`lastName`](https://next.fakerjs.dev/api/person.html#lastname), and [`suffix`](https://next.fakerjs.dev/api/person.html#suffix). Not all locales support all of these parts. * * Many of the methods in this module can optionally choose either female, male or mixed names. * - * Job-related data is also available. To generate a job title, use [`jobTitle`](https://next.fakerjs.dev/api/person.html#jobtitle). - * + * Job-related data is also available. To generate a job title, use [`jobTitle`](https://next.fakerjs.dev/api/person.html#jobtitle). + * * This module can also generate other personal information which might appear in user profiles, such as [`gender`](https://next.fakerjs.dev/api/person.html#gender), [`zodiacSign`](https://next.fakerjs.dev/api/person.html#zodiacsign), and [`bio`](https://next.fakerjs.dev/api/person.html#bio). - * + * * ### Related modules * * For personal contact information like phone numbers and email addresses, see the [`faker.phone`](https://next.fakerjs.dev/api/phone.html) and [`faker.internet`](https://next.fakerjs.dev/api/internet.html) modules. - */ export class PersonModule { constructor(private readonly faker: Faker) { @@ -102,7 +101,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 +131,7 @@ export class PersonModule { last_name_patterns, male_last_name_patterns, female_last_name_patterns, - } = this.faker.definitions.person; + } = this.faker.rawDefinitions.person ?? {}; if ( last_name_patterns != null || @@ -179,7 +178,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, @@ -320,7 +319,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/__snapshots__/location.spec.ts.snap b/test/__snapshots__/location.spec.ts.snap index d263703c76d..0a65cd8231a 100644 --- a/test/__snapshots__/location.spec.ts.snap +++ b/test/__snapshots__/location.spec.ts.snap @@ -140,8 +140,6 @@ exports[`location > 42 > streetAddress > with boolean 1`] = `"7917 Miller Park"` exports[`location > 42 > streetAddress > with useFullAddress options 1`] = `"7917 Miller Park Apt. 410"`; -exports[`location > 42 > streetName 1`] = `"b"`; - exports[`location > 42 > timeZone 1`] = `"America/North_Dakota/New_Salem"`; exports[`location > 42 > zipCode > noArgs 1`] = `"79177"`; @@ -298,8 +296,6 @@ exports[`location > 1211 > streetAddress > with boolean 1`] = `"487 Breana Wells exports[`location > 1211 > streetAddress > with useFullAddress options 1`] = `"487 Breana Wells Apt. 616"`; -exports[`location > 1211 > streetName 1`] = `"c"`; - exports[`location > 1211 > timeZone 1`] = `"Pacific/Fiji"`; exports[`location > 1211 > zipCode > noArgs 1`] = `"48721-9061"`; @@ -456,8 +452,6 @@ exports[`location > 1337 > streetAddress > with boolean 1`] = `"51225 Alexys Gat exports[`location > 1337 > streetAddress > with useFullAddress options 1`] = `"51225 Alexys Gateway Apt. 552"`; -exports[`location > 1337 > streetName 1`] = `"a"`; - exports[`location > 1337 > timeZone 1`] = `"America/Guatemala"`; exports[`location > 1337 > zipCode > noArgs 1`] = `"51225"`; diff --git a/test/all_functional.spec.ts b/test/all_functional.spec.ts index 872629eaa1b..7e93c185d52 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', @@ -28,6 +29,48 @@ const BROKEN_LOCALE_METHODS = { companySuffix: ['az'], }, location: { + streetName: [ + 'af_ZA', + 'ar', + 'dv', + 'el', + 'en', + 'en_AU', + 'en_BORK', + 'en_CA', + 'en_GB', + 'en_GH', + 'en_IE', + 'en_IN', + 'en_NG', + 'en_US', + 'en_ZA', + 'es', + 'fa', + 'fi', + 'fr', + 'fr_BE', + 'fr_CA', + 'fr_CH', + 'fr_LU', + 'hu', + 'hy', + 'id_ID', + 'it', + 'ja', + 'ne', + 'nl', + 'nl_BE', + 'pl', + 'pt_BR', + 'pt_PT', + 'ur', + 'vi', + 'zh_CN', + 'zh_TW', + 'zu_ZA', + ], + streetAddress: ['fr_CH'], state: ['az', 'nb_NO', 'sk'], stateAbbr: ['sk'], }, diff --git a/test/faker.spec.ts b/test/faker.spec.ts index e2031e4ecb5..65b2d117ad5 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -30,6 +30,19 @@ describe('faker', () => { } }); + describe('rawDefinitions', () => { + it('locale definition accessability', () => { + // Metadata + expect(faker.rawDefinitions.metadata.title).toBeDefined(); + // Standard modules + expect(faker.rawDefinitions.location?.city_name).toBeDefined(); + // Custom modules + expect(faker.rawDefinitions.business?.credit_card_types).toBeDefined(); + expect(faker.rawDefinitions.missing).toBeUndefined(); + expect(faker.rawDefinitions.business?.missing).toBeUndefined(); + }); + }); + describe('definitions', () => { it('locale definition accessability', () => { // Metadata @@ -38,8 +51,8 @@ describe('faker', () => { expect(faker.definitions.location.city_name).toBeDefined(); // Custom modules expect(faker.definitions.business.credit_card_types).toBeDefined(); - expect(faker.definitions.missing).toBeUndefined(); - expect(faker.definitions.business.missing).toBeUndefined(); + expect(faker.definitions.missing).toBeDefined(); + expect(() => faker.definitions.business.missing).toThrow(); }); }); diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts new file mode 100644 index 00000000000..5814fecd417 --- /dev/null +++ b/test/locale-proxy.spec.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest'; +import type { AirlineDefinitions, MetadataDefinitions } from '../src'; +import { en, FakerError } from '../src'; +import { createLocaleAccess } from '../src/locale-proxy'; + +describe('LocaleAccess', () => { + const locale = createLocaleAccess(en); + const unavailable = createLocaleAccess({ + metadata: {} as MetadataDefinitions, + airline: { airline: [] } as AirlineDefinitions, + }); + + 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(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.category = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to replace a category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + 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', () => { + 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(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.category.missing = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to add a new entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline.missing = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to replace an entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline.airline = []; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); +}); diff --git a/test/location.spec.ts b/test/location.spec.ts index 3c41d351278..058c729e432 100644 --- a/test/location.spec.ts +++ b/test/location.spec.ts @@ -39,7 +39,10 @@ const NON_SEEDED_BASED_RUN = 5; describe('location', () => { seededTests(faker, 'location', (t) => { - t.itEach('street', 'streetName'); + t.it('street'); + + // EN does not have streetName data + t.skip('streetName'); t.it('buildingNumber'); diff --git a/test/person.spec.ts b/test/person.spec.ts index beca664386a..5520a38caa4 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, + ...(fakerMK.rawDefinitions.person?.female_prefix ?? []), + ...(fakerMK.rawDefinitions.person?.female_first_name ?? []), + ...(fakerMK.rawDefinitions.person?.female_last_name ?? []), + ...(fakerMK.rawDefinitions.person?.suffix ?? []), ]; 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, + ...(fakerMK.rawDefinitions.person?.male_prefix ?? []), + ...(fakerMK.rawDefinitions.person?.male_first_name ?? []), + ...(fakerMK.rawDefinitions.person?.male_last_name ?? []), + ...(fakerMK.rawDefinitions.person?.suffix ?? []), ]; 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, + ...(fakerMK.rawDefinitions.person?.suffix ?? []), ]; 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, + ...(fakerMK.rawDefinitions.person?.suffix ?? []), ]; const fullName = fakerMK.person.fullName({ From 9db8ff9e20c81553c6472e45991bf93697545999 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 12:06:22 +0200 Subject: [PATCH 02/29] chore: fix typo --- test/faker.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/faker.spec.ts b/test/faker.spec.ts index 65b2d117ad5..c0530db5f3c 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -1,6 +1,6 @@ import type { SpyInstance } from 'vitest'; import { describe, expect, it, vi } from 'vitest'; -import { faker, Faker } from '../src'; +import { Faker, faker } from '../src'; import { FakerError } from '../src/errors/faker-error'; describe('faker', () => { @@ -31,7 +31,7 @@ describe('faker', () => { }); describe('rawDefinitions', () => { - it('locale definition accessability', () => { + it('locale rawDefinition accessibility', () => { // Metadata expect(faker.rawDefinitions.metadata.title).toBeDefined(); // Standard modules @@ -44,7 +44,7 @@ describe('faker', () => { }); describe('definitions', () => { - it('locale definition accessability', () => { + it('locale definition accessibility', () => { // Metadata expect(faker.definitions.metadata.title).toBeDefined(); // Standard modules From b4959654f8acb102e29ca2250150fd9ecf30e4a1 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 12:18:52 +0200 Subject: [PATCH 03/29] test: extend proxy tests --- test/locale-proxy.spec.ts | 66 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 5814fecd417..ed14210836c 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { AirlineDefinitions, MetadataDefinitions } from '../src'; -import { en, FakerError } from '../src'; +import { FakerError, en } from '../src'; import { createLocaleAccess } from '../src/locale-proxy'; describe('LocaleAccess', () => { @@ -10,6 +10,16 @@ describe('LocaleAccess', () => { airline: { airline: [] } as AirlineDefinitions, }); + // Category-Level checks + + 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'); }); @@ -34,6 +44,36 @@ describe('LocaleAccess', () => { }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); + it('should not be possible to delete a missing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.category; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + // Entry-Level checks + + 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( @@ -90,4 +130,28 @@ describe('LocaleAccess', () => { locale.airline.airline = []; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); + + it('should not be possible to delete a missing entry in a missing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.category.missing; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete a missing entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline.missing; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete an existing entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline.airline; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); }); From c0be7e16ea52a89a24f5857ff4629e6ed26464bb Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 12:24:16 +0200 Subject: [PATCH 04/29] chore: fix import order --- test/locale-proxy.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index ed14210836c..22fa225c9c1 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { AirlineDefinitions, MetadataDefinitions } from '../src'; -import { FakerError, en } from '../src'; +import { en, FakerError } from '../src'; import { createLocaleAccess } from '../src/locale-proxy'; describe('LocaleAccess', () => { From 109ef31da469189b123c6131bb5b9d0b3c379e8d Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 12:26:07 +0200 Subject: [PATCH 05/29] test: use describe instead of comment sections --- test/locale-proxy.spec.ts | 276 +++++++++++++++++++------------------- 1 file changed, 138 insertions(+), 138 deletions(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 22fa225c9c1..d9fb1b7be38 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -10,148 +10,148 @@ describe('LocaleAccess', () => { airline: { airline: [] } as AirlineDefinitions, }); - // Category-Level checks - - 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(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - locale.category = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to replace a category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - locale.airline = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to delete a missing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete locale.category; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to delete an existing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete locale.airline; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - // Entry-Level checks - - 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. + 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(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.category = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to replace a category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete a missing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.category; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + }); + + 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. + ) + ); + }); + + 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', () => { - expect(() => unavailable.airline.airline).toThrowError( - new FakerError( - `The locale data for 'airline.airline' aren't applicable to this locale. + ) + ); + }); + + 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', () => { + 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(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - locale.category.missing = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to add a new entry in an existing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - locale.airline.missing = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to replace an entry in an existing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - locale.airline.airline = []; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to delete a missing entry in a missing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete locale.category.missing; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to delete a missing entry in an existing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete locale.airline.missing; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); - }); - - it('should not be possible to delete an existing entry in an existing category', () => { - expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete locale.airline.airline; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + ) + ); + }); + + it('should not be possible to add a new entry in a missing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.category.missing = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to add a new entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline.missing = {}; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to replace an entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + locale.airline.airline = []; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete a missing entry in a missing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.category.missing; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete a missing entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline.missing; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); + + it('should not be possible to delete an existing entry in an existing category', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete locale.airline.airline; + }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }); }); }); From 0f42fda250f0f5035cee467f9134f955de122200 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 13:51:36 +0200 Subject: [PATCH 06/29] chore: apply suggestions --- test/locale-proxy.spec.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index d9fb1b7be38..61e061b6e6c 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -30,7 +30,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error locale.category = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -38,7 +38,7 @@ describe('LocaleAccess', () => { it('should not be possible to replace a category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error locale.airline = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -46,7 +46,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error delete locale.category; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -54,7 +54,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete an existing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error delete locale.airline; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -109,7 +109,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new entry in a missing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error locale.category.missing = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -117,7 +117,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new entry in an existing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error locale.airline.missing = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -125,7 +125,7 @@ describe('LocaleAccess', () => { it('should not be possible to replace an entry in an existing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error locale.airline.airline = []; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -133,7 +133,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing entry in a missing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error delete locale.category.missing; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -141,7 +141,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing entry in an existing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error delete locale.airline.missing; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -149,7 +149,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete an existing entry in an existing category', () => { expect(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error delete locale.airline.airline; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); From eebccd053389c0b27ce23dd7210a34e3c611065e Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:17:02 +0200 Subject: [PATCH 07/29] docs: extend migration guide --- docs/guide/upgrading.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 1987ed0e0bf..086ea1360b8 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -98,6 +98,15 @@ 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` + +Previously, using `faker.definitions.animal.cat` returned `undefined`, when the data were accessed but missing in that locale, thus `faker.animal.cat()` returned one of `a`-`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 addressed that now by changing the implementation to throwing an error, requesting you to add the missing data instead. +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. + ### `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. From 5c70e3288c4d796fba44b865d3707ff2b0f2f6cc Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:21:38 +0200 Subject: [PATCH 08/29] chore: typo --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 086ea1360b8..ca5cf6de8a0 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -103,7 +103,7 @@ For more information refer to our [Localization Guide](localization). Previously, using `faker.definitions.animal.cat` returned `undefined`, when the data were accessed but missing in that locale, thus `faker.animal.cat()` returned one of `a`-`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 addressed that now by changing the implementation to throwing an error, requesting you to add the missing data instead. +We addressed that now by changing the implementation to throw an error, requesting you to add the missing data instead. 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. From 5c737253ab742e74d1740b576ced3b11e221b788 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:24:59 +0200 Subject: [PATCH 09/29] chore: reword --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index ca5cf6de8a0..83a3f7f9cd4 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -103,7 +103,7 @@ For more information refer to our [Localization Guide](localization). Previously, using `faker.definitions.animal.cat` returned `undefined`, when the data were accessed but missing in that locale, thus `faker.animal.cat()` returned one of `a`-`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 addressed that now by changing the implementation to throw an error, requesting you to add the missing data instead. +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. From df38ac7871dba5b3efe34742278ef91d1cb180a3 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:26:09 +0200 Subject: [PATCH 10/29] chore: mark string type --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 83a3f7f9cd4..3f8f3a63e97 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -101,7 +101,7 @@ For more information refer to our [Localization Guide](localization). ### For missing locale data, Faker will now throw instead of returning `undefined` or `a`-`c` Previously, using `faker.definitions.animal.cat` returned `undefined`, when the data were accessed but missing in that locale, thus `faker.animal.cat()` returned one of `a`-`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). +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. From d8fa41fa51dbd73cdf251e532ff6eeffb9757e6d Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:36:07 +0200 Subject: [PATCH 11/29] docs: add examples --- docs/guide/upgrading.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 3f8f3a63e97..b41b9a92ee0 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -107,6 +107,35 @@ We have now addressed this by changing the implementation so that an error is th 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.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. From 05d2cab0159de6f8bc64e6a4539bc19981eecd85 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 14:48:23 +0200 Subject: [PATCH 12/29] docs: improve example --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index b41b9a92ee0..db9e805a4d9 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -131,7 +131,7 @@ const fakerZH_CN_noFallbacks = new Faker({ faker.name.prefix(); // 'Mr' // Previously: -//fakerZH_CN.person.prefix(); // undefined +//fakerZH_CN_noFallbacks.person.prefix(); // undefined // Now: fakerZH_CN.person.prefix(); // throws a FakerError ``` From 79cb31a2e051d39808b77a37586a9f3f920930c5 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 15:04:54 +0200 Subject: [PATCH 13/29] docs: apply suggestions --- docs/guide/upgrading.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index db9e805a4d9..4933562a19b 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -100,7 +100,11 @@ For more information refer to our [Localization Guide](localization). ### For missing locale data, Faker will now throw instead of returning `undefined` or `a`-`c` -Previously, using `faker.definitions.animal.cat` returned `undefined`, when the data were accessed but missing in that locale, thus `faker.animal.cat()` returned one of `a`-`c` (`arrayElement`'s default value). +::: note Hinweis +The following section mostly applies to custom-built Faker instances. +::: + +Previously, for example when `en` doesn't have data for `animal.cat` then `faker.animal.cat()` would have returned one of `a`-`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. From 885627b7cd81d386efb5a9bd3ba80f3feb2438a9 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 15:05:43 +0200 Subject: [PATCH 14/29] chore: fix typo --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 4933562a19b..6374ef45250 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -100,7 +100,7 @@ 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 Hinweis +::: note Note The following section mostly applies to custom-built Faker instances. ::: From 0ff62afbb4e5d5b664987bb2e09f2c6148c75f26 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 2 Apr 2023 19:54:37 +0200 Subject: [PATCH 15/29] chore: apply suggestion --- docs/guide/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 6374ef45250..8fcba300c0a 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -104,7 +104,7 @@ For more information refer to our [Localization Guide](localization). The following section mostly applies to custom-built Faker instances. ::: -Previously, for example when `en` doesn't have data for `animal.cat` then `faker.animal.cat()` would have returned one of `a`-`c` (`arrayElement`'s default value). +Previously, for example if `en` didn't have data for `animal.cat`, then `faker.animal.cat()` would have returned one of `a`-`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. From e0b8ac2e3db7790ebc5834530c596b1b4c2f4505 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Tue, 4 Apr 2023 09:05:33 +0200 Subject: [PATCH 16/29] test: remove expected failure --- test/all_functional.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/all_functional.spec.ts b/test/all_functional.spec.ts index 7e93c185d52..819e38d24b4 100644 --- a/test/all_functional.spec.ts +++ b/test/all_functional.spec.ts @@ -70,7 +70,6 @@ const BROKEN_LOCALE_METHODS = { 'zh_TW', 'zu_ZA', ], - streetAddress: ['fr_CH'], state: ['az', 'nb_NO', 'sk'], stateAbbr: ['sk'], }, From 021e851f78aa6ee7a8a1ac6ea0784b2ac8e5771b Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Wed, 12 Apr 2023 23:31:41 +0200 Subject: [PATCH 17/29] chore: apply suggestions Co-authored-by: Shinigami --- docs/guide/upgrading.md | 2 +- src/locale-proxy.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 5d79af48c8e..42a2b96586a 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -103,7 +103,7 @@ For more information refer to our [Localization Guide](localization). 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`-`c` (`arrayElement`'s default value). +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. diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index ea82597591c..0a8d30f1198 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -11,7 +11,7 @@ export type LocaleAccess = Readonly<{ }>; /** - * Creates a proxy for LocaleDefinition that throws an error when a property is accessed that is not defined. + * 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. */ @@ -50,7 +50,7 @@ export function createLocaleAccess(locale: LocaleDefinition): LocaleAccess { } /** - * Creates a proxy for a category that throws an error when a property is accessed that is not defined. + * 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. From f69b47464f784b9a657af9d0358fbeeeeeb9d3e3 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Wed, 12 Apr 2023 23:33:59 +0200 Subject: [PATCH 18/29] chore: lost commit --- test/faker.spec.ts | 2 +- test/locale-proxy.spec.ts | 30 ++++++++++-------------------- test/location.spec.ts | 10 ++++++++++ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/test/faker.spec.ts b/test/faker.spec.ts index c189a7ea812..a35c942a33a 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -39,7 +39,7 @@ describe('faker', () => { // Non-existing module expect(faker.rawDefinitions.missing).toBeUndefined(); // Non-existing definition in a non-existing module - expect(faker.definitions.missing?.missing).toBeUndefined(); + expect(faker.rawDefinitions.missing?.missing).toBeUndefined(); // Non-existing definition in an existing module expect(faker.rawDefinitions.location?.missing).toBeUndefined(); }); diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 61e061b6e6c..eb1142ebea6 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -29,32 +29,28 @@ describe('LocaleAccess', () => { it('should not be possible to add a new category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. locale.category = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to replace a category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. locale.airline = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to delete a missing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. delete locale.category; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to delete an existing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. delete locale.airline; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); @@ -108,48 +104,42 @@ describe('LocaleAccess', () => { it('should not be possible to add a new entry in a missing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. locale.category.missing = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to add a new entry in an existing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. locale.airline.missing = {}; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to replace an entry in an existing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. locale.airline.airline = []; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to delete a missing entry in a missing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. delete locale.category.missing; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to delete a missing entry in an existing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. delete locale.airline.missing; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); it('should not be possible to delete an existing entry in an existing category', () => { expect(() => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error LocaleAccess is read-only. delete locale.airline.airline; }).toThrowError(new FakerError('LocaleAccess is read-only.')); }); diff --git a/test/location.spec.ts b/test/location.spec.ts index 453c83241ea..5e342ce6d2b 100644 --- a/test/location.spec.ts +++ b/test/location.spec.ts @@ -176,6 +176,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"') ); }); From 2a4cc3ffefb6c711027cb0d14b25994c223d5a7c Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 15 Apr 2023 19:50:20 +0200 Subject: [PATCH 19/29] chore: apply suggestions --- src/faker.ts | 8 +++---- src/locale-proxy.ts | 44 ++++++++++++++++++--------------------- test/locale-proxy.spec.ts | 15 ++++++------- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/faker.ts b/src/faker.ts index e64e99573b8..fd2e90be9fd 100644 --- a/src/faker.ts +++ b/src/faker.ts @@ -3,8 +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 { LocaleAccess } from './locale-proxy'; -import { createLocaleAccess } from './locale-proxy'; +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'; @@ -63,7 +63,7 @@ import { mergeLocales } from './utils/merge-locales'; */ export class Faker { readonly rawDefinitions: LocaleDefinition; - readonly definitions: LocaleAccess; + readonly definitions: LocaleProxy; private _defaultRefDate: () => Date = () => new Date(); /** @@ -334,7 +334,7 @@ export class Faker { } this.rawDefinitions = locale as LocaleDefinition; - this.definitions = createLocaleAccess(this.rawDefinitions); + this.definitions = createLocaleProxy(this.rawDefinitions); } /** diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index 0a8d30f1198..8d3548fbcb4 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -4,18 +4,22 @@ 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 LocaleAccess = Readonly<{ +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 createLocaleAccess(locale: LocaleDefinition): LocaleAccess { +export function createLocaleProxy(locale: LocaleDefinition): LocaleProxy { return new Proxy({} as LocaleDefinition, { has(): true { return true; @@ -35,18 +39,13 @@ export function createLocaleAccess(locale: LocaleDefinition): LocaleAccess { )); }, - set(): never { - throw new FakerError('LocaleAccess is read-only.'); - }, - - deleteProperty(): never { - throw new FakerError('LocaleAccess is read-only.'); - }, + set: throwReadOnlyError, + deleteProperty: throwReadOnlyError, - ownKeys(): Array { + ownKeys(): Array { return Object.keys(locale); }, - }) as LocaleAccess; + }) as LocaleProxy; } /** @@ -55,17 +54,19 @@ export function createLocaleAccess(locale: LocaleDefinition): LocaleAccess { * @param categoryName The name of the category. * @param categoryData The module to create the proxy for. */ -function createCategoryProxy>( +function createCategoryProxy< + CategoryData extends Record +>( categoryName: string, - categoryData: T = {} as T -): Required { - return new Proxy({} as Required, { - has(_, entryName: keyof T): boolean { + categoryData: CategoryData = {} as CategoryData +): Required { + return new Proxy({} as Required, { + has(_, entryName: keyof CategoryData): boolean { const value = categoryData[entryName]; return value != null && (!Array.isArray(value) || value.length !== 0); }, - get(_, entryName: keyof T): T[keyof T] { + get(_, entryName: keyof CategoryData): CategoryData[keyof CategoryData] { const value = categoryData[entryName]; if (value == null) { throw new FakerError( @@ -83,13 +84,8 @@ function createCategoryProxy>( } }, - set(): never { - throw new FakerError('LocaleAccess is read-only.'); - }, - - deleteProperty(): never { - throw new FakerError('LocaleAccess is read-only.'); - }, + set: throwReadOnlyError, + deleteProperty: throwReadOnlyError, ownKeys(): Array { return Object.keys(categoryData); diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index eb1142ebea6..77b98f01b59 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -1,14 +1,10 @@ import { describe, expect, it } from 'vitest'; -import type { AirlineDefinitions, MetadataDefinitions } from '../src'; +import type { MetadataDefinitions } from '../src'; import { en, FakerError } from '../src'; -import { createLocaleAccess } from '../src/locale-proxy'; +import { createLocaleProxy } from '../src/locale-proxy'; describe('LocaleAccess', () => { - const locale = createLocaleAccess(en); - const unavailable = createLocaleAccess({ - metadata: {} as MetadataDefinitions, - airline: { airline: [] } as AirlineDefinitions, - }); + const locale = createLocaleProxy(en); describe('category', () => { it('should be possible to check for a missing category', () => { @@ -94,6 +90,11 @@ describe('LocaleAccess', () => { }); it('should not be possible to access an unavailable entry in a present category', () => { + const unavailable = createLocaleProxy({ + metadata: {} as MetadataDefinitions, + airline: { airline: [] }, + }); + expect(() => unavailable.airline.airline).toThrowError( new FakerError( `The locale data for 'airline.airline' aren't applicable to this locale. From 372e1f2f1b8656a59f2aae55c23fe8cbe75f3bcc Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 15 Apr 2023 19:57:03 +0200 Subject: [PATCH 20/29] chore: apply suggestion --- src/locale-proxy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index 8d3548fbcb4..167f7d65a96 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -22,6 +22,7 @@ const throwReadOnlyError: () => never = () => { export function createLocaleProxy(locale: LocaleDefinition): LocaleProxy { return new Proxy({} as LocaleDefinition, { has(): true { + // Categories are always present (proxied), that why we return true. return true; }, From fd80d4fda7c862c256ddca37b92768ebc1d5be14 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 15 Apr 2023 20:08:58 +0200 Subject: [PATCH 21/29] test: update error message --- test/locale-proxy.spec.ts | 40 +++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 77b98f01b59..0be0f6e682d 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -27,28 +27,36 @@ describe('LocaleAccess', () => { expect(() => { // @ts-expect-error LocaleAccess is read-only. locale.category = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. locale.airline = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. delete locale.category; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. delete locale.airline; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); }); }); @@ -107,42 +115,54 @@ describe('LocaleAccess', () => { expect(() => { // @ts-expect-error LocaleAccess is read-only. locale.category.missing = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. locale.airline.missing = {}; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. locale.airline.airline = []; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. delete locale.category.missing; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. delete locale.airline.missing; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).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 LocaleAccess is read-only. delete locale.airline.airline; - }).toThrowError(new FakerError('LocaleAccess is read-only.')); + }).toThrowError( + new FakerError('You cannot edit the locale data on the faker instance') + ); }); }); }); From 4b9207106ff6c633d87c6773b4816c387f9a5d10 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 15 Apr 2023 20:37:56 +0200 Subject: [PATCH 22/29] Update test/faker.spec.ts --- test/faker.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/faker.spec.ts b/test/faker.spec.ts index a35c942a33a..0e02d67dff8 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -1,6 +1,6 @@ import type { SpyInstance } from 'vitest'; import { describe, expect, it, vi } from 'vitest'; -import { Faker, faker } from '../src'; +import { faker, Faker} from '../src'; import { FakerError } from '../src/errors/faker-error'; describe('faker', () => { From 385d4fae52956778b9d856ef2a6f39a779a7d07f Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 15 Apr 2023 20:44:02 +0200 Subject: [PATCH 23/29] chore: format --- test/faker.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/faker.spec.ts b/test/faker.spec.ts index 0e02d67dff8..207dac4a7a3 100644 --- a/test/faker.spec.ts +++ b/test/faker.spec.ts @@ -1,6 +1,6 @@ import type { SpyInstance } from 'vitest'; import { describe, expect, it, vi } from 'vitest'; -import { faker, Faker} from '../src'; +import { faker, Faker } from '../src'; import { FakerError } from '../src/errors/faker-error'; describe('faker', () => { From 9b298c72813296be16ba56f9b6d00e5d12e6f0fb Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Mon, 17 Apr 2023 11:44:32 +0200 Subject: [PATCH 24/29] test: add tests for Object.keys --- test/locale-proxy.spec.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 0be0f6e682d..ae44a78c73d 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -5,6 +5,7 @@ import { createLocaleProxy } from '../src/locale-proxy'; describe('LocaleAccess', () => { const locale = createLocaleProxy(en); + const enAirline = en.airline ?? { never: 'missing' }; describe('category', () => { it('should be possible to check for a missing category', () => { @@ -58,6 +59,16 @@ describe('LocaleAccess', () => { 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([]); + }); + + it('should be possible to get all categories keys on actual locale', () => { + expect(Object.keys(locale).sort()).toEqual(Object.keys(en).sort()); + }); }); describe('entry', () => { @@ -164,5 +175,15 @@ describe('LocaleAccess', () => { 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() + ); + }); }); }); From f99f72c7e6f86ffde88806a5366461d1dc63f636 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Thu, 20 Apr 2023 22:26:20 +0200 Subject: [PATCH 25/29] fix: proxy-impl --- src/locale-proxy.ts | 36 ++++++++++++++++-------------------- test/locale-proxy.spec.ts | 2 +- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index 167f7d65a96..32fe30e34dd 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -20,32 +20,29 @@ const throwReadOnlyError: () => never = () => { * @param locale The locale definition to create the proxy for. */ export function createLocaleProxy(locale: LocaleDefinition): LocaleProxy { - return new Proxy({} as LocaleDefinition, { + const proxies = {} as LocaleDefinition; + return new Proxy(locale, { has(): true { // Categories are always present (proxied), that why we return true. return true; }, get( - target, + target: LocaleDefinition, categoryName: keyof LocaleDefinition ): LocaleDefinition[keyof LocaleDefinition] { - if (categoryName in target) { - return target[categoryName]; + if (categoryName in proxies) { + return proxies[categoryName]; } - return (target[categoryName] = createCategoryProxy( + return (proxies[categoryName] = createCategoryProxy( categoryName, - locale[categoryName] + target[categoryName] )); }, set: throwReadOnlyError, deleteProperty: throwReadOnlyError, - - ownKeys(): Array { - return Object.keys(locale); - }, }) as LocaleProxy; } @@ -61,14 +58,17 @@ function createCategoryProxy< categoryName: string, categoryData: CategoryData = {} as CategoryData ): Required { - return new Proxy({} as Required, { - has(_, entryName: keyof CategoryData): boolean { - const value = categoryData[entryName]; + return new Proxy(categoryData, { + has(target: CategoryData, entryName: keyof CategoryData): boolean { + const value = target[entryName]; return value != null && (!Array.isArray(value) || value.length !== 0); }, - get(_, entryName: keyof CategoryData): CategoryData[keyof CategoryData] { - const value = categoryData[entryName]; + 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()}' are missing in this locale. @@ -87,9 +87,5 @@ function createCategoryProxy< set: throwReadOnlyError, deleteProperty: throwReadOnlyError, - - ownKeys(): Array { - return Object.keys(categoryData); - }, - }); + }) as Required; } diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index ae44a78c73d..5b2494fe371 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -63,7 +63,7 @@ describe('LocaleAccess', () => { it('should be possible to get all categories keys on empty locale', () => { const empty = createLocaleProxy({ metadata: {} as MetadataDefinitions }); - expect(Object.keys(empty)).toEqual([]); + expect(Object.keys(empty)).toEqual(['metadata']); }); it('should be possible to get all categories keys on actual locale', () => { From 106bf7001252ec60c1c2523390e8ed6bb942d732 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Thu, 20 Apr 2023 22:32:52 +0200 Subject: [PATCH 26/29] Apply suggestions from code review --- src/locale-proxy.ts | 2 +- test/locale-proxy.spec.ts | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index 32fe30e34dd..ed8f394dab7 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -23,7 +23,7 @@ export function createLocaleProxy(locale: LocaleDefinition): LocaleProxy { const proxies = {} as LocaleDefinition; return new Proxy(locale, { has(): true { - // Categories are always present (proxied), that why we return true. + // Categories are always present (proxied), that's why we return true. return true; }, diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 5b2494fe371..56cada86e99 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -26,7 +26,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. locale.category = {}; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -35,7 +35,7 @@ describe('LocaleAccess', () => { it('should not be possible to replace a category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. locale.airline = {}; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -44,7 +44,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. delete locale.category; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -53,7 +53,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete an existing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. delete locale.airline; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -124,7 +124,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new entry in a missing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. locale.category.missing = {}; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -133,7 +133,7 @@ describe('LocaleAccess', () => { it('should not be possible to add a new entry in an existing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. locale.airline.missing = {}; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -142,7 +142,7 @@ describe('LocaleAccess', () => { it('should not be possible to replace an entry in an existing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. locale.airline.airline = []; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -151,7 +151,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing entry in a missing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. delete locale.category.missing; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -160,7 +160,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete a missing entry in an existing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. delete locale.airline.missing; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') @@ -169,7 +169,7 @@ describe('LocaleAccess', () => { it('should not be possible to delete an existing entry in an existing category', () => { expect(() => { - // @ts-expect-error LocaleAccess is read-only. + // @ts-expect-error: LocaleProxy is read-only. delete locale.airline.airline; }).toThrowError( new FakerError('You cannot edit the locale data on the faker instance') From 24e14cfd17ad19d99c32fa7ce608105327aa7567 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Fri, 21 Apr 2023 23:06:12 +0200 Subject: [PATCH 27/29] refactor: use null as not applicable data --- src/locale-proxy.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index ed8f394dab7..ce9fdd6e5bd 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -61,7 +61,7 @@ function createCategoryProxy< return new Proxy(categoryData, { has(target: CategoryData, entryName: keyof CategoryData): boolean { const value = target[entryName]; - return value != null && (!Array.isArray(value) || value.length !== 0); + return value != null; }, get( @@ -69,17 +69,17 @@ function createCategoryProxy< entryName: keyof CategoryData ): CategoryData[keyof CategoryData] { const value = target[entryName]; - if (value == null) { + 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 == null) { 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 if (Array.isArray(value) && value.length === 0) { - 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 { return value; } From 2ab602313b7fd0a6e3f9c6d0b8bf80e34d4c7de9 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 23 Apr 2023 13:10:20 +0200 Subject: [PATCH 28/29] chore: own review --- test/locale-proxy.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/locale-proxy.spec.ts b/test/locale-proxy.spec.ts index 37f55f979d2..88ce9555645 100644 --- a/test/locale-proxy.spec.ts +++ b/test/locale-proxy.spec.ts @@ -3,7 +3,7 @@ import type { MetadataDefinitions } from '../src'; import { en, FakerError } from '../src'; import { createLocaleProxy } from '../src/locale-proxy'; -describe('LocaleAccess', () => { +describe('LocaleProxy', () => { const locale = createLocaleProxy(en); const enAirline = en.airline ?? { never: 'missing' }; From 783b7bac22d8dfa2627f2da2ad40e017d3241b61 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sun, 23 Apr 2023 13:21:34 +0200 Subject: [PATCH 29/29] Update src/locale-proxy.ts --- src/locale-proxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/locale-proxy.ts b/src/locale-proxy.ts index ec641c981ca..c5c6aa1b733 100644 --- a/src/locale-proxy.ts +++ b/src/locale-proxy.ts @@ -74,7 +74,7 @@ function createCategoryProxy< `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 == null) { + } 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.