From c715aa22dfa9f688fdc77ae9b640461bd9f4e1fe Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Wed, 1 Apr 2015 23:45:52 -0700 Subject: [PATCH] Introduce reactNativeComponent Summary: Creating a new native component is a pain - you have to write native code, manually export the properties you want to use in JS, then copy those props in JS in `validAttributes` and make sure you specify differs for non-scalar props, otherwise changes won't be sent to native. This diff adds native code to automatically export `nativePropTypes`, and consumes those in a new helper `requireNativeComponent`. Right now the actual types aren't used in JS, but I changed the logic to default to using `deepDiffer` for objects without an explicit differ specified (this should be much safer in general - we've had a few bugs related to bad diff configuration). The only disadvantage to always using `deepDiffer` for `Objects` is perf, so `requireNativeComponent` has an option to provide optimized differ functions, which is demonstrated in `ScrollView.js`. The native export is done similarly to the way we figure out property setters, but with class methods (prefixed with `getViewPropDef_`) that simply return dictionary literals with the type info. This also sets the stage for potentially doing JS-side type checking, which would give us better errors with more context and useful stack traces. Finally, this gives us the ability to warn or throw when requiring unsupported native views. cc @ide Test Plan: SliderIOS still works, sticky headers stick properly (they break if diffing breaks), other apps/interactions seems fine. --- Libraries/Components/ScrollView/ScrollView.js | 11 +- Libraries/Components/SliderIOS/SliderIOS.js | 9 +- .../__tests__/diffRawProperties-test.js | 138 ++++++++++++++++++ .../createReactIOSNativeComponentClass.js | 1 + Libraries/ReactIOS/diffRawProperties.js | 15 +- Libraries/ReactIOS/requireNativeComponent.js | 35 +++++ Libraries/react-native/react-native.js | 1 + React/Modules/RCTUIManager.m | 38 ++++- React/Views/RCTViewManager.h | 6 + 9 files changed, 232 insertions(+), 22 deletions(-) create mode 100644 Libraries/ReactIOS/__tests__/diffRawProperties-test.js create mode 100644 Libraries/ReactIOS/requireNativeComponent.js diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index fc7fc7223c861f..380208e6e03742 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -32,6 +32,7 @@ var flattenStyle = require('flattenStyle'); var insetsDiffer = require('insetsDiffer'); var invariant = require('invariant'); var pointsDiffer = require('pointsDiffer'); +var requireNativeComponent = require('requireNativeComponent'); var PropTypes = React.PropTypes; @@ -355,10 +356,12 @@ if (Platform.OS === 'android') { uiViewClassName: 'AndroidHorizontalScrollView', }); } else if (Platform.OS === 'ios') { - var RCTScrollView = createReactIOSNativeComponentClass({ - validAttributes: validAttributes, - uiViewClassName: 'RCTScrollView', - }); + var differs = { + contentInset: insetsDiffer, + contentOffset: pointsDiffer, + scrollIndicatorInsets: insetsDiffer, + }; + var RCTScrollView = requireNativeComponent('RCTScrollView', differs); } module.exports = ScrollView; diff --git a/Libraries/Components/SliderIOS/SliderIOS.js b/Libraries/Components/SliderIOS/SliderIOS.js index a8f2f512516efc..5adf38249f96d6 100644 --- a/Libraries/Components/SliderIOS/SliderIOS.js +++ b/Libraries/Components/SliderIOS/SliderIOS.js @@ -14,13 +14,11 @@ var NativeMethodsMixin = require('NativeMethodsMixin'); var PropTypes = require('ReactPropTypes'); var React = require('React'); -var ReactIOSViewAttributes = require('ReactIOSViewAttributes'); var StyleSheet = require('StyleSheet'); var View = require('View'); -var createReactIOSNativeComponentClass = - require('createReactIOSNativeComponentClass'); var merge = require('merge'); +var requireNativeComponent = require('requireNativeComponent'); type Event = Object; @@ -94,9 +92,6 @@ var styles = StyleSheet.create({ }, }); -var RCTSlider = createReactIOSNativeComponentClass({ - validAttributes: merge(ReactIOSViewAttributes.UIView, {value: true}), - uiViewClassName: 'RCTSlider', -}); +var RCTSlider = requireNativeComponent('RCTSlider'); module.exports = SliderIOS; diff --git a/Libraries/ReactIOS/__tests__/diffRawProperties-test.js b/Libraries/ReactIOS/__tests__/diffRawProperties-test.js new file mode 100644 index 00000000000000..8b5af29d409b75 --- /dev/null +++ b/Libraries/ReactIOS/__tests__/diffRawProperties-test.js @@ -0,0 +1,138 @@ +/** + * 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); + }); + + }); diff --git a/Libraries/ReactIOS/createReactIOSNativeComponentClass.js b/Libraries/ReactIOS/createReactIOSNativeComponentClass.js index 5a5af04dc815da..f658e792d53837 100644 --- a/Libraries/ReactIOS/createReactIOSNativeComponentClass.js +++ b/Libraries/ReactIOS/createReactIOSNativeComponentClass.js @@ -37,6 +37,7 @@ var createReactIOSNativeComponentClass = function( }; Constructor.displayName = viewConfig.uiViewClassName; Constructor.prototype = new ReactIOSNativeComponent(viewConfig); + Constructor.viewConfig = viewConfig; return Constructor; }; diff --git a/Libraries/ReactIOS/diffRawProperties.js b/Libraries/ReactIOS/diffRawProperties.js index 3a5de284f4910e..96e43e4a3552f8 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 @@ -31,6 +33,7 @@ function diffRawProperties( var validAttributeConfig; var nextProp; var prevProp; + var differ; var isScalar; var shouldUpdate; @@ -43,15 +46,11 @@ function diffRawProperties( prevProp = prevProps && prevProps[propKey]; nextProp = nextProps[propKey]; 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 || {}; diff --git a/Libraries/ReactIOS/requireNativeComponent.js b/Libraries/ReactIOS/requireNativeComponent.js new file mode 100644 index 00000000000000..8f195e16987682 --- /dev/null +++ b/Libraries/ReactIOS/requireNativeComponent.js @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule requireNativeComponent + * @flow + */ +'use strict'; + +var RCTUIManager = require('NativeModules').UIManager; + +var createReactIOSNativeComponentClass = require('createReactIOSNativeComponentClass'); + +function requireNativeComponent(viewName: string, customDiffers?: Object): Function { + var viewConfig = RCTUIManager.viewConfigs[viewName]; + if (!viewConfig) { + console.warn( + 'Native view `' + viewName + '` is not available. Make sure the ' + + 'native module is properly built and included in your project.' + ); + viewConfig = RCTUIManager.viewConfigs.RCTView; + } + viewConfig.validAttributes = {}; + for (var key in viewConfig.nativePropTypes) { + var customDiffer = customDiffers && customDiffers[key]; + viewConfig.validAttributes[key] = customDiffer ? {diff: customDiffer} : true; + } + return createReactIOSNativeComponentClass(viewConfig); +} + +module.exports = requireNativeComponent; diff --git a/Libraries/react-native/react-native.js b/Libraries/react-native/react-native.js index 46148bd86d27e5..66aa0cd958a7c2 100644 --- a/Libraries/react-native/react-native.js +++ b/Libraries/react-native/react-native.js @@ -60,6 +60,7 @@ var ReactNative = Object.assign(Object.create(require('React')), { // Plugins DeviceEventEmitter: require('RCTDeviceEventEmitter'), NativeModules: require('NativeModules'), + requireNativeComponent: require('requireNativeComponent'), addons: { LinkedStateMixin: require('LinkedStateMixin'), diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index 82324f28126095..fb6b1562a312e2 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -192,6 +192,7 @@ @implementation RCTUIManager NSMutableDictionary *_defaultShadowViews; // RCT thread only NSMutableDictionary *_defaultViews; // Main thread only NSDictionary *_viewManagers; + NSDictionary *_viewConfigs; } @synthesize bridge =_bridge; @@ -209,6 +210,23 @@ @implementation RCTUIManager return name; } +static void RCTAddViewInfoForModule(Class managerClass, NSMutableDictionary *viewConfigs) +{ + static const char *prefix = "getViewPropDef_"; + static const NSUInteger prefixLength = sizeof("getViewPropDef_") - 1; + unsigned int methodCount = 0; + Method *methods = class_copyMethodList(objc_getMetaClass(class_getName(managerClass)), &methodCount); + for (unsigned int i = 0; i < methodCount; i++) { + Method method = methods[i]; + SEL getInfo = method_getName(method); + const char *selName = sel_getName(getInfo); + if (strlen(selName) > prefixLength && strncmp(selName, prefix, prefixLength) == 0) { + NSDictionary *info = ((NSDictionary *(*)(id, SEL))method_getImplementation(method))(managerClass, getInfo); + viewConfigs[info[@"name"]] = info[@"type"]; + } + } +} + /** * This private constructor should only be called when creating * isolated UIImanager instances for testing. Normal initialization @@ -261,13 +279,24 @@ - (void)setBridge:(RCTBridge *)bridge _shadowQueue = _bridge.shadowQueue; // Get view managers from bridge + NSMutableDictionary *viewConfigs = [NSMutableDictionary new]; + NSMutableDictionary *viewManagers = [[NSMutableDictionary alloc] init]; + NSString *baseViewName = RCTViewNameForModuleName([RCTViewManager moduleName]); + viewConfigs[baseViewName] = [NSMutableDictionary new]; + RCTAddViewInfoForModule([RCTViewManager class], viewConfigs[baseViewName]); [_bridge.modules enumerateKeysAndObjectsUsingBlock:^(NSString *moduleName, RCTViewManager *manager, BOOL *stop) { if ([manager isKindOfClass:[RCTViewManager class]]) { - viewManagers[RCTViewNameForModuleName(moduleName)] = manager; + NSString *viewName = RCTViewNameForModuleName(moduleName); + viewManagers[viewName] = manager; + if (viewName == baseViewName) { + return; + } + viewConfigs[viewName] = [viewConfigs[baseViewName] mutableCopy]; // include base props + RCTAddViewInfoForModule([manager class], viewConfigs[viewName]); } }]; - + _viewConfigs = [viewConfigs copy]; _viewManagers = [viewManagers copy]; } } @@ -1371,7 +1400,10 @@ - (NSDictionary *)constantsToExport allJSConstants[name] = [constantsNamespace copy]; } }]; - + allJSConstants[@"viewConfigs"] = [NSMutableDictionary new]; + [_viewConfigs enumerateKeysAndObjectsUsingBlock:^(NSString *viewName, NSDictionary *viewProps, BOOL *stop) { + allJSConstants[@"viewConfigs"][viewName] = @{@"nativePropTypes": viewProps, @"uiViewClassName": viewName}; + }]; return allJSConstants; } diff --git a/React/Views/RCTViewManager.h b/React/Views/RCTViewManager.h index 32babecc96e428..d16e08a631281c 100644 --- a/React/Views/RCTViewManager.h +++ b/React/Views/RCTViewManager.h @@ -122,6 +122,9 @@ typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, RCTSparseArray *v (!json && !RCTCopyProperty(view, defaultView, @#keyPath))) { \ RCTLogError(@"%@ does not have setter for `%s` property", [view class], #name); \ } \ +} \ ++ (NSDictionary *)getViewPropDef_##name { \ + return @{@"name": @#name, @"type": @#type}; \ } #define RCT_REMAP_SHADOW_PROPERTY(name, keyPath, type) \ @@ -138,6 +141,9 @@ typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, RCTSparseArray *v * refer to "json", "view" and "defaultView" to implement the required logic. */ #define RCT_CUSTOM_VIEW_PROPERTY(name, type, viewClass) \ ++ (NSDictionary *)getViewPropDef_##name { \ +return @{@"name": @#name, @"type": @#type}; \ +} \ - (void)set_##name:(id)json forView:(viewClass *)view withDefaultView:(viewClass *)defaultView #define RCT_CUSTOM_SHADOW_PROPERTY(name, type, viewClass) \