Update formatName and abbreviateName to return a string or undefined (#…

…2844)

.changeset/unlucky-suits-promise.md

@@ -0,0 +1,6 @@
+---
+'@shopify/react-i18n': minor
+'@shopify/name': minor
+---
+
+Update formatName and abbreviateName to return a string or undefined

packages/name/src/formatName.ts

@@ -1,35 +1,44 @@
 import {FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE} from './constants';
 import {languageFromLocale} from './utilities';
+import {nonEmptyOrUndefined} from './utilities/nonEmptyOrUndefined';
 
 // Note: A similar Ruby implementation of this function also exists at https://github.com/Shopify/shopify-i18n/blob/main/lib/shopify-i18n/name_formatter.rb.
 export function formatName({
   name,
   locale,
   options,
 }: {
-  name: {givenName?: string; familyName?: string};
+  name: {givenName?: string | null; familyName?: string | null};
   locale: string;
   options?: {full?: boolean};
 }) {
-  if (!name.givenName) {
-    return name.familyName || '';
+  const givenName = nonEmptyOrUndefined(name?.givenName);
+  const familyName = nonEmptyOrUndefined(name?.familyName);
+
+  if (familyName && !givenName) {
+    return familyName;
   }
-  if (!name.familyName) {
-    return name.givenName;
+
+  if (givenName && !familyName) {
+    return givenName;
   }
 
-  const isFullName = Boolean(options && options.full);
+  if (givenName && familyName) {
+    const isFullName = Boolean(options && options.full);
 
-  const customNameFormatter =
-    FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE.get(
-      languageFromLocale(locale),
-    );
+    const customNameFormatter =
+      FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE.get(
+        languageFromLocale(locale),
+      );
 
-  if (customNameFormatter) {
-    return customNameFormatter(name.givenName, name.familyName, isFullName);
-  }
-  if (isFullName) {
-    return `${name.givenName} ${name.familyName}`;
+    if (customNameFormatter) {
+      return customNameFormatter(givenName, familyName, isFullName);
+    }
+
+    if (isFullName) {
+      return `${givenName} ${familyName}`;
+    }
   }
-  return name.givenName;
+
+  return givenName;
 }

packages/name/src/tests/abbreviateName.test.ts

@@ -1,9 +1,26 @@
 import {formatName} from '../formatName';
 import {abbreviateName} from '../abbreviateName';
+import * as formatNameFunction from '../formatName';
 
 const locale = 'en';
 
 describe('#abbreviateName()', () => {
+  it('returns only givenName abbreviated when familyName is undefined', () => {
+    const name = {givenName: 'Michael', familyName: undefined};
+    expect(abbreviateName({name, locale})).toBe('M');
+  });
+
+  it('returns only familyName abbreviated when givenName is undefined', () => {
+    const name = {givenName: undefined, familyName: 'Garfinkle'};
+    expect(abbreviateName({name, locale})).toBe('G');
+  });
+
+  it('returns undefined if no abbreviation found', () => {
+    // no abbreviation as names are undefined
+    const name = {givenName: undefined, familyName: undefined};
+    expect(abbreviateName({name, locale})).toBeUndefined();
+  });
+
   it('returns formatName if no abbreviation found', () => {
     // no abbreviation as has space in family name
     const name = {givenName: 'Michael', familyName: 'van Finkle'};
@@ -14,4 +31,16 @@ describe('#abbreviateName()', () => {
     const name = {givenName: 'Michael', familyName: 'Garfinkle'};
     expect(abbreviateName({name, locale})).toBe('MG');
   });
+
+  it('calls formatName when tryAbbreviateName returns undefined', () => {
+    const formatNameSpy = jest
+      .spyOn(formatNameFunction, 'formatName')
+      .mockImplementation(jest.fn());
+
+    abbreviateName({name: {}, locale});
+
+    expect(formatNameSpy).toHaveBeenCalled();
+
+    formatNameSpy.mockRestore();
+  });
 });

packages/name/src/tests/formatName.test.ts

@@ -1,10 +1,29 @@
+import * as nonEmptyOrUndefinedFunction from '../utilities/nonEmptyOrUndefined';
 import {formatName} from '../formatName';
 
 describe('#formatName()', () => {
-  it('returns an empty string when nothing is defined', () => {
-    const name = {givenName: undefined, familyName: undefined};
+  it('calls nonEmptyOrUndefined on givenName and familyName', () => {
+    const nonEmptyOrUndefinedSpy = jest
+      .spyOn(nonEmptyOrUndefinedFunction, 'nonEmptyOrUndefined')
+      .mockImplementation(jest.fn());
+
+    formatName({name: {}, locale: ''});
+
+    expect(nonEmptyOrUndefinedSpy).toHaveBeenCalledTimes(2);
+
+    nonEmptyOrUndefinedSpy.mockRestore();
+  });
+
+  it('returns undefined', () => {
+    const testCases = [undefined, null, ' ', ''];
     const locale = 'en-CA';
-    expect(formatName({name, locale})).toBe('');
+
+    testCases.forEach((givenName) => {
+      testCases.forEach((familyName) => {
+        const name = {givenName, familyName};
+        expect(formatName({name, locale})).toBeUndefined();
+      });
+    });
   });
 
   it('returns only the givenName when familyName is missing', () => {
@@ -135,7 +154,7 @@ describe('#formatName()', () => {
     ).toBe('last');
   });
 
-  it('returns a string when familyName is undefined using full', () => {
+  it('returns undefined when familyName is undefined using full', () => {
     const name = {givenName: '', familyName: undefined};
     const locale = 'en-CA';
     const options = {full: true};
@@ -146,10 +165,10 @@ describe('#formatName()', () => {
         locale,
         options,
       }),
-    ).toBe('');
+    ).toBeUndefined();
   });
 
-  it('returns a string when givenName and familyName are missing using full', () => {
+  it('returns undefined when givenName and familyName are missing using full', () => {
     const name = {givenName: undefined, familyName: undefined};
     const locale = 'en-CA';
     const options = {full: true};
@@ -160,7 +179,7 @@ describe('#formatName()', () => {
         locale,
         options,
       }),
-    ).toBe('');
+    ).toBeUndefined();
   });
 
   it('defaults to givenName familyName for unknown locale using full', () => {

packages/name/src/utilities/nonEmptyOrUndefined.ts

@@ -0,0 +1,10 @@
+/**
+ * @returns A trimmed non-empty value.  If the trimmed value is empty, undefined is returned
+ */
+export function nonEmptyOrUndefined(input?: string | null): string | undefined {
+  if (input && input.trim().length) {
+    return input.trim();
+  }
+
+  return undefined;
+}

packages/name/src/utilities/tests/nonEmptyOrUndefined.test.ts

@@ -0,0 +1,17 @@
+import {nonEmptyOrUndefined} from '../nonEmptyOrUndefined';
+
+describe('#nonEmptyOrUndefined()', () => {
+  it('returns undefined', () => {
+    const testCases = [undefined, null, '', ' '];
+    testCases.forEach((testCase) => {
+      expect(nonEmptyOrUndefined(testCase)).toBeUndefined();
+    });
+  });
+
+  it('returns trimmed value', () => {
+    const testCases = ['text', ' text', ' text '];
+    testCases.forEach((testCase) => {
+      expect(nonEmptyOrUndefined(testCase)).toBe('text');
+    });
+  });
+});

packages/react-i18n/src/i18n.ts

@@ -387,12 +387,12 @@ export class I18n {
   }
 
   formatName(
-    firstName?: string,
-    lastName?: string,
+    givenName?: string | null,
+    familyName?: string | null,
     options?: {full?: boolean},
   ) {
     return importedFormatName({
-      name: {givenName: firstName, familyName: lastName},
+      name: {givenName, familyName},
       locale: this.locale,
       options,
     });