From d0106c0e408d4e60f5ae6fc49516423f42a0a89e Mon Sep 17 00:00:00 2001 From: Pavlo Tymchuk Date: Mon, 16 Sep 2019 23:07:23 +0200 Subject: [PATCH 1/3] [react-devtools-shared] Added string type check for object name prop in getDisplayName function from utils.js file; tests included; --- .../src/__tests__/utils-test.js | 58 +++++++++++++++++++ packages/react-devtools-shared/src/utils.js | 8 +-- 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 packages/react-devtools-shared/src/__tests__/utils-test.js diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js new file mode 100644 index 0000000000000..ae4d3a0e42982 --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -0,0 +1,58 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {getDisplayName} from 'react-devtools-shared/src/utils'; + +describe('utils', () => { + describe('getDisplayName', () => { + const fallbackName = 'TestFallbackName'; + + it('should return default fallback name for empty object', () => { + const result = getDisplayName({}); + expect(result).toEqual('Anonymous'); + }); + + it('should return displayName property from object', () => { + const obj = { + displayName: 'TestDisplayName', + }; + const result = getDisplayName(obj); + expect(result).toEqual(obj.displayName); + }); + + it('should return name property from object', () => { + const obj = { + name: 'TestName', + }; + const result = getDisplayName(obj); + expect(result).toEqual(obj.name); + }); + + it('should return provided fallback name for empty object', () => { + const result = getDisplayName({}, fallbackName); + expect(result).toEqual(fallbackName); + }); + + it('should provide fallback name when displayName prop is not a string', () => { + const obj = { + displayName: {}, + }; + const result = getDisplayName(obj, fallbackName); + expect(result).toEqual(fallbackName); + }); + + it('should provide fallback name when name prop is not a string', () => { + const obj = { + name: {}, + }; + const result = getDisplayName(obj, fallbackName); + expect(result).toEqual(fallbackName); + }); + }); +}); diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 990ecf996896e..9def7c5e76099 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -46,17 +46,15 @@ export function getDisplayName( return nameFromCache; } - let displayName; + let displayName = fallbackName; // The displayName property is not guaranteed to be a string. // It's only safe to use for our purposes if it's a string. // github.com/facebook/react-devtools/issues/803 if (typeof type.displayName === 'string') { displayName = type.displayName; - } - - if (!displayName) { - displayName = type.name || fallbackName; + } else if (typeof type.name === 'string') { + displayName = type.name; } cachedDisplayNames.set(type, displayName); From 2556fac19b82356acdf4980664846eec376fd060 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 17 Sep 2019 13:10:26 -0700 Subject: [PATCH 2/3] Re-added empty string check to getDisplayName() --- packages/react-devtools-shared/src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 9def7c5e76099..5c22b92100b62 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -53,7 +53,7 @@ export function getDisplayName( // github.com/facebook/react-devtools/issues/803 if (typeof type.displayName === 'string') { displayName = type.displayName; - } else if (typeof type.name === 'string') { + } else if (typeof type.name === 'string' && type.name !== '') { displayName = type.name; } From 43e09fe52be4f6902f57dfd4e678a7dd719f1079 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 17 Sep 2019 13:11:25 -0700 Subject: [PATCH 3/3] Tweaked tests to use real functions This more closely simulates how the utility is being used in production, and would catch cases like anonymous functions (with empty string names). --- .../src/__tests__/utils-test.js | 50 ++++++------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index ae4d3a0e42982..37e445974a28c 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -11,48 +11,30 @@ import {getDisplayName} from 'react-devtools-shared/src/utils'; describe('utils', () => { describe('getDisplayName', () => { - const fallbackName = 'TestFallbackName'; - - it('should return default fallback name for empty object', () => { - const result = getDisplayName({}); - expect(result).toEqual('Anonymous'); - }); - - it('should return displayName property from object', () => { - const obj = { - displayName: 'TestDisplayName', - }; - const result = getDisplayName(obj); - expect(result).toEqual(obj.displayName); + it('should return a function name', () => { + function FauxComponent() {} + expect(getDisplayName(FauxComponent)).toEqual('FauxComponent'); }); - it('should return name property from object', () => { - const obj = { - name: 'TestName', - }; - const result = getDisplayName(obj); - expect(result).toEqual(obj.name); + it('should return a displayName name if specified', () => { + function FauxComponent() {} + FauxComponent.displayName = 'OverrideDisplayName'; + expect(getDisplayName(FauxComponent)).toEqual('OverrideDisplayName'); }); - it('should return provided fallback name for empty object', () => { - const result = getDisplayName({}, fallbackName); - expect(result).toEqual(fallbackName); + it('should return the fallback for anonymous functions', () => { + expect(getDisplayName(() => {}, 'Fallback')).toEqual('Fallback'); }); - it('should provide fallback name when displayName prop is not a string', () => { - const obj = { - displayName: {}, - }; - const result = getDisplayName(obj, fallbackName); - expect(result).toEqual(fallbackName); + it('should return Anonymous for anonymous functions without a fallback', () => { + expect(getDisplayName(() => {})).toEqual('Anonymous'); }); - it('should provide fallback name when name prop is not a string', () => { - const obj = { - name: {}, - }; - const result = getDisplayName(obj, fallbackName); - expect(result).toEqual(fallbackName); + // Simulate a reported bug: + // https://github.com/facebook/react/issues/16685 + it('should return a fallback when the name prop is not a string', () => { + const FauxComponent = {name: {}}; + expect(getDisplayName(FauxComponent, 'Fallback')).toEqual('Fallback'); }); }); });