From 4645f8f14066b15480043a3f36821f72af5bfcb0 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Fri, 14 Apr 2023 00:30:02 +0200 Subject: [PATCH 01/21] refactor(helpers): remove default value from arrayElement --- src/modules/helpers/index.ts | 16 ++++----- test/__snapshots__/helpers.spec.ts.snap | 6 ---- test/__snapshots__/location.spec.ts.snap | 6 ---- test/all_functional.spec.ts | 46 ++++++++++++++++++++++++ test/helpers.spec.ts | 23 ++++++++---- test/location.spec.ts | 5 ++- 6 files changed, 72 insertions(+), 30 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index a5805af8030..2e4828558c0 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -793,15 +793,15 @@ export class HelpersModule { * * @since 6.3.0 */ - arrayElement( - // TODO @Shinigami92 2022-04-30: We want to remove this default value, but currently it's not possible because some definitions could be empty - // See https://github.com/faker-js/faker/issues/893 - array: ReadonlyArray = ['a', 'b', 'c'] as unknown as ReadonlyArray - ): T { + arrayElement(array: ReadonlyArray): T { const index = array.length > 1 ? this.faker.number.int({ max: array.length - 1 }) : 0; + const value = array[index]; + if (value === undefined) { + throw new FakerError('Cannot get value from empty set.'); + } - return array[index]; + return value; } /** @@ -1106,10 +1106,6 @@ export class HelpersModule { fake(pattern: string | string[]): string { if (Array.isArray(pattern)) { pattern = this.arrayElement(pattern); - // TODO @ST-DDT 2022-10-15: Remove this check after we fail in `arrayElement` when the array is empty - if (pattern == null) { - throw new FakerError('Array of pattern strings cannot be empty.'); - } } // find first matching {{ and }} diff --git a/test/__snapshots__/helpers.spec.ts.snap b/test/__snapshots__/helpers.spec.ts.snap index 665e19a1269..19e52c4ac09 100644 --- a/test/__snapshots__/helpers.spec.ts.snap +++ b/test/__snapshots__/helpers.spec.ts.snap @@ -1,7 +1,5 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`helpers > 42 > arrayElement > noArgs 1`] = `"b"`; - exports[`helpers > 42 > arrayElement > with array 1`] = `"o"`; exports[`helpers > 42 > arrayElements > noArgs 1`] = ` @@ -226,8 +224,6 @@ exports[`helpers > 42 > weightedArrayElement > with array 1`] = `"sunny"`; exports[`helpers > 42 > weightedArrayElement > with array with percentages 1`] = `"sunny"`; -exports[`helpers > 1211 > arrayElement > noArgs 1`] = `"c"`; - exports[`helpers > 1211 > arrayElement > with array 1`] = `"!"`; exports[`helpers > 1211 > arrayElements > noArgs 1`] = ` @@ -469,8 +465,6 @@ exports[`helpers > 1211 > weightedArrayElement > with array 1`] = `"snowy"`; exports[`helpers > 1211 > weightedArrayElement > with array with percentages 1`] = `"snowy"`; -exports[`helpers > 1337 > arrayElement > noArgs 1`] = `"a"`; - exports[`helpers > 1337 > arrayElement > with array 1`] = `"l"`; exports[`helpers > 1337 > arrayElements > noArgs 1`] = ` diff --git a/test/__snapshots__/location.spec.ts.snap b/test/__snapshots__/location.spec.ts.snap index 1d09415c9b2..d32e86ed1ab 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"`; @@ -292,8 +290,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"`; @@ -444,8 +440,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..25355d62c65 100644 --- a/test/all_functional.spec.ts +++ b/test/all_functional.spec.ts @@ -28,8 +28,50 @@ const BROKEN_LOCALE_METHODS = { companySuffix: ['az'], }, location: { + city: ['th'], state: ['az', 'nb_NO', 'sk'], stateAbbr: ['sk'], + 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', + ], }, random: { locale: '*', // locale() has been pseudo removed @@ -40,6 +82,10 @@ const BROKEN_LOCALE_METHODS = { person: { prefix: ['az', 'id_ID', 'ru', 'zh_CN', 'zh_TW'], suffix: ['az', 'it', 'mk', 'pt_PT', 'ru'], + jobArea: ['ar', 'fr', 'fr_BE', 'fr_CA', 'fr_CH', 'fr_LU'], + jobDescriptor: ['ar', 'fr', 'fr_BE', 'fr_CA', 'fr_CH', 'fr_LU'], + jobTitle: ['ar', 'fr', 'fr_BE', 'fr_CA', 'fr_CH', 'fr_LU', 'ur'], + jobType: ['ur'], }, } satisfies { [module in keyof Faker]?: SkipConfig; diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 2c63595abf8..c48a1128f37 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -70,7 +70,7 @@ describe('helpers', () => { }); t.describe('arrayElement', (t) => { - t.it('noArgs').it('with array', 'Hello World!'.split('')); + t.it('with array', 'Hello World!'.split('')); }); t.describe('enumValue', (t) => { @@ -206,6 +206,13 @@ describe('helpers', () => { expect(actual).toBe('hello'); }); + + it('should throw on an empty array', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + expect(() => faker.helpers.arrayElement([])).toThrowError( + new FakerError('Cannot get value from empty set.') + ); + }); }); describe('enumValue', () => { @@ -867,9 +874,10 @@ describe('helpers', () => { expect(Object.keys(testObject)).toContain(actual); }); - it('should return undefined if given object is empty', () => { - const actual = faker.helpers.objectKey({}); - expect(actual).toBeUndefined(); + it('should throw if given object is empty', () => { + expect(() => faker.helpers.objectKey({})).toThrowError( + new FakerError('Cannot get value from empty set.') + ); }); }); @@ -886,8 +894,9 @@ describe('helpers', () => { }); it('should return undefined if given object is empty', () => { - const actual = faker.helpers.objectValue({}); - expect(actual).toBeUndefined(); + expect(() => faker.helpers.objectValue({})).toThrowError( + new FakerError('Cannot get value from empty set.') + ); }); }); @@ -946,7 +955,7 @@ describe('helpers', () => { it('does not allow empty array parameters', () => { expect(() => faker.helpers.fake([])).toThrowError( - new FakerError('Array of pattern strings cannot be empty.') + new FakerError('Cannot get value from empty set.') ); }); diff --git a/test/location.spec.ts b/test/location.spec.ts index 69af0a6d255..20597da650e 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'); + + // todo - missing locale data + t.skip('streetName'); t.it('buildingNumber'); From 80e2d59cdb3e83a43ae700f46eeddec26348f06f Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Fri, 14 Apr 2023 00:32:39 +0200 Subject: [PATCH 02/21] docs(helpers): add `@throw` to arrayElement --- src/modules/helpers/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 2e4828558c0..980837dd474 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -788,6 +788,8 @@ export class HelpersModule { * @template T The type of the entries to pick from. * @param array Array to pick the value from. * + * @throws If array is empty. + * * @example * faker.helpers.arrayElement(['cat', 'dog', 'mouse']) // 'dog' * From 99c16e40c88ae6788eb2bf942fd62c035cbc707e Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Fri, 14 Apr 2023 00:35:13 +0200 Subject: [PATCH 03/21] refactor(helpers): remove default value from arrayElements --- src/modules/helpers/index.ts | 4 +--- test/__snapshots__/helpers.spec.ts.snap | 21 --------------------- test/helpers.spec.ts | 3 +-- 3 files changed, 2 insertions(+), 26 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 980837dd474..9ec31f007d5 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -883,9 +883,7 @@ export class HelpersModule { * @since 6.3.0 */ arrayElements( - // TODO @Shinigami92 2022-04-30: We want to remove this default value, but currently it's not possible because some definitions could be empty - // See https://github.com/faker-js/faker/issues/893 - array: ReadonlyArray = ['a', 'b', 'c'] as unknown as ReadonlyArray, + array: ReadonlyArray, count?: | number | { diff --git a/test/__snapshots__/helpers.spec.ts.snap b/test/__snapshots__/helpers.spec.ts.snap index 19e52c4ac09..64562da72e0 100644 --- a/test/__snapshots__/helpers.spec.ts.snap +++ b/test/__snapshots__/helpers.spec.ts.snap @@ -2,13 +2,6 @@ exports[`helpers > 42 > arrayElement > with array 1`] = `"o"`; -exports[`helpers > 42 > arrayElements > noArgs 1`] = ` -[ - "b", - "c", -] -`; - exports[`helpers > 42 > arrayElements > with array 1`] = ` [ "r", @@ -226,14 +219,6 @@ exports[`helpers > 42 > weightedArrayElement > with array with percentages 1`] = exports[`helpers > 1211 > arrayElement > with array 1`] = `"!"`; -exports[`helpers > 1211 > arrayElements > noArgs 1`] = ` -[ - "a", - "c", - "b", -] -`; - exports[`helpers > 1211 > arrayElements > with array 1`] = ` [ "d", @@ -467,12 +452,6 @@ exports[`helpers > 1211 > weightedArrayElement > with array with percentages 1`] exports[`helpers > 1337 > arrayElement > with array 1`] = `"l"`; -exports[`helpers > 1337 > arrayElements > noArgs 1`] = ` -[ - "b", -] -`; - exports[`helpers > 1337 > arrayElements > with array 1`] = ` [ "l", diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index c48a1128f37..53686605bcd 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -119,8 +119,7 @@ describe('helpers', () => { }); t.describe('arrayElements', (t) => { - t.it('noArgs') - .it('with array', 'Hello World!'.split('')) + t.it('with array', 'Hello World!'.split('')) .it('with array and count', 'Hello World!'.split(''), 3) .it('with array and count range', 'Hello World!'.split(''), { min: 1, From c56014728ceb56a9c551c060cbef9dcb2bdcf0a2 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Fri, 14 Apr 2023 16:46:39 +0200 Subject: [PATCH 04/21] refactor(helpers): allow arrays with undefined values --- src/modules/helpers/index.ts | 10 +++++----- test/helpers.spec.ts | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 9ec31f007d5..b5c5915d7d8 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -796,14 +796,14 @@ export class HelpersModule { * @since 6.3.0 */ arrayElement(array: ReadonlyArray): T { - const index = - array.length > 1 ? this.faker.number.int({ max: array.length - 1 }) : 0; - const value = array[index]; - if (value === undefined) { + if (array.length === 0) { throw new FakerError('Cannot get value from empty set.'); } - return value; + const index = + array.length > 1 ? this.faker.number.int({ max: array.length - 1 }) : 0; + + return array[index]; } /** diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 53686605bcd..13bb286c887 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -212,6 +212,14 @@ describe('helpers', () => { new FakerError('Cannot get value from empty set.') ); }); + + describe('should not throw on an array with nullish elements', () => { + it.each(['', 0, undefined, null, false])('%s', (nullishValue) => { + expect(() => + faker.helpers.arrayElement([nullishValue]) + ).not.toThrowError(); + }); + }); }); describe('enumValue', () => { From cbd1731a588e35882b262fae95fa37a2ed6fc7e0 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Fri, 14 Apr 2023 16:58:08 +0200 Subject: [PATCH 05/21] docs: breaking array element behavior --- docs/guide/upgrading.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 7618ab8457f..db69e6ab27a 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -105,6 +105,17 @@ For more information refer to our [Localization Guide](localization). The `faker.location.zipCodeByState` method has been deprecated, but will also now throw an error if the current locale does not have a `postcode_by_state` definition. +### Methods will throw on empty data set inputs + +The methods `faker.helper.arrayElement` and `faker.helper.arrayElements` previously defaulted the `array` argument to a simply string array if non was provided. +This behavior is no longer supported. + +Additionally, by providing an empty array argument (`[]`) the functions previously returned `undefined`. +This behavior was undesired and has been removed. +The methods will now throw an `FakerError` instead. + +The same thing happens now if you provide an empty object `{}` to `faker.helpers.objectKey` or `faker.helpers.objectValue`. + ### Other deprecated methods removed/replaced | Old method | New method | From 944e73b85043a25451999bb6b0f779f6edd4c68f Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Sun, 16 Apr 2023 15:16:01 +0200 Subject: [PATCH 06/21] refactor(helpers): array element error --- src/modules/helpers/index.ts | 2 +- test/helpers.spec.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index b5c5915d7d8..e5d4b69c4e6 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -797,7 +797,7 @@ export class HelpersModule { */ arrayElement(array: ReadonlyArray): T { if (array.length === 0) { - throw new FakerError('Cannot get value from empty set.'); + throw new FakerError('Cannot get value from empty dataset.'); } const index = diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 13bb286c887..a32215af6aa 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -209,7 +209,7 @@ describe('helpers', () => { it('should throw on an empty array', () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-return expect(() => faker.helpers.arrayElement([])).toThrowError( - new FakerError('Cannot get value from empty set.') + new FakerError('Cannot get value from empty dataset.') ); }); @@ -883,7 +883,7 @@ describe('helpers', () => { it('should throw if given object is empty', () => { expect(() => faker.helpers.objectKey({})).toThrowError( - new FakerError('Cannot get value from empty set.') + new FakerError('Cannot get value from empty dataset.') ); }); }); @@ -902,7 +902,7 @@ describe('helpers', () => { it('should return undefined if given object is empty', () => { expect(() => faker.helpers.objectValue({})).toThrowError( - new FakerError('Cannot get value from empty set.') + new FakerError('Cannot get value from empty dataset.') ); }); }); @@ -962,7 +962,7 @@ describe('helpers', () => { it('does not allow empty array parameters', () => { expect(() => faker.helpers.fake([])).toThrowError( - new FakerError('Cannot get value from empty set.') + new FakerError('Cannot get value from empty dataset.') ); }); From 399d4b23090647ef2d6d2d3c1347f710c9624a67 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Sun, 16 Apr 2023 15:32:48 +0200 Subject: [PATCH 07/21] docs: add examples to upgrading --- docs/guide/upgrading.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index db69e6ab27a..00b4e34bde5 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -116,6 +116,38 @@ The methods will now throw an `FakerError` instead. The same thing happens now if you provide an empty object `{}` to `faker.helpers.objectKey` or `faker.helpers.objectValue`. +**Old** + +```ts +const allTags = ['dogs', 'cats', 'fish', 'horses', 'sheep']; +const tags = faker.helpers.arrayElements(allTags, { min: 0, max: 3 }); +// `tags` might be an empty array which was no problem in v7 +const featuredTag = faker.helpers.arrayElement(tags); +// `featureTag` will will be typed as `string` but could actually be `undefined` +``` + +**New** + +```ts +const allTags = ['dogs', 'cats', 'fish', 'horses', 'sheep']; +const tags = faker.helpers.arrayElements(allTags, { min: 0, max: 3 }); +// `tags` might be an empty array which will throw in v8 +const featuredTag = + tags.length === 0 ? undefined : faker.helpers.arrayElement(tags); +// `featureTag` has to be explicitly set to `undefined` on your side + +// OR + +const allTags = ['dogs', 'cats', 'fish', 'horses', 'sheep']; +const tags = faker.helpers.arrayElements(allTags, { min: 0, max: 3 }); +let featuredTag: string | undefined; +try { + featuredTag = faker.helpers.arrayElement(post.tags); +} catch (e) { + // handle error and do something special +} +``` + ### Other deprecated methods removed/replaced | Old method | New method | From 7e305d180de7f137eadb4b4723222ba298536bf7 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Sun, 16 Apr 2023 15:50:11 +0200 Subject: [PATCH 08/21] chore: update todo comment --- test/location.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/location.spec.ts b/test/location.spec.ts index 20597da650e..ab991ac3bb2 100644 --- a/test/location.spec.ts +++ b/test/location.spec.ts @@ -41,7 +41,7 @@ describe('location', () => { seededTests(faker, 'location', (t) => { t.it('street'); - // todo - missing locale data + // TODO @xDivisionByZerox 2023-04-16: add street name locale data to `en` t.skip('streetName'); t.it('buildingNumber'); From 7b4ba2f4bad12c0657547e868de5a216864c26b8 Mon Sep 17 00:00:00 2001 From: DivisionByZero Date: Sun, 16 Apr 2023 22:41:56 +0200 Subject: [PATCH 09/21] Apply suggestions from code review Co-authored-by: Shinigami --- 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 00b4e34bde5..546a7c1b6c8 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -107,7 +107,7 @@ The `faker.location.zipCodeByState` method has been deprecated, but will also no ### Methods will throw on empty data set inputs -The methods `faker.helper.arrayElement` and `faker.helper.arrayElements` previously defaulted the `array` argument to a simply string array if non was provided. +The methods `faker.helpers.arrayElement` and `faker.helpers.arrayElements` previously defaulted the `array` argument to a simple string array if none was provided. This behavior is no longer supported. Additionally, by providing an empty array argument (`[]`) the functions previously returned `undefined`. From 909a1e6ea6ac0f2cfa915514e63c3916b74a03a8 Mon Sep 17 00:00:00 2001 From: DivisionByZero Date: Sun, 16 Apr 2023 22:42:24 +0200 Subject: [PATCH 10/21] Apply suggestions from code review 2 Co-authored-by: Shinigami --- 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 546a7c1b6c8..e2582a81321 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -123,7 +123,7 @@ const allTags = ['dogs', 'cats', 'fish', 'horses', 'sheep']; const tags = faker.helpers.arrayElements(allTags, { min: 0, max: 3 }); // `tags` might be an empty array which was no problem in v7 const featuredTag = faker.helpers.arrayElement(tags); -// `featureTag` will will be typed as `string` but could actually be `undefined` +// `featureTag` will be typed as `string` but could actually be `undefined` ``` **New** From 8bdd01ffce89e0620ef6249ede6ac69fef32f71d Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Mon, 17 Apr 2023 15:35:57 +0200 Subject: [PATCH 11/21] docs: more explicit phrasing --- docs/guide/upgrading.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index e2582a81321..1bc17f97b49 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -108,7 +108,8 @@ The `faker.location.zipCodeByState` method has been deprecated, but will also no ### Methods will throw on empty data set inputs The methods `faker.helpers.arrayElement` and `faker.helpers.arrayElements` previously defaulted the `array` argument to a simple string array if none was provided. -This behavior is no longer supported. +This behavior is no longer supported, as the default value has been removed. +You are now required to provide an argument. Additionally, by providing an empty array argument (`[]`) the functions previously returned `undefined`. This behavior was undesired and has been removed. From 1e453a02acc1d1bc166d8a924fc1fc337ff042ad Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 17:26:25 +0200 Subject: [PATCH 12/21] refactor(helpers): throw on undefined arguments --- src/modules/helpers/index.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 6caf4db8e80..5675799c1cf 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -796,6 +796,12 @@ export class HelpersModule { * @since 6.3.0 */ arrayElement(array: ReadonlyArray): T { + if (array == null) { + throw new Error( + 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported. You would have known this if you used TypeScript :)' + ); + } + if (array.length === 0) { throw new FakerError('Cannot get value from empty dataset.'); } @@ -897,6 +903,12 @@ export class HelpersModule { max: number; } ): T[] { + if (array == null) { + throw new Error( + 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported. You would have known this if you used TypeScript :)' + ); + } + if (array.length === 0) { return []; } From 0257d589a65c9a25422376702624beaa16ab2c26 Mon Sep 17 00:00:00 2001 From: DivisionByZero Date: Thu, 20 Apr 2023 19:29:02 +0200 Subject: [PATCH 13/21] docs(helpers): rephrase JSDocs, again Co-authored-by: ST-DDT --- docs/guide/upgrading.md | 2 +- src/modules/helpers/index.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 679be58fa9c..92e0954f4a2 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -112,7 +112,7 @@ This behavior is no longer supported, as the default value has been removed. You are now required to provide an argument. Additionally, by providing an empty array argument (`[]`) the functions previously returned `undefined`. -This behavior was undesired and has been removed. +This behavior violated the expected return type of the method. The methods will now throw an `FakerError` instead. The same thing happens now if you provide an empty object `{}` to `faker.helpers.objectKey` or `faker.helpers.objectValue`. diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index fb8e15c96d2..79cebd65513 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -794,9 +794,9 @@ export class HelpersModule { * * @template T The type of the elements to pick from. * - * @param array Array to pick the value from. + * @param array The array to pick the value from. * - * @throws If array is empty. + * @throws If the given array is empty. * * @example * faker.helpers.arrayElement(['cat', 'dog', 'mouse']) // 'dog' From aa3ae3b471a3efe30bc21d75e60913942e8ae2e6 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 19:36:01 +0200 Subject: [PATCH 14/21] refactor(helpers): throw FakerError --- src/modules/helpers/index.ts | 8 ++++---- test/helpers.spec.ts | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 79cebd65513..0bde124d039 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -805,8 +805,8 @@ export class HelpersModule { */ arrayElement(array: ReadonlyArray): T { if (array == null) { - throw new Error( - 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported. You would have known this if you used TypeScript :)' + throw new FakerError( + 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported.' ); } @@ -914,8 +914,8 @@ export class HelpersModule { } ): T[] { if (array == null) { - throw new Error( - 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported. You would have known this if you used TypeScript :)' + throw new FakerError( + 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' ); } diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index a32215af6aa..90b200a6126 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -217,7 +217,11 @@ describe('helpers', () => { it.each(['', 0, undefined, null, false])('%s', (nullishValue) => { expect(() => faker.helpers.arrayElement([nullishValue]) - ).not.toThrowError(); + ).not.toThrowError( + new FakerError( + 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' + ) + ); }); }); }); From e5e8f95fa7edbac6e4ca3182461cad17ae5cf5b5 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 19:53:30 +0200 Subject: [PATCH 15/21] test(helpers): add call on no arguments --- test/helpers.spec.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 90b200a6126..04e2d8ed359 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -206,6 +206,16 @@ describe('helpers', () => { expect(actual).toBe('hello'); }); + it('should throw with no arguments', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + expect(() => faker.helpers.arrayElement()).toThrowError( + new FakerError( + 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported.' + ) + ); + }); + it('should throw on an empty array', () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-return expect(() => faker.helpers.arrayElement([])).toThrowError( @@ -482,6 +492,16 @@ describe('helpers', () => { } } ); + + it('should throw with no arguments', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + expect(() => faker.helpers.arrayElements()).toThrowError( + new FakerError( + 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' + ) + ); + }); }); describe('slugify()', () => { From f721e8d38ec175f532523ea3a762d44775a8fdf5 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 19:56:21 +0200 Subject: [PATCH 16/21] test(helpers): revert specific error in nullish value case --- test/helpers.spec.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 04e2d8ed359..12633ba00da 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -227,11 +227,7 @@ describe('helpers', () => { it.each(['', 0, undefined, null, false])('%s', (nullishValue) => { expect(() => faker.helpers.arrayElement([nullishValue]) - ).not.toThrowError( - new FakerError( - 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' - ) - ); + ).not.toThrowError(); }); }); }); From 0be6ae2ad3e271079cd1e57de5bfcbe5c6d270b0 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 20:37:08 +0200 Subject: [PATCH 17/21] chore: add todo's --- src/modules/helpers/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 0bde124d039..48c8cea09c3 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -804,6 +804,7 @@ export class HelpersModule { * @since 6.3.0 */ arrayElement(array: ReadonlyArray): T { + // TODO @xDivisionByZerox 2023-04-20: Remove in v9 if (array == null) { throw new FakerError( 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported.' @@ -913,6 +914,7 @@ export class HelpersModule { max: number; } ): T[] { + // TODO @xDivisionByZerox 2023-04-20: Remove in v9 if (array == null) { throw new FakerError( 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' From 7bc02c636bb4d28c8a0298f67d159f1be41634c6 Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 20:41:02 +0200 Subject: [PATCH 18/21] chore: replace `@ts-ignore` with `@ts-expect-error` --- test/helpers.spec.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 12633ba00da..d2c5d2f5088 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -207,8 +207,7 @@ describe('helpers', () => { }); it('should throw with no arguments', () => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error `arrayElement` without arguments is not supported in TypeScript expect(() => faker.helpers.arrayElement()).toThrowError( new FakerError( 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported.' @@ -490,8 +489,7 @@ describe('helpers', () => { ); it('should throw with no arguments', () => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error `arrayElements` without arguments is not supported in TypeScript expect(() => faker.helpers.arrayElements()).toThrowError( new FakerError( 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' From 39dd31aa2d5ab0ccb99a40bead93647d7176e29d Mon Sep 17 00:00:00 2001 From: xDivisionByZerox Date: Thu, 20 Apr 2023 20:45:32 +0200 Subject: [PATCH 19/21] test(helpers): arrayElements with nullish values --- test/helpers.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index d2c5d2f5088..0518a7a6729 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -496,6 +496,17 @@ describe('helpers', () => { ) ); }); + + describe('should not throw on an array with nullish elements', () => { + it.each(['', 0, undefined, null, false])('%s', (nullishValue) => { + expect(() => + faker.helpers.arrayElements( + [nullishValue, nullishValue, nullishValue], + 2 + ) + ).not.toThrowError(); + }); + }); }); describe('slugify()', () => { From 14f8c1aae48fa9d12498b0d63a4d42b16afdbabb Mon Sep 17 00:00:00 2001 From: DivisionByZero Date: Thu, 20 Apr 2023 23:21:16 +0200 Subject: [PATCH 20/21] Apply suggestions from code review Co-authored-by: Shinigami --- test/helpers.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 0518a7a6729..73156a2efa7 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -207,7 +207,7 @@ describe('helpers', () => { }); it('should throw with no arguments', () => { - // @ts-expect-error `arrayElement` without arguments is not supported in TypeScript + // @ts-expect-error: `arrayElement` without arguments is not supported in TypeScript expect(() => faker.helpers.arrayElement()).toThrowError( new FakerError( 'Calling `faker.helpers.arrayElement()` without arguments is no longer supported.' @@ -489,7 +489,7 @@ describe('helpers', () => { ); it('should throw with no arguments', () => { - // @ts-expect-error `arrayElements` without arguments is not supported in TypeScript + // @ts-expect-error: `arrayElements` without arguments is not supported in TypeScript expect(() => faker.helpers.arrayElements()).toThrowError( new FakerError( 'Calling `faker.helpers.arrayElements()` without arguments is no longer supported.' From 3285d1d6e1f6fb9a53a37fdabd9724209ce55352 Mon Sep 17 00:00:00 2001 From: Shinigami Date: Fri, 21 Apr 2023 18:25:43 +0200 Subject: [PATCH 21/21] Apply suggestions from code review Co-authored-by: Eric Cheng --- docs/guide/upgrading.md | 2 +- test/helpers.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md index 92e0954f4a2..6354c1785fe 100644 --- a/docs/guide/upgrading.md +++ b/docs/guide/upgrading.md @@ -111,7 +111,7 @@ The methods `faker.helpers.arrayElement` and `faker.helpers.arrayElements` previ This behavior is no longer supported, as the default value has been removed. You are now required to provide an argument. -Additionally, by providing an empty array argument (`[]`) the functions previously returned `undefined`. +Additionally, when passing in an empty array argument (`[]`), the functions previously returned `undefined`. This behavior violated the expected return type of the method. The methods will now throw an `FakerError` instead. diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts index 73156a2efa7..2dcb64aafb7 100644 --- a/test/helpers.spec.ts +++ b/test/helpers.spec.ts @@ -929,7 +929,7 @@ describe('helpers', () => { expect(Object.values(testObject)).toContain(actual); }); - it('should return undefined if given object is empty', () => { + it('should throw if given object is empty', () => { expect(() => faker.helpers.objectValue({})).toThrowError( new FakerError('Cannot get value from empty dataset.') ); @@ -989,7 +989,7 @@ describe('helpers', () => { expect(actual).toMatch(/^\d{5}$/); }); - it('does not allow empty array parameters', () => { + it('should throw with empty array parameters', () => { expect(() => faker.helpers.fake([])).toThrowError( new FakerError('Cannot get value from empty dataset.') );