WIP: Remove Haste from Libraries/ and convert to relative requires by skevy · Pull Request #14196 · facebook/react-native

skevy · 2017-05-25T21:59:32Z

Was chatting with @cpojer, and we were discussing how if we remove Haste requires from RN, we can actually rid ourselves of the awful and unstable providesModuleNodeModules option in the packager and in jest-haste-map (at least in OSS). Now that React is use in RN with a flat bundle (thanks @bvaughn!), and fbjs Haste requires were removed long ago, this seems like it might be a good move.

This would change Facebook's internal RN development workflow for anyone that works on RN core, but wouldn't change it for product engineers at Facebook, theoretically. The @providesModule pragmas are still here -- so if a FB engineer says in product code, require('View'), things should still work. But in OSS, we can turn off the providesModuleNodeModules option and just treat RN like a regular npm dependency.

Marking this as WIP -- as I haven't vetted this super well yet. Curious what others thoughts are.

skevy · 2017-05-25T22:02:04Z

Oh, also, here's the script I used to do this. Maybe not the prettiest thing ever, but it gets the job done.

https://gist.github.com/skevy/c323242673fb2c124269fd53b607d878

skevy · 2017-05-25T22:03:48Z

Libraries/Alert/Alert.js

+const NativeModules = require('../BatchedBridge/NativeModules');
+const Platform = require('../Utilities/Platform');

 import type { AlertType, AlertButtonStyle } from 'AlertIOS';


Just realized that I didn't modify the import type statements. Not sure if this is necessary, since flow in this repo is working in haste mode, but maybe I should change them for both consistency as well as so that that module mode isn't required for people consuming RN with Flow.

eslint-bot · 2017-05-25T22:08:34Z

Libraries/Alert/Alert.js

-const Platform = require('Platform');
+const AlertIOS = require('./AlertIOS');
+const NativeModules = require('../BatchedBridge/NativeModules');
+const Platform = require('../Utilities/Platform');


../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:34Z

Libraries/Components/Button.js

-const React = require('React');
+const ColorPropType = require('../StyleSheet/ColorPropType');
+const Platform = require('../Utilities/Platform');
+const React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:34Z

Libraries/Components/Button.js

-const View = require('View');
+const StyleSheet = require('../StyleSheet/StyleSheet');
+const Text = require('../Text/Text');
+const TouchableNativeFeedback = require('./Touchable/TouchableNativeFeedback');


./Touchable/TouchableNativeFeedback Required module not found

eslint-bot · 2017-05-25T22:08:34Z

Libraries/Components/Keyboard/KeyboardAvoidingView.js

-const Platform = require('Platform');
+const Keyboard = require('./Keyboard');
+const LayoutAnimation = require('../../LayoutAnimation/LayoutAnimation');
+const Platform = require('../../Utilities/Platform');


../../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:34Z

Libraries/Animated/src/AnimatedImplementation.js

+var InteractionManager = require('../../Interaction/InteractionManager');
+var Interpolation = require('./Interpolation');
+var NativeAnimatedHelper = require('./NativeAnimatedHelper');
+var React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Components/TextInput/TextInput.js

+const DocumentSelectionState = require('../../vendor/document/selection/DocumentSelectionState');
+const EventEmitter = require('../../EventEmitter/EventEmitter');
+const NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+const Platform = require('../../Utilities/Platform');


../../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Components/ProgressViewIOS/ProgressViewIOS.ios.js

-var Image = require('Image');
-var NativeMethodsMixin = require('NativeMethodsMixin');
-var React = require('React');
+var Image = require('../../Image/Image');


../../Image/Image Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Components/ToolbarAndroid/ToolbarAndroid.android.js

-var UIManager = require('UIManager');
-var ViewPropTypes = require('ViewPropTypes');
-var ColorPropType = require('ColorPropType');
+var ReactNativeViewAttributes = require('../View/ReactNativeViewAttributes');


no-unused-vars: 'ReactNativeViewAttributes' is assigned a value but never used.

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Modal/Modal.js

+const AppContainer = require('../ReactNative/AppContainer');
+const I18nManager = require('../ReactNative/I18nManager');
+const Platform = require('../Utilities/Platform');
+const React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/CameraRoll/CameraRoll.js


