Skip to content

Commit

Permalink
Remove RN fiber createClass wrapper around View
Browse files Browse the repository at this point in the history 
Reviewed By: spicyj

Differential Revision: D5241527

fbshipit-source-id: 9209004544e83cc0f03fcaa27c9b1acf8db09930
  • Loading branch information
@facebook-github-bot
Brian Vaughn authored and facebook-github-bot committed Jun 21, 2017
1 parent 31e1f37 commit 1199592
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 109 deletions.
7 changes: 5 additions & 2 deletions Libraries/Components/ScrollView/ScrollView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const invariant = require('fbjs/lib/invariant');
const processDecelerationRate = require('processDecelerationRate');
const requireNativeComponent = require('requireNativeComponent');

import type {NativeMethodsMixinType} from 'ReactNativeTypes';

/**
* Component that wraps platform ScrollView while providing
* integration with touch locking "responder" system.
Expand Down Expand Up @@ -592,8 +594,8 @@ const ScrollView = React.createClass({
this._scrollViewRef = ref;
},

_innerViewRef: (null: ?View),
_setInnerViewRef: function(ref: ?View) {
_innerViewRef: (null: ?NativeMethodsMixinType),
_setInnerViewRef: function(ref: ?NativeMethodsMixinType) {
this._innerViewRef = ref;
},

Expand Down Expand Up @@ -825,6 +827,7 @@ if (Platform.OS === 'android') {
(ScrollView: ReactClass<any>),
nativeOnlyProps,
);
// $FlowFixMe (bvaughn) Update ComponentInterface in ViewPropTypes to include a string type (for Fiber host components) in a follow-up.
RCTScrollContentView = requireNativeComponent('RCTScrollContentView', View);
}

Expand Down
103 changes: 4 additions & 99 deletions Libraries/Components/View/View.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,15 @@
'use strict';

const NativeMethodsMixin = require('NativeMethodsMixin');
const NativeModules = require('NativeModules');
const Platform = require('Platform');
const PropTypes = require('prop-types');
const React = require('React');
const ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
const ReactNativeStyleAttributes = require('ReactNativeStyleAttributes');
const ReactNativeViewAttributes = require('ReactNativeViewAttributes');
const ViewPropTypes = require('ViewPropTypes');

const invariant = require('fbjs/lib/invariant');
const requireNativeComponent = require('requireNativeComponent');
const warning = require('fbjs/lib/warning');

const {
AccessibilityComponentTypes,
AccessibilityTraits,
} = require('ViewAccessibility');

const forceTouchAvailable = (NativeModules.PlatformConstants &&
NativeModules.PlatformConstants.forceTouchAvailable) || false;

import type {ViewProps} from 'ViewPropTypes';

Expand Down Expand Up @@ -94,18 +83,9 @@ const View = React.createClass({
// `propTypes` should not be accessed directly on View since this wrapper only
// exists for DEV mode. However it's important for them to be declared.
// If the object passed to `createClass` specifies `propTypes`, Flow will
// create a static type from it. This property will be over-written below with
// a warn-on-use getter though.
// TODO (bvaughn) Remove the warn-on-use comment after April 1.
// create a static type from it.
propTypes: ViewPropTypes,

// ReactElementValidator will (temporarily) use this private accessor when
// detected to avoid triggering the warning message.
// TODO (bvaughn) Remove this after April 1 ReactNative RC is tagged.
statics: {
__propTypesSecretDontUseThesePlease: ViewPropTypes
},

/**
* `NativeMethodsMixin` will look for this when invoking `setNativeProps`. We
* make `this` look like an actual native component class.
Expand All @@ -132,70 +112,6 @@ const View = React.createClass({
},
});

// Warn about unsupported use of View static properties as these will no longer
// be supported with React fiber. This warning message will go away in the next
// ReactNative release. Use defineProperty() rather than createClass() statics
// because the mixin process auto-triggers the 1-time warning message.
// TODO (bvaughn) Remove this after April 1 ReactNative RC is tagged.
function mixinStatics (target) {
let warnedAboutAccessibilityTraits = false;
let warnedAboutAccessibilityComponentType = false;
let warnedAboutForceTouchAvailable = false;
let warnedAboutPropTypes = false;

// $FlowFixMe https://github.com/facebook/flow/issues/285
Object.defineProperty(target, 'AccessibilityTraits', {
get: function() {
warning(
warnedAboutAccessibilityTraits,
'View.AccessibilityTraits has been deprecated and will be ' +
'removed in a future version of ReactNative. Use ' +
'ViewAccessibility.AccessibilityTraits instead.'
);
warnedAboutAccessibilityTraits = true;
return AccessibilityTraits;
}
});
// $FlowFixMe https://github.com/facebook/flow/issues/285
Object.defineProperty(target, 'AccessibilityComponentType', {
get: function() {
warning(
warnedAboutAccessibilityComponentType,
'View.AccessibilityComponentType has been deprecated and will be ' +
'removed in a future version of ReactNative. Use ' +
'ViewAccessibility.AccessibilityComponentTypes instead.'
);
warnedAboutAccessibilityComponentType = true;
return AccessibilityComponentTypes;
}
});
// $FlowFixMe https://github.com/facebook/flow/issues/285
Object.defineProperty(target, 'forceTouchAvailable', {
get: function() {
warning(
warnedAboutForceTouchAvailable,
'View.forceTouchAvailable has been deprecated and will be removed ' +
'in a future version of ReactNative. Use ' +
'NativeModules.PlatformConstants.forceTouchAvailable instead.'
);
warnedAboutForceTouchAvailable = true;
return forceTouchAvailable;
}
});
// $FlowFixMe https://github.com/facebook/flow/issues/285
Object.defineProperty(target, 'propTypes', {
get: function() {
warning(
warnedAboutPropTypes,
'View.propTypes has been deprecated and will be removed in a future ' +
'version of ReactNative. Use ViewPropTypes instead.'
);
warnedAboutPropTypes = true;
return ViewPropTypes;
}
});
}

const RCTView = requireNativeComponent('RCTView', View, {
nativeOnly: {
nativeBackgroundAndroid: true,
Expand All @@ -216,21 +132,10 @@ if (__DEV__) {
}
}

// TODO (bvaughn) Remove feature flags once all static View accessors are gone.
// We temporarily wrap fiber native views with the create-class View above,
// Because external code sometimes accesses static properties of this view.
let ViewToExport = RCTView;
if (
__DEV__ ||
ReactNativeFeatureFlags.useFiber
) {
mixinStatics(View);
if (__DEV__) {
ViewToExport = View;
} else {
// TODO (bvaughn) Remove this mixin once all static View accessors are gone.
mixinStatics((RCTView : any));
}

// TODO (bvaughn) Temporarily mask Flow warnings for View property accesses.
// We're wrapping the string type (Fiber) for now to avoid any actual problems.
module.exports = ((ViewToExport : any) : typeof View);
// No one should depend on the DEV-mode createClass View wrapper.
module.exports = ((ViewToExport : any) : typeof RCTView);
9 changes: 4 additions & 5 deletions Libraries/Experimental/SwipeableRow/SwipeableQuickActionButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
'use strict';

const Image = require('Image');
const React = require('React');
const PropTypes = require('prop-types');
const React = require('React');
const Text = require('Text');
const TouchableHighlight = require('TouchableHighlight');
const View = require('View');

const ViewPropTypes = require('ViewPropTypes');

import type {ImageSource} from 'ImageSource';
Expand All @@ -31,12 +30,12 @@ class SwipeableQuickActionButton extends React.Component {
props: {
accessibilityLabel?: string,
imageSource: ImageSource | number,
imageStyle?: ?View.propTypes.style,
imageStyle?: ?ViewPropTypes.style,
onPress?: Function,
style?: ?View.propTypes.style,
style?: ?ViewPropTypes.style,
testID?: string,
text?: ?(string | Object | Array<string | Object>),
textStyle?: ?View.propTypes.style,
textStyle?: ?ViewPropTypes.style,
};

static propTypes = {
Expand Down
4 changes: 3 additions & 1 deletion Libraries/Experimental/WindowedListView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const infoLog = require('infoLog');
const invariant = require('fbjs/lib/invariant');
const nullthrows = require('fbjs/lib/nullthrows');

import type {NativeMethodsMixinType} from 'ReactNativeTypes';

const DEBUG = false;

/**
Expand Down Expand Up @@ -613,7 +615,7 @@ type CellProps = {
};
class CellRenderer extends React.Component {
props: CellProps;
_containerRef: View;
_containerRef: NativeMethodsMixinType;
_offscreenRenderDone = false;
_timeout = 0;
_lastLayout: ?Object = null;
Expand Down
4 changes: 2 additions & 2 deletions RNTester/js/RNTesterPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
*/
'use strict';

var React = require('react');
var PropTypes = require('prop-types');
var React = require('react');
var ReactNative = require('react-native');
var {
ScrollView,
Expand All @@ -37,7 +37,7 @@ class RNTesterPage extends React.Component {
var ContentWrapper;
var wrapperProps = {};
if (this.props.noScroll) {
ContentWrapper = (View: ReactClass<any>);
ContentWrapper = ((View: any): ReactClass<any>);
} else {
ContentWrapper = (ScrollView: ReactClass<any>);
// $FlowFixMe found when converting React.createClass to ES6
Expand Down

2 comments on commit 1199592

@slorber
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bvaughn @spicyj any chance this commit leads to View import being "RCTView" string in production while it's a component in dev?

@bvaughn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the intent, yes. The current implementation provides an addition check/warning in DEV that isn't included in production so that we don't have to pay the cost of the extra wrapper component around each View.

Please sign in to comment.