From 5d4140c513c9b6a7d88ff0516021a7f7345b70df Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Tue, 14 Jul 2015 17:06:03 -0700 Subject: [PATCH] [ReactNative] deepDiff by default --- .../__tests__/diffRawProperties-test.js | 150 ++++++++++++++++++ Libraries/ReactIOS/diffRawProperties.js | 30 ++-- Libraries/ReactIOS/requireNativeComponent.js | 6 +- 3 files changed, 166 insertions(+), 20 deletions(-) create mode 100644 Libraries/ReactIOS/__tests__/diffRawProperties-test.js diff --git a/Libraries/ReactIOS/__tests__/diffRawProperties-test.js b/Libraries/ReactIOS/__tests__/diffRawProperties-test.js new file mode 100644 index 000000000..3d1fa3478 --- /dev/null +++ b/Libraries/ReactIOS/__tests__/diffRawProperties-test.js @@ -0,0 +1,150 @@ +/** + * Copyright 2004-present Facebook. All Rights Reserved. + */ +'use strict'; + +jest.dontMock('diffRawProperties'); +jest.dontMock('deepDiffer'); +var diffRawProperties = require('diffRawProperties'); + +describe('diffRawProperties', function() { + + it('should work with simple example', () => { + expect(diffRawProperties( + null, + {a: 1, c: 3}, + {b: 2, c: 3}, + {a: true, b: true} + )).toEqual({a: null, b: 2}); + }); + + it('should skip fields that are equal', () => { + expect(diffRawProperties( + null, + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0}, + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0}, + {a: true, b: true, c: true, d: true, e: true, f: true} + )).toEqual(null); + }); + + it('should remove fields', () => { + expect(diffRawProperties( + null, + {a: 1}, + {}, + {a: true} + )).toEqual({a: null}); + }); + + it('should ignore invalid fields', () => { + expect(diffRawProperties( + null, + {a: 1}, + {b: 2}, + {} + )).toEqual(null); + }); + + it('should override the updatePayload argument', () => { + var updatePayload = {a: 1}; + var result = diffRawProperties( + updatePayload, + {}, + {b: 2}, + {b: true} + ); + + expect(result).toBe(updatePayload); + expect(result).toEqual({a: 1, b: 2}); + }); + + it('should use the diff attribute', () => { + var diffA = jest.genMockFunction().mockImpl((a, b) => true); + var diffB = jest.genMockFunction().mockImpl((a, b) => false); + expect(diffRawProperties( + null, + {a: [1], b: [3]}, + {a: [2], b: [4]}, + {a: {diff: diffA}, b: {diff: diffB}} + )).toEqual({a: [2]}); + expect(diffA).toBeCalledWith([1], [2]); + expect(diffB).toBeCalledWith([3], [4]); + }); + + it('should not use the diff attribute on addition/removal', () => { + var diffA = jest.genMockFunction(); + var diffB = jest.genMockFunction(); + expect(diffRawProperties( + null, + {a: [1]}, + {b: [2]}, + {a: {diff: diffA}, b: {diff: diffB}} + )).toEqual({a: null, b: [2]}); + expect(diffA).not.toBeCalled(); + expect(diffB).not.toBeCalled(); + }); + + it('should do deep diffs of Objects by default', () => { + expect(diffRawProperties( + null, + {a: [1], b: {k: [3,4]}, c: {k: [4,4]} }, + {a: [2], b: {k: [3,4]}, c: {k: [4,5]} }, + {a: true, b: true, c: true} + )).toEqual({a: [2], c: {k: [4,5]}}); + }); + + it('should work with undefined styles', () => { + expect(diffRawProperties( + null, + {a: 1, c: 3}, + undefined, + {a: true, b: true} + )).toEqual({a: null}); + expect(diffRawProperties( + null, + undefined, + {a: 1, c: 3}, + {a: true, b: true} + )).toEqual({a: 1}); + expect(diffRawProperties( + null, + undefined, + undefined, + {a: true, b: true} + )).toEqual(null); + }); + + it('should work with empty styles', () => { + expect(diffRawProperties( + null, + {a: 1, c: 3}, + {}, + {a: true, b: true} + )).toEqual({a: null}); + expect(diffRawProperties( + null, + {}, + {a: 1, c: 3}, + {a: true, b: true} + )).toEqual({a: 1}); + expect(diffRawProperties( + null, + {}, + {}, + {a: true, b: true} + )).toEqual(null); + }); + + // Function properties are just markers to native that events should be sent. + it('should convert functions to booleans', () => { + // Note that if the property changes from one function to another, we don't + // need to send an update. + expect(diffRawProperties( + null, + {a: function() { return 1; }, b: function() { return 2; }, c: 3}, + {b: function() { return 9; }, c: function() { return 3; }, }, + {a: true, b: true, c: true} + )).toEqual({a: null, c: true}); + }); + + }); diff --git a/Libraries/ReactIOS/diffRawProperties.js b/Libraries/ReactIOS/diffRawProperties.js index ddd6edbea..96a2ae795 100644 --- a/Libraries/ReactIOS/diffRawProperties.js +++ b/Libraries/ReactIOS/diffRawProperties.js @@ -11,6 +11,8 @@ */ 'use strict'; +var deepDiffer = require('deepDiffer'); + /** * diffRawProperties takes two sets of props and a set of valid attributes * and write to updatePayload the values that changed or were deleted @@ -33,6 +35,7 @@ function diffRawProperties( var prevProp; var isScalar; var shouldUpdate; + var differ; if (nextProps) { for (var propKey in nextProps) { @@ -53,16 +56,11 @@ function diffRawProperties( } if (prevProp !== nextProp) { - // If you want a property's diff to be detected, you must configure it - // to be so - *or* it must be a scalar property. For now, we'll allow - // creation with any attribute that is not scalar, but we should - // eventually even reject those unless they are properly configured. + // Scalars and new props are always updated. Objects use deepDiffer by + // default, but can be optimized with custom differs. + differ = validAttributeConfig.diff || deepDiffer; isScalar = typeof nextProp !== 'object' || nextProp === null; - shouldUpdate = isScalar || - !prevProp || - validAttributeConfig.diff && - validAttributeConfig.diff(prevProp, nextProp); - + shouldUpdate = isScalar || !prevProp || differ(prevProp, nextProp); if (shouldUpdate) { updatePayload = updatePayload || {}; updatePayload[propKey] = nextProp; @@ -99,14 +97,14 @@ function diffRawProperties( if (nextProp === undefined) { nextProp = null; // null is a sentinel we explicitly send to native } - // If you want a property's diff to be detected, you must configure it - // to be so - *or* it must be a scalar property. For now, we'll allow - // creation with any attribute that is not scalar, but we should - // eventually even reject those unless they are properly configured. + // Scalars and new props are always updated. Objects use deepDiffer by + // default, but can be optimized with custom differs. + differ = validAttributeConfig.diff || deepDiffer; isScalar = typeof nextProp !== 'object' || nextProp === null; - shouldUpdate = isScalar && prevProp !== nextProp || - validAttributeConfig.diff && - validAttributeConfig.diff(prevProp, nextProp); + shouldUpdate = + isScalar && + prevProp !== nextProp || + differ(prevProp, nextProp); if (shouldUpdate) { updatePayload = updatePayload || {}; updatePayload[propKey] = nextProp; diff --git a/Libraries/ReactIOS/requireNativeComponent.js b/Libraries/ReactIOS/requireNativeComponent.js index 825739738..716b8f806 100644 --- a/Libraries/ReactIOS/requireNativeComponent.js +++ b/Libraries/ReactIOS/requireNativeComponent.js @@ -15,7 +15,6 @@ var RCTUIManager = require('NativeModules').UIManager; var UnimplementedView = require('UnimplementedView'); var createReactNativeComponentClass = require('createReactNativeComponentClass'); -var deepDiffer = require('deepDiffer'); var insetsDiffer = require('insetsDiffer'); var pointsDiffer = require('pointsDiffer'); var matricesDiffer = require('matricesDiffer'); @@ -54,9 +53,8 @@ function requireNativeComponent( viewConfig.uiViewClassName = viewName; viewConfig.validAttributes = {}; for (var key in nativeProps) { - // TODO: deep diff by default in diffRawProperties instead of setting it here - var differ = TypeToDifferMap[nativeProps[key]] || deepDiffer; - viewConfig.validAttributes[key] = {diff: differ}; + var differ = TypeToDifferMap[nativeProps[key]]; + viewConfig.validAttributes[key] = differ ? {diff: differ} : true; } if (__DEV__) { wrapperComponent && verifyPropTypes(wrapperComponent, viewConfig);