From 99d229e186e1e5d08cb1fc417b4b911ade7aef33 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 23 Oct 2019 00:55:30 -0700 Subject: [PATCH] Make deepDiffer behavior with function inputs configurable, add logging Summary: Changelog: [Internal] Reviewed By: yungsters Differential Revision: D17951644 fbshipit-source-id: 34bff1937a6157b049193359cb0ad346c48287a6 --- .../differ/__tests__/deepDiffer-test.js | 52 +++++++++++++++++++ Libraries/Utilities/differ/deepDiffer.js | 39 ++++++++++++-- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/Libraries/Utilities/differ/__tests__/deepDiffer-test.js b/Libraries/Utilities/differ/__tests__/deepDiffer-test.js index 71ae73e59fb..a2aca448f33 100644 --- a/Libraries/Utilities/differ/__tests__/deepDiffer-test.js +++ b/Libraries/Utilities/differ/__tests__/deepDiffer-test.js @@ -128,4 +128,56 @@ describe('deepDiffer', function() { ), ).toBe(false); }); + it('should consider all functions equal', () => { + expect(deepDiffer(() => {}, x => x)).toBe(false); + const f = () => {}; + expect(deepDiffer(f, f)).toBe(false); + }); + it('should compare functions if unsafelyIgnoreFunctions is false', () => { + expect( + deepDiffer(() => {}, x => x, undefined, {unsafelyIgnoreFunctions: false}), + ).toBe(true); + const f = () => {}; + expect(deepDiffer(f, f, undefined, {unsafelyIgnoreFunctions: false})).toBe( + false, + ); + + // shorthand, omitting maxDepth + expect(deepDiffer(() => {}, x => x, {unsafelyIgnoreFunctions: false})).toBe( + true, + ); + expect(deepDiffer(f, f, {unsafelyIgnoreFunctions: false})).toBe(false); + }); + it('should log when implicitly considering two different functions equal', () => { + function a() {} + function b() {} + const listeners = {onDifferentFunctionsIgnored: jest.fn()}; + deepDiffer.unstable_setLogListeners(listeners); + try { + deepDiffer(a, a); + expect(listeners.onDifferentFunctionsIgnored).not.toHaveBeenCalled(); + + deepDiffer(a, b); + expect(listeners.onDifferentFunctionsIgnored.mock.calls).toEqual([ + ['a', 'b'], + ]); + } finally { + deepDiffer.unstable_setLogListeners(null); + } + }); + it('should not log when explicitly considering two different functions equal', () => { + function a() {} + function b() {} + const listeners = {onDifferentFunctionsIgnored: jest.fn()}; + deepDiffer.unstable_setLogListeners(listeners); + try { + deepDiffer(a, a, {unsafelyIgnoreFunctions: true}); + expect(listeners.onDifferentFunctionsIgnored).not.toHaveBeenCalled(); + + deepDiffer(a, b, {unsafelyIgnoreFunctions: true}); + expect(listeners.onDifferentFunctionsIgnored).not.toHaveBeenCalled(); + } finally { + deepDiffer.unstable_setLogListeners(null); + } + }); }); diff --git a/Libraries/Utilities/differ/deepDiffer.js b/Libraries/Utilities/differ/deepDiffer.js index cae6cde1d06..ff8bd4628d1 100644 --- a/Libraries/Utilities/differ/deepDiffer.js +++ b/Libraries/Utilities/differ/deepDiffer.js @@ -10,14 +10,31 @@ 'use strict'; +let logListeners; + +type LogListeners = {| + +onDifferentFunctionsIgnored: (nameOne: ?string, nameTwo: ?string) => void, +|}; + +type Options = {|+unsafelyIgnoreFunctions?: boolean|}; + +function unstable_setLogListeners(listeners: ?LogListeners) { + logListeners = listeners; +} + /* * @returns {bool} true if different, false if equal */ const deepDiffer = function( one: any, two: any, - maxDepth: number = -1, + maxDepthOrOptions: Options | number = -1, + maybeOptions?: Options, ): boolean { + const options = + typeof maxDepthOrOptions === 'number' ? maybeOptions : maxDepthOrOptions; + const maxDepth = + typeof maxDepthOrOptions === 'number' ? maxDepthOrOptions : -1; if (maxDepth === 0) { return true; } @@ -26,8 +43,19 @@ const deepDiffer = function( return false; } if (typeof one === 'function' && typeof two === 'function') { - // We consider all functions equal - return false; + // We consider all functions equal unless explicitly configured otherwise + let unsafelyIgnoreFunctions = options?.unsafelyIgnoreFunctions; + if (unsafelyIgnoreFunctions == null) { + if ( + logListeners && + logListeners.onDifferentFunctionsIgnored && + (!options || !('unsafelyIgnoreFunctions' in options)) + ) { + logListeners.onDifferentFunctionsIgnored(one.name, two.name); + } + unsafelyIgnoreFunctions = true; + } + return !unsafelyIgnoreFunctions; } if (typeof one !== 'object' || one === null) { // Primitives can be directly compared @@ -48,13 +76,13 @@ const deepDiffer = function( return true; } for (let ii = 0; ii < len; ii++) { - if (deepDiffer(one[ii], two[ii], maxDepth - 1)) { + if (deepDiffer(one[ii], two[ii], maxDepth - 1, options)) { return true; } } } else { for (const key in one) { - if (deepDiffer(one[key], two[key], maxDepth - 1)) { + if (deepDiffer(one[key], two[key], maxDepth - 1, options)) { return true; } } @@ -70,3 +98,4 @@ const deepDiffer = function( }; module.exports = deepDiffer; +module.exports.unstable_setLogListeners = unstable_setLogListeners;