necolas / react-native-web

Cross-platform React UI packages
https://necolas.github.io/react-native-web
MIT License
21.6k stars 1.79k forks source link

Clean up Flow errors #465

Closed peggyrayzis closed 7 years ago

peggyrayzis commented 7 years ago

Hey there! πŸ™‹ Thank you so much for your work on this project, we're using react-native-web & loving it for our new PWA at Major League Soccer.

We've been experiencing some Flow errors and would like to start a conversation on how to fix them. Once we determine a solution, I'll work on the PR.

In our project, we're aliasing RN --> RNW in our webpack build to take advantage of the cross-platform benefits. We also add module.name_mapper='\(react-native\)' -> 'react-native-web' to our .flowconfig to avoid Required module not found errors.

What is the current behavior?

When I run flow in my project, I get 40+ errors from RNW. To silence these, I have to add .*/node_modules/react-native-web/* to the ignore section of my .flowconfig. When I ignore all of RNW, then I get Required module not found errors from Flow because the module name mapper isn't able to find RNW in my project. In order to silence these errors, I have to add declare module 'react-native' { declare var exports: any; } to my project's types file, as suggested in https://github.com/facebook/flow/issues/869. This is problematic because I can't take advantage of the typing currently in RNW.

Here are the errors I get when I run flow in my project without explicitly ignoring RNW in my config ``` node_modules/react-native-web/src/apis/AppRegistry/renderApplication.js:16 16: RootComponent: Component, ^^^^^^^^^ Component. Application of polymorphic type needs . (Can use `*` for inferrable ones) node_modules/react-native-web/src/apis/AppRegistry/renderApplication.js:28 28: export function getApplication(RootComponent: Component, initialProps: Object): Object { ^^^^^^^^^ Component. Application of polymorphic type needs . (Can use `*` for inferrable ones) node_modules/react-native-web/src/apis/Dimensions/index.js:26 26: height: win.innerHeight, ^^^^^^^^^^^ property `innerHeight`. Property not found in 26: height: win.innerHeight, ^^^ object literal node_modules/react-native-web/src/apis/Dimensions/index.js:28 28: width: win.innerWidth ^^^^^^^^^^ property `innerWidth`. Property not found in 28: width: win.innerWidth ^^^ object literal node_modules/react-native-web/src/apis/InteractionManager/index.js:34 34: clearInteractionHandle(handle) { ^^^^^^ parameter `handle`. Missing annotation node_modules/react-native-web/src/apis/NetInfo/index.js:52 52: fetch(): Promise { ^^^^^^^ Promise. Application of polymorphic type needs . (Can use `*` for inferrable ones) node_modules/react-native-web/src/apis/NetInfo/index.js:101 101: fetch(): Promise { ^^^^^^^ Promise. Application of polymorphic type needs . (Can use `*` for inferrable ones) node_modules/react-native-web/src/apis/StyleSheet/flattenStyle.js:22 22: function flattenStyle(style) { ^^^^^ parameter `style`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:69 69: constructor(props) { ^^^^^ parameter `props`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:71 71: this._debouncedOnScrollEnd = debounce(this._handleScrollEnd, 100); ^^^^^^^^^^^^^^^^^^^^^ property `_debouncedOnScrollEnd`. Property not found in 71: this._debouncedOnScrollEnd = debounce(this._handleScrollEnd, 100); ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:72 72: this._state = { isScrolling: false }; ^^^^^^ property `_state`. Property not found in 72: this._state = { isScrolling: false }; ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:75 75: _handlePreventableScrollEvent = handler => { ^^^^^^^ parameter `handler`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:76 76: return e => { ^ parameter `e`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:87 87: _handleScroll = e => { ^ parameter `e`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:91 91: this._debouncedOnScrollEnd(e); ^^^^^^^^^^^^^^^^^^^^^ property `_debouncedOnScrollEnd`. Property not found in 91: this._debouncedOnScrollEnd(e); ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:92 92: if (this._state.isScrolling) { ^^^^^^ property `_state`. Property not found in 92: if (this._state.isScrolling) { ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:103 103: _handleScrollStart(e) { ^ parameter `e`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:104 104: this._state.isScrolling = true; ^^^^^^ property `_state`. Property not found in 104: this._state.isScrolling = true; ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:105 105: this._state.scrollLastTick = Date.now(); ^^^^^^ property `_state`. Property not found in 105: this._state.scrollLastTick = Date.now(); ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:108 108: _handleScrollTick(e) { ^ parameter `e`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:110 110: this._state.scrollLastTick = Date.now(); ^^^^^^ property `_state`. Property not found in 110: this._state.scrollLastTick = Date.now(); ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:116 116: _handleScrollEnd(e) { ^ parameter `e`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:118 118: this._state.isScrolling = false; ^^^^^^ property `_state`. Property not found in 118: this._state.isScrolling = false; ^^^^ ScrollViewBase node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:124 124: _shouldEmitScrollEvent(lastTick, eventThrottle) { ^^^^^^^^ parameter `lastTick`. Missing annotation node_modules/react-native-web/src/components/ScrollView/ScrollViewBase.js:124 124: _shouldEmitScrollEvent(lastTick, eventThrottle) { ^^^^^^^^^^^^^ parameter `eventThrottle`. Missing annotation node_modules/react-native-web/src/components/ScrollView/index.js:55 55: getScrollResponder(): Component { ^^^^^^^^^ Component. Application of polymorphic type needs . (Can use `*` for inferrable ones) node_modules/react-native-web/src/components/Touchable/TouchableWithoutFeedback.js:36 36: const TouchableWithoutFeedback = React.createClass({ ^ call of method `createClass`. Expected a React PropType instead of 82: pressRetentionOffset: EdgeInsetsPropType, ^^^^^^^^^^^^^^^^^^ object type node_modules/react-native-web/src/components/Touchable/TouchableWithoutFeedback.js:36 36: const TouchableWithoutFeedback = React.createClass({ ^ call of method `createClass`. Expected a React PropType instead of 91: hitSlop: EdgeInsetsPropType ^^^^^^^^^^^^^^^^^^ object type node_modules/react-native-web/src/components/Touchable/TouchableWithoutFeedback.js:162 162: const child = React.Children.only(this.props.children); ^^^^^^^^ property `children`. Property not found in 162: const child = React.Children.only(this.props.children); ^^^^^^^^^^ propTypes of React component node_modules/react-native-web/src/modules/NativeMethodsMixin/index.js:33 33: measure(callback) { ^^^^^^^^ parameter `callback`. Missing annotation node_modules/react-native-web/src/modules/NativeMethodsMixin/index.js:52 52: measureInWindow(callback) { ^^^^^^^^ parameter `callback`. Missing annotation node_modules/react-native-web/src/modules/NativeMethodsMixin/index.js:59 59: measureLayout(relativeToNativeNode, onSuccess, onFail) { ^^^^^^^^^^^^^^^^^^^^ parameter `relativeToNativeNode`. Missing annotation node_modules/react-native-web/src/modules/NativeMethodsMixin/index.js:59 59: measureLayout(relativeToNativeNode, onSuccess, onFail) { ^^^^^^^^^ parameter `onSuccess`. Missing annotation node_modules/react-native-web/src/modules/NativeMethodsMixin/index.js:59 59: measureLayout(relativeToNativeNode, onSuccess, onFail) { ^^^^^^ parameter `onFail`. Missing annotation node_modules/react-native-web/src/modules/applyLayout/index.js:38 38: const applyLayout = Component => { ^^^^^^^^^ parameter `Component`. Missing annotation node_modules/react-native-web/src/modules/applyNativeMethods/index.js:10 10: const applyNativeMethods = Component => { ^^^^^^^^^ parameter `Component`. Missing annotation node_modules/react-native-web/src/propTypes/StyleSheetPropType.js:8 8: module.exports = function StyleSheetPropType(shape) { ^^^^^ parameter `shape`. Missing annotation node_modules/react-native-web/src/propTypes/StyleSheetPropType.js:13 13: return function(props, propName, componentName, location?) { ^^^^^ parameter `props`. Missing annotation node_modules/react-native-web/src/propTypes/StyleSheetPropType.js:13 13: return function(props, propName, componentName, location?) { ^^^^^^^^ parameter `propName`. Missing annotation node_modules/react-native-web/src/propTypes/StyleSheetPropType.js:13 13: return function(props, propName, componentName, location?) { ^^^^^^^^^^^^^ parameter `componentName`. Missing annotation node_modules/react-native-web/src/propTypes/StyleSheetPropType.js:13 13: return function(props, propName, componentName, location?) { ^^^^^^^^^ parameter `location`. Missing annotation ```

What is the expected behaviour?

Ideally, I would like to clean up these errors so I don't have to sacrifice type safety when using RNW.

Another suggestion would be to add a flow-typed libdef for RNW, but I don't know if it would help here. It appears that the module name mapper in .flowconfig is not smart enough to tell Flow to look at the stubbed RNW libdef in the flow-typed directory when I'm importing from RN. Also, I think writing a libdef would be extremely time-consuming - RN doesn't even have one to go off of since their project is typed internally.

Steps to reproduce Clone RNW, from the root run:

  1. npm i --global flow-bin
  2. flow init
  3. flow

to see all the errors currently in the project.

Environment (include versions)

OS: El Capitan 10.11.6 Device: Macbook Pro Browser: Chrome 58.0.3029.81 React Native for Web (version): 0.0.94 React (version): 15.4.2 Flow: 0.45.0

cc @kkemple

necolas commented 7 years ago

Hey, I haven't really been keeping on top of flow and using types. I'm definitely open to a PR and suggestions on how best to get it up to scratch.

lohmander commented 7 years ago

We'd really like to see this fixed too! :) Is this being actively worked on @peggyrayzis?

peggyrayzis commented 7 years ago

@lohmander Yes it is. πŸ˜€ I made #466 to start the process, but it still hasn't been merged yet.

necolas commented 7 years ago

Your PR was applied in be3c78f31794c3292f706718aaa893e09a1c40d6. Thanks for getting this started. Going to close this issue and we can continue the fixes as PRs :)

necolas commented 7 years ago

Latest master has no flow errors. Please let me know what else we can do

peggyrayzis commented 7 years ago

Wow, thank you so much for resolving these! I was planning on doing it but have been super busy with the launch of our React Native Web app at MLS.

Would you be open to adding a Travis script so the build fails on any Flow errors? I can take care of this tomorrow if you'd like.

necolas commented 7 years ago

Should already be taken care of. npm test now runs flow