-const {PropTypes, checkPropTypes} = require('React');
-const RCTCameraRollManager = require('NativeModules').CameraRollManager;
+const {PropTypes, checkPropTypes} = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Components/View/View.js

-const Platform = require('Platform');
+const NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+const NativeModules = require('../../BatchedBridge/NativeModules');
+const Platform = require('../../Utilities/Platform');


../../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:35Z

Libraries/Inspector/InspectorPanel.js

+const ElementProperties = require('./ElementProperties');
+const NetworkOverlay = require('./NetworkOverlay');
+const PerformanceOverlay = require('./PerformanceOverlay');
+const React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Components/Touchable/TouchableWithoutFeedback.js

-const EdgeInsetsPropType = require('EdgeInsetsPropType');
-const React = require('React');
+const EdgeInsetsPropType = require('../../StyleSheet/EdgeInsetsPropType');
+const React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Components/ScrollView/ScrollView.js

+const Animated = require('../../Animated/src/Animated');
+const ColorPropType = require('../../StyleSheet/ColorPropType');
+const EdgeInsetsPropType = require('../../StyleSheet/EdgeInsetsPropType');
+const Platform = require('../../Utilities/Platform');


../../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Components/AppleTV/TVViewPropTypes.js

 */
 'use strict';
-var PropTypes = require('React').PropTypes;
+var PropTypes = require('../../react-native/React').PropTypes;


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Image/nativeImageSource.js

 'use strict';

-const Platform = require('Platform');
+const Platform = require('../Utilities/Platform');


../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Components/Slider/Slider.js

+var NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+var ReactNativeViewAttributes = require('../View/ReactNativeViewAttributes');
+var Platform = require('../../Utilities/Platform');
+var React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:36Z

Libraries/Components/UnimplementedViews/UnimplementedView.js


-var React = require('React');
-var StyleSheet = require('StyleSheet');
+var React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:37Z

Libraries/Components/ActivityIndicator/ActivityIndicator.js

+const ColorPropType = require('../../StyleSheet/ColorPropType');
+const NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+const Platform = require('../../Utilities/Platform');
+const React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:37Z

Libraries/Components/Navigation/NavigatorIOS.ios.js

+var ReactNative = require('../../Renderer/src/renderers/native/ReactNative');
+var StaticContainer = require('../StaticContainer.react');
+var StyleSheet = require('../../StyleSheet/StyleSheet');
+var TVEventHandler = require('../AppleTV/TVEventHandler');


../AppleTV/TVEventHandler Required module not found

eslint-bot · 2017-05-25T22:08:37Z

Libraries/Inspector/NetworkOverlay.js

-const WebSocketInterceptor = require('WebSocketInterceptor');
-const XHRInterceptor = require('XHRInterceptor');
+const ListView = require('../Lists/ListView/ListView');
+const React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:37Z

Libraries/Inspector/StyleInspector.js

-var StyleSheet = require('StyleSheet');
-var Text = require('Text');
-var View = require('View');
+var React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:37Z

Libraries/Animated/src/Animated.js

-var Text = require('Text');
-var View = require('View');
+var AnimatedImplementation = require('./AnimatedImplementation');
+var Image = require('../../Image/Image');


../../Image/Image Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/Experimental/Incremental.js

-const InteractionManager = require('InteractionManager');
-const React = require('React');
+const InteractionManager = require('../Interaction/InteractionManager');
+const React = require('../react-native/React');


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/StyleSheet/LayoutPropTypes.js

 'use strict';

-var ReactPropTypes = require('React').PropTypes;
+var ReactPropTypes = require('../react-native/React').PropTypes;


../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/Components/SegmentedControlIOS/SegmentedControlIOS.ios.js

-var NativeMethodsMixin = require('NativeMethodsMixin');
-var React = require('React');
+var NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+var React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/Components/TextInput/TextInput.js

+const EventEmitter = require('../../EventEmitter/EventEmitter');
+const NativeMethodsMixin = require('../../Renderer/src/renderers/native/NativeMethodsMixin');
+const Platform = require('../../Utilities/Platform');
+const React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/Lists/ListView/ListView.js

-var Platform = require('Platform');
-var React = require('React');
+var ListViewDataSource = require('./ListViewDataSource');
+var Platform = require('../../Utilities/Platform');


../../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:38Z

Libraries/Components/Button.js

