[ReactNative] Export EdgeInsetsPropType and PointPropType

[ide]

Added some more exports to React that are either necessary or often useful for component authors.

[sahrens]

cc @vjeux - any reason not to do this?

[vjeux]

There's a thread on react about this. The main counter argument is that it introduces a cyclic dependency so it's a nightmare to console.log. But, this would make the require boilerplate shorter

[ide]

console.log should handle cycles anyway so you can debug cyclic objects. That said I think there isn't a cycle for ReactNative since the Object.create(React) is not the React object, so assigning React to the clone doesn't introduce a cycle.

[sophiebits]

facebook/react#3221

Personally this strikes me as a little weird and I don't want it.

[ide]

In this instance there's no cycle since react-native is exporting React along with some other native extensions. Another option is remove the Object.assign bit and just have:

// react-native.js
module.exports = {
  React: require('React'),
  // react native components
};

and then have the pattern for using it be var {React, View} = require('react-native') or alternatively

var ReactNative = require('react-native');
var {React, View} = ReactNative;

[vjeux]

If we were going this way, we should probably not extend from React.

var {React, View} = require('react-native'); // good

var React = require('react-native');
React.createClass // bad, only in React.React

var ReactNative = require('react-native');
ReactNative.React.createClass // good

[ide]

@vjeux I think so too. Perhaps the question we should answer is whether destructuring assignment is the right pattern for the react-native module.

In web React's case, we usually write var React = require('react') and JSX handles the basic DOM components, so we don't write var {div, span} = require('react');.

With React Native, does it make more sense to lowercase the basic components like View, Text, Image, and maybe Touchable* and let JSX handle them? Other components and libraries like Navigator and Vibration would be published as separate packages, which helps reduce the size of the JS bundle as well. The code would look like:

var React = require('react-native');
var Navigator = require('react-native-navigator');
var Vibration = require('react-native-vibration');
...
render() {
  return <view style={styles.container}><text>hi</text></view>;
}
...
var styles = React.createStyleSheet({
  container: {flex: 1}
}); 

[vjeux]

There are many issues with

var React = require('react-native');
var Navigator = require('react-native-navigator');
var Vibration = require('react-native-vibration');

The first one is that we're going to run into naming conflicts with npm. Someone else will steal a name that we want and we're going to either reach out and negotiate or use a less ideal one.

Then, you're running into crazy dependency issues where you have different versions of all of those plugins and conflicts. We really want to ship one bundle where everything is in sync for the core components.

One idea I had is:

var React = require('react-native/react');
var Navigator = require('react-native/navigator');
var Vibration = require('react-native/vibration');

Everything is bundled together, no namespace clash and we don't bundle things that we don't use

[ide]

I removed the React self-export so now this is just the struct prop types and the static renderers.

[ericvicenti]

StaticContainer and StaticRenderer are both considered deprecated techniques.

A more modern approach will do rendering early, and store the descriptor (the result of the render) in the component state, and get inlined in the render function.

[ide]

Removed Static helpers, it's now just prop types for EdgeInsets and Point.

[sahrens]

LGTM - @ericvicenti: can you import and land?

[sahrens]

Actually I'll just take it.

[sahrens]

@facebook-github-bot import

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1057649787583002/int_phab to review.