refactor(helpers)!: remove default value from arrayElement (#2045)

faker-js · Apr 21, 2023 · 0564446 · 0564446
1 parent 9161a06
commit 0564446
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 58 deletions.
diff --git a/docs/guide/upgrading.md b/docs/guide/upgrading.md
@@ -105,6 +105,50 @@ 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.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, as the default value has been removed.
+You are now required to provide an argument.
+
+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.
+
+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 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                                                                                                      |

diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts
@@ -794,18 +794,27 @@ 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 the given array is empty.
    *
    * @example
    * faker.helpers.arrayElement(['cat', 'dog', 'mouse']) // 'dog'
    *
    * @since 6.3.0
    */
-  arrayElement<T = string>(
-    // 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<T> = ['a', 'b', 'c'] as unknown as ReadonlyArray<T>
-  ): T {
+  arrayElement<T>(array: ReadonlyArray<T>): 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.'
+      );
+    }
+
+    if (array.length === 0) {
+      throw new FakerError('Cannot get value from empty dataset.');
+    }
+
     const index =
       array.length > 1 ? this.faker.number.int({ max: array.length - 1 }) : 0;
 
@@ -891,9 +900,7 @@ export class HelpersModule {
    * @since 6.3.0
    */
   arrayElements<T>(
-    // 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<T> = ['a', 'b', 'c'] as unknown as ReadonlyArray<T>,
+    array: ReadonlyArray<T>,
     count?:
       | number
       | {
@@ -907,6 +914,13 @@ 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.'
+      );
+    }
+
     if (array.length === 0) {
       return [];
     }
@@ -1117,10 +1131,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
@@ -1,16 +1,7 @@
 // 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`] = `
-[
-  "b",
-  "c",
-]
-`;
-
 exports[`helpers > 42 > arrayElements > with array 1`] = `
 [
   "r",
@@ -226,18 +217,8 @@ 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`] = `
-[
-  "a",
-  "c",
-  "b",
-]
-`;
-
 exports[`helpers > 1211 > arrayElements > with array 1`] = `
 [
   "d",
@@ -469,16 +450,8 @@ 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`] = `
-[
-  "b",
-]
-`;
-
 exports[`helpers > 1337 > arrayElements > with array 1`] = `
 [
   "l",

diff --git a/test/__snapshots__/location.spec.ts.snap b/test/__snapshots__/location.spec.ts.snap
@@ -142,8 +142,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"`;
@@ -296,8 +294,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"`;
@@ -450,8 +446,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
@@ -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<Faker[module]>;

diff --git 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) => {
@@ -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,
@@ -206,6 +205,30 @@ describe('helpers', () => {
 
           expect(actual).toBe('hello');
         });
+
+        it('should throw with no arguments', () => {
+          // @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.'
+            )
+          );
+        });
+
+        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 dataset.')
+          );
+        });
+
+        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', () => {
@@ -464,6 +487,26 @@ describe('helpers', () => {
             }
           }
         );
+
+        it('should throw with no arguments', () => {
+          // @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.'
+            )
+          );
+        });
+
+        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()', () => {
@@ -867,9 +910,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 dataset.')
+          );
         });
       });
 
@@ -885,9 +929,10 @@ describe('helpers', () => {
           expect(Object.values(testObject)).toContain(actual);
         });
 
-        it('should return undefined if given object is empty', () => {
-          const actual = faker.helpers.objectValue({});
-          expect(actual).toBeUndefined();
+        it('should throw if given object is empty', () => {
+          expect(() => faker.helpers.objectValue({})).toThrowError(
+            new FakerError('Cannot get value from empty dataset.')
+          );
         });
       });
 
@@ -944,9 +989,9 @@ 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('Array of pattern strings cannot be empty.')
+            new FakerError('Cannot get value from empty dataset.')
           );
         });
 

diff --git 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 @xDivisionByZerox 2023-04-16: add street name locale data to `en`
+    t.skip('streetName');
 
     t.it('buildingNumber');