-const Platform = require('Platform');
-const React = require('React');
+const ColorPropType = require('../StyleSheet/ColorPropType');
+const Platform = require('../Utilities/Platform');


../Utilities/Platform Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Utilities/Dimensions.js

-var RCTDeviceEventEmitter = require('RCTDeviceEventEmitter');
+var DeviceInfo = require('./DeviceInfo');
+var EventEmitter = require('../EventEmitter/EventEmitter');
+var Platform = require('./Platform');


./Platform Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Components/StatusBar/StatusBar.js

 'use strict';

-const React = require('React');
+const React = require('../../react-native/React');


../../react-native/React Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Utilities/HMRClient.js

 'use strict';

-const Platform = require('Platform');
+const Platform = require('./Platform');


./Platform Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Components/Picker/Picker.js

-var Platform = require('Platform');
-var React = require('React');
+var ColorPropType = require('../../StyleSheet/ColorPropType');
+var PickerIOS = require('./PickerIOS');


./PickerIOS Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Utilities/HMRClient.js

@@ -73,7 +73,7 @@ Error: ${e.message}`
    };
    activeWS.onmessage = ({data}) => {
      // Moving to top gives errors due to NativeModules not being initialized


./HMRLoadingView Required module not found

eslint-bot · 2017-05-25T22:08:39Z

Libraries/Components/Picker/Picker.js

-var React = require('React');
+var ColorPropType = require('../../StyleSheet/ColorPropType');
+var PickerIOS = require('./PickerIOS');
+var PickerAndroid = require('./PickerAndroid');


./PickerAndroid Required module not found

eslint-bot · 2017-05-25T22:08:40Z

Libraries/Components/Keyboard/KeyboardAvoidingView.js

+const Platform = require('../../Utilities/Platform');
 const PropTypes = require('prop-types');
-const React = require('React');
+const React = require('../../react-native/React');


../../react-native/React Required module not found

ericvicenti · 2017-05-25T23:32:25Z

This approach looks great to me!

Clearly the lint bot needs to be reconfigured as well

davidaurelio · 2017-05-26T09:37:44Z

Please sync up with @rozele how this is going to work with react-native-windows. They are currently overriding core modules using haste, e.g. Text.windows.js

rozele · 2017-05-26T13:19:33Z

Thanks @davidaurelio. @skevy for third-party plugin platforms like Microsoft/react-native-windows, we need the ability to override core modules. Throughout the react-native codebase, there are conditionals that allow differentiated native behaviors for iOS and Android, e.g. https://github.com/facebook/react-native/blob/master/Libraries/Components/ScrollView/ScrollView.js#L583-L593. We need to be able to also add in conditions for native behaviors of plugin platforms.

I'd be as happy as anyone to see providesModuleNodeModule go away, but we need to replace it with something equally flexible.

skevy · 2017-05-26T15:03:51Z

@rozele seems reasonable! I'll give it some thought. Thanks for letting me know!

skevy · 2017-05-31T17:06:48Z

Gonna close this for now.

The path forward is:

Figure out a sane way to handle new React Native platforms without relying on Haste (this is a big project that requires some careful thought -- I can open up a new issue on the metro-bundler repo when that exists and we can discuss this further)
Perhaps in the meantime, we can add a pre-publish step that strips haste from the JS in libraries, and publish two folders in the NPM release -- Libraries and Libraries-Haste. I'll open up a new PR that outlines my proposal here.

Thanks for the feedback everyone -- we'll do this eventually!

vovkasm · 2018-01-14T10:35:17Z

But now... platforms easily handled without Haste...

    import { Platform } from 'react-native'

    const Button = Platform.select({
        ios: () => require('./button-ios'),
        android: () => require('./button-android')
    })

    export default Button

Remove Haste from Libraries/ and convert to relative requires

91003a7

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels May 25, 2017

skevy commented May 25, 2017

View reviewed changes

eslint-bot reviewed May 25, 2017

View reviewed changes

davidaurelio added GH Review: needs-revision and removed GH Review: review-needed labels May 26, 2017

shergin added the JavaScript label May 26, 2017

skevy closed this May 31, 2017

rdy mentioned this pull request Mar 27, 2018

Revisting: Remove Haste from Libraries/ and convert to relative requires to shrink bundle sizes #18598

Closed

Conversation

skevy commented May 25, 2017

Uh oh!

skevy commented May 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericvicenti commented May 25, 2017