Skip to content
This repository was archived by the owner on Jan 10, 2025. It is now read-only.

Commit c294df9

Browse files
authored
Update formatName and abbreviateName to return a string or undefined (#2844)
1 parent 8e0fcba commit c294df9

File tree

7 files changed

+116
-26
lines changed

7 files changed

+116
-26
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/react-i18n': minor
3+
'@shopify/name': minor
4+
---
5+
6+
Update formatName and abbreviateName to return a string or undefined

packages/name/src/formatName.ts

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,44 @@
11
import {FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE} from './constants';
22
import {languageFromLocale} from './utilities';
3+
import {nonEmptyOrUndefined} from './utilities/nonEmptyOrUndefined';
34

45
// 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.
56
export function formatName({
67
name,
78
locale,
89
options,
910
}: {
10-
name: {givenName?: string; familyName?: string};
11+
name: {givenName?: string | null; familyName?: string | null};
1112
locale: string;
1213
options?: {full?: boolean};
1314
}) {
14-
if (!name.givenName) {
15-
return name.familyName || '';
15+
const givenName = nonEmptyOrUndefined(name?.givenName);
16+
const familyName = nonEmptyOrUndefined(name?.familyName);
17+
18+
if (familyName && !givenName) {
19+
return familyName;
1620
}
17-
if (!name.familyName) {
18-
return name.givenName;
21+
22+
if (givenName && !familyName) {
23+
return givenName;
1924
}
2025

21-
const isFullName = Boolean(options && options.full);
26+
if (givenName && familyName) {
27+
const isFullName = Boolean(options && options.full);
2228

23-
const customNameFormatter =
24-
FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE.get(
25-
languageFromLocale(locale),
26-
);
29+
const customNameFormatter =
30+
FAMILY_NAME_GIVEN_NAME_ORDERING_INDEXED_BY_LANGUAGE.get(
31+
languageFromLocale(locale),
32+
);
2733

28-
if (customNameFormatter) {
29-
return customNameFormatter(name.givenName, name.familyName, isFullName);
30-
}
31-
if (isFullName) {
32-
return `${name.givenName} ${name.familyName}`;
34+
if (customNameFormatter) {
35+
return customNameFormatter(givenName, familyName, isFullName);
36+
}
37+
38+
if (isFullName) {
39+
return `${givenName} ${familyName}`;
40+
}
3341
}
34-
return name.givenName;
42+
43+
return givenName;
3544
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
11
import {formatName} from '../formatName';
22
import {abbreviateName} from '../abbreviateName';
3+
import * as formatNameFunction from '../formatName';
34

45
const locale = 'en';
56

67
describe('#abbreviateName()', () => {
8+
it('returns only givenName abbreviated when familyName is undefined', () => {
9+
const name = {givenName: 'Michael', familyName: undefined};
10+
expect(abbreviateName({name, locale})).toBe('M');
11+
});
12+
13+
it('returns only familyName abbreviated when givenName is undefined', () => {
14+
const name = {givenName: undefined, familyName: 'Garfinkle'};
15+
expect(abbreviateName({name, locale})).toBe('G');
16+
});
17+
18+
it('returns undefined if no abbreviation found', () => {
19+
// no abbreviation as names are undefined
20+
const name = {givenName: undefined, familyName: undefined};
21+
expect(abbreviateName({name, locale})).toBeUndefined();
22+
});
23+
724
it('returns formatName if no abbreviation found', () => {
825
// no abbreviation as has space in family name
926
const name = {givenName: 'Michael', familyName: 'van Finkle'};
@@ -14,4 +31,16 @@ describe('#abbreviateName()', () => {
1431
const name = {givenName: 'Michael', familyName: 'Garfinkle'};
1532
expect(abbreviateName({name, locale})).toBe('MG');
1633
});
34+
35+
it('calls formatName when tryAbbreviateName returns undefined', () => {
36+
const formatNameSpy = jest
37+
.spyOn(formatNameFunction, 'formatName')
38+
.mockImplementation(jest.fn());
39+
40+
abbreviateName({name: {}, locale});
41+
42+
expect(formatNameSpy).toHaveBeenCalled();
43+
44+
formatNameSpy.mockRestore();
45+
});
1746
});

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,29 @@
1+
import * as nonEmptyOrUndefinedFunction from '../utilities/nonEmptyOrUndefined';
12
import {formatName} from '../formatName';
23

34
describe('#formatName()', () => {
4-
it('returns an empty string when nothing is defined', () => {
5-
const name = {givenName: undefined, familyName: undefined};
5+
it('calls nonEmptyOrUndefined on givenName and familyName', () => {
6+
const nonEmptyOrUndefinedSpy = jest
7+
.spyOn(nonEmptyOrUndefinedFunction, 'nonEmptyOrUndefined')
8+
.mockImplementation(jest.fn());
9+
10+
formatName({name: {}, locale: ''});
11+
12+
expect(nonEmptyOrUndefinedSpy).toHaveBeenCalledTimes(2);
13+
14+
nonEmptyOrUndefinedSpy.mockRestore();
15+
});
16+
17+
it('returns undefined', () => {
18+
const testCases = [undefined, null, ' ', ''];
619
const locale = 'en-CA';
7-
expect(formatName({name, locale})).toBe('');
20+
21+
testCases.forEach((givenName) => {
22+
testCases.forEach((familyName) => {
23+
const name = {givenName, familyName};
24+
expect(formatName({name, locale})).toBeUndefined();
25+
});
26+
});
827
});
928

1029
it('returns only the givenName when familyName is missing', () => {
@@ -135,7 +154,7 @@ describe('#formatName()', () => {
135154
).toBe('last');
136155
});
137156

138-
it('returns a string when familyName is undefined using full', () => {
157+
it('returns undefined when familyName is undefined using full', () => {
139158
const name = {givenName: '', familyName: undefined};
140159
const locale = 'en-CA';
141160
const options = {full: true};
@@ -146,10 +165,10 @@ describe('#formatName()', () => {
146165
locale,
147166
options,
148167
}),
149-
).toBe('');
168+
).toBeUndefined();
150169
});
151170

152-
it('returns a string when givenName and familyName are missing using full', () => {
171+
it('returns undefined when givenName and familyName are missing using full', () => {
153172
const name = {givenName: undefined, familyName: undefined};
154173
const locale = 'en-CA';
155174
const options = {full: true};
@@ -160,7 +179,7 @@ describe('#formatName()', () => {
160179
locale,
161180
options,
162181
}),
163-
).toBe('');
182+
).toBeUndefined();
164183
});
165184

166185
it('defaults to givenName familyName for unknown locale using full', () => {
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/**
2+
* @returns A trimmed non-empty value. If the trimmed value is empty, undefined is returned
3+
*/
4+
export function nonEmptyOrUndefined(input?: string | null): string | undefined {
5+
if (input && input.trim().length) {
6+
return input.trim();
7+
}
8+
9+
return undefined;
10+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {nonEmptyOrUndefined} from '../nonEmptyOrUndefined';
2+
3+
describe('#nonEmptyOrUndefined()', () => {
4+
it('returns undefined', () => {
5+
const testCases = [undefined, null, '', ' '];
6+
testCases.forEach((testCase) => {
7+
expect(nonEmptyOrUndefined(testCase)).toBeUndefined();
8+
});
9+
});
10+
11+
it('returns trimmed value', () => {
12+
const testCases = ['text', ' text', ' text '];
13+
testCases.forEach((testCase) => {
14+
expect(nonEmptyOrUndefined(testCase)).toBe('text');
15+
});
16+
});
17+
});

packages/react-i18n/src/i18n.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,12 +387,12 @@ export class I18n {
387387
}
388388

389389
formatName(
390-
firstName?: string,
391-
lastName?: string,
390+
givenName?: string | null,
391+
familyName?: string | null,
392392
options?: {full?: boolean},
393393
) {
394394
return importedFormatName({
395-
name: {givenName: firstName, familyName: lastName},
395+
name: {givenName, familyName},
396396
locale: this.locale,
397397
options,
398398
});

0 commit comments

Comments
 (0)