Flow strict ScrollView; get rid of InternalScrollViewType (#22301)

Summary: Relates to #22100. I left 2 `$FlowFixMe`s as I was not sure how to handle generic `React.Element<>` and which native props can I pass to ScrollView (would be cool to document it once we got proper types there). I also got rid of `InternalScrollViewType` because we have better typings in original `ScrollView` now. Pull Request resolved: #22301 Reviewed By: TheSavior Differential Revision: D13103990 Pulled By: RSNara fbshipit-source-id: 9664ee9d7f570b00992215e10901e5317f24fe5c
facebook · Mar 8, 2019 · fe533a4 · fe533a4
1 parent 205171c
commit fe533a4
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 90 deletions.
diff --git a/Libraries/Components/ScrollView/InternalScrollViewType.js b/Libraries/Components/ScrollView/InternalScrollViewType.js
diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js
@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  *
  * @format
- * @flow
+ * @flow strict-local
  */
 
 'use strict';
@@ -18,7 +18,6 @@ const ScrollResponder = require('ScrollResponder');
 const ScrollViewStickyHeader = require('ScrollViewStickyHeader');
 const StyleSheet = require('StyleSheet');
 const View = require('View');
-const InternalScrollViewType = require('InternalScrollViewType');
 
 const dismissKeyboard = require('dismissKeyboard');
 const flattenStyle = require('flattenStyle');
@@ -27,7 +26,7 @@ const processDecelerationRate = require('processDecelerationRate');
 const requireNativeComponent = require('requireNativeComponent');
 const resolveAssetSource = require('resolveAssetSource');
 
-import type {PressEvent} from 'CoreEventTypes';
+import type {PressEvent, ScrollEvent, LayoutEvent} from 'CoreEventTypes';
 import type {EdgeInsetsProp} from 'EdgeInsetsPropType';
 import type {NativeMethodsMixinType} from 'ReactNativeTypes';
 import type {ViewStyleProp} from 'StyleSheet';
@@ -227,7 +226,7 @@ type IOSProps = $ReadOnly<{|
    * Fires when the scroll view scrolls to top after the status bar has been tapped
    * @platform ios
    */
-  onScrollToTop?: ?Function,
+  onScrollToTop?: (event: ScrollEvent) => void,
   /**
    * When true, shows a horizontal scroll indicator.
    * The default value is true.
@@ -410,33 +409,30 @@ export type Props = $ReadOnly<{|
    *   - `false`, deprecated, use 'never' instead
    *   - `true`, deprecated, use 'always' instead
    */
-  /* $FlowFixMe(>=0.92.0 site=react_native_fb) This comment suppresses an error
-   * found when Flow v0.92 was deployed. To see the error, delete this comment
-   * and run Flow. */
   keyboardShouldPersistTaps?: ?('always' | 'never' | 'handled' | false | true),
   /**
    * Called when the momentum scroll starts (scroll which occurs as the ScrollView glides to a stop).
    */
-  onMomentumScrollBegin?: ?Function,
+  onMomentumScrollBegin?: (event: ScrollEvent) => void,
   /**
    * Called when the momentum scroll ends (scroll which occurs as the ScrollView glides to a stop).
    */
-  onMomentumScrollEnd?: ?Function,
+  onMomentumScrollEnd?: (event: ScrollEvent) => void,
 
   /**
    * Fires at most once per frame during scrolling. The frequency of the
    * events can be controlled using the `scrollEventThrottle` prop.
    */
-  onScroll?: ?Function,
+  onScroll?: (event: ScrollEvent) => void,
   /**
    * Called when the user begins to drag the scroll view.
    */
-  onScrollBeginDrag?: ?Function,
+  onScrollBeginDrag?: (event: ScrollEvent) => void,
   /**
    * Called when the user stops dragging the scroll view and it either stops
    * or begins to glide.
    */
-  onScrollEndDrag?: ?Function,
+  onScrollEndDrag?: (event: ScrollEvent) => void,
   /**
    * Called when scrollable content view of the ScrollView changes.
    *
@@ -446,7 +442,7 @@ export type Props = $ReadOnly<{|
    * It's implemented using onLayout handler attached to the content container
    * which this ScrollView renders.
    */
-  onContentSizeChange?: ?Function,
+  onContentSizeChange?: (contentWidth: number, contentHeight: number) => void,
   onKeyboardDidShow?: (event: PressEvent) => void,
   /**
    * When true, the scroll view stops on multiples of the scroll view's size
@@ -525,6 +521,7 @@ export type Props = $ReadOnly<{|
    *
    * See [RefreshControl](docs/refreshcontrol.html).
    */
+  // $FlowFixMe - how to handle generic type without existential opereator?
   refreshControl?: ?React.Element<any>,
   children?: React.Node,
 |}>;
@@ -598,8 +595,8 @@ class ScrollView extends React.Component<Props, State> {
    */
   _scrollResponder: typeof ScrollResponder.Mixin = createScrollResponder(this);
 
-  constructor(...args) {
-    super(...args);
+  constructor(props: Props) {
+    super(props);
 
     /**
      * Part 2: Removing ScrollResponder.Mixin
@@ -617,6 +614,7 @@ class ScrollView extends React.Component<Props, State> {
         typeof ScrollResponder.Mixin[key] === 'function' &&
         key.startsWith('scrollResponder')
       ) {
+        // $FlowFixMe - dynamically adding properties to a class
         (this: any)[key] = ScrollResponder.Mixin[key].bind(this);
       }
     }
@@ -630,6 +628,7 @@ class ScrollView extends React.Component<Props, State> {
     Object.keys(ScrollResponder.Mixin)
       .filter(key => typeof ScrollResponder.Mixin[key] !== 'function')
       .forEach(key => {
+        // $FlowFixMe - dynamically adding properties to a class
         (this: any)[key] = ScrollResponder.Mixin[key];
       });
   }
@@ -685,7 +684,7 @@ class ScrollView extends React.Component<Props, State> {
     }
   }
 
-  setNativeProps(props: Object) {
+  setNativeProps(props: {[key: string]: mixed}) {
     this._scrollViewRef && this._scrollViewRef.setNativeProps(props);
   }
 
@@ -699,17 +698,18 @@ class ScrollView extends React.Component<Props, State> {
     ...typeof ScrollView,
     ...typeof ScrollResponder.Mixin,
   } {
+    // $FlowFixMe - overriding type to include ScrollResponder.Mixin
     return ((this: any): {
       ...typeof ScrollView,
       ...typeof ScrollResponder.Mixin,
     });
   }
 
-  getScrollableNode(): any {
+  getScrollableNode(): ?number {
     return ReactNative.findNodeHandle(this._scrollViewRef);
   }
 
-  getInnerViewNode(): any {
+  getInnerViewNode(): ?number {
     return ReactNative.findNodeHandle(this._innerViewRef);
   }
 
@@ -725,17 +725,23 @@ class ScrollView extends React.Component<Props, State> {
    * This is deprecated due to ambiguity (y before x), and SHOULD NOT BE USED.
    */
   scrollTo(
-    y?: number | {x?: number, y?: number, animated?: boolean},
-    x?: number,
-    animated?: boolean,
+    options?: {x?: number, y?: number, animated?: boolean} | number,
+    deprecatedX?: number,
+    deprecatedAnimated?: boolean,
   ) {
-    if (typeof y === 'number') {
+    let x, y, animated;
+    if (typeof options === 'number') {
       console.warn(
         '`scrollTo(y, x, animated)` is deprecated. Use `scrollTo({x: 5, y: 5, ' +
           'animated: true})` instead.',
       );
-    } else {
-      ({x, y, animated} = y || {});
+      y = options;
+      x = deprecatedX;
+      animated = deprecatedAnimated;
+    } else if (options) {
+      y = options.y;
+      x = options.x;
+      animated = options.animated;
     }
     this._scrollResponder.scrollResponderScrollTo({
       x: x || 0,
@@ -832,7 +838,7 @@ class ScrollView extends React.Component<Props, State> {
     }
   }
 
-  _handleScroll = (e: Object) => {
+  _handleScroll = (e: ScrollEvent) => {
     if (__DEV__) {
       if (
         this.props.onScroll &&
@@ -859,16 +865,16 @@ class ScrollView extends React.Component<Props, State> {
     this._scrollResponder.scrollResponderHandleScroll(e);
   };
 
-  _handleLayout = (e: Object) => {
-    if (this.props.invertStickyHeaders) {
+  _handleLayout = (e: LayoutEvent) => {
+    if (this.props.invertStickyHeaders === true) {
       this.setState({layoutHeight: e.nativeEvent.layout.height});
     }
     if (this.props.onLayout) {
       this.props.onLayout(e);
     }
   };
 
-  _handleContentOnLayout = (e: Object) => {
+  _handleContentOnLayout = (e: LayoutEvent) => {
     const {width, height} = e.nativeEvent.layout;
     this.props.onContentSizeChange &&
       this.props.onContentSizeChange(width, height);
@@ -888,7 +894,7 @@ class ScrollView extends React.Component<Props, State> {
     let ScrollViewClass;
     let ScrollContentContainerViewClass;
     if (Platform.OS === 'android') {
-      if (this.props.horizontal) {
+      if (this.props.horizontal === true) {
         ScrollViewClass = AndroidHorizontalScrollView;
         ScrollContentContainerViewClass = AndroidHorizontalScrollContentView;
       } else {
@@ -911,10 +917,10 @@ class ScrollView extends React.Component<Props, State> {
     );
 
     const contentContainerStyle = [
-      this.props.horizontal && styles.contentContainerHorizontal,
+      this.props.horizontal === true && styles.contentContainerHorizontal,
       this.props.contentContainerStyle,
     ];
-    if (__DEV__ && this.props.style) {
+    if (__DEV__ && this.props.style !== undefined) {
       const style = flattenStyle(this.props.style);
       const childLayoutProps = ['alignItems', 'justifyContent'].filter(
         prop => style && style[prop] !== undefined,
@@ -966,7 +972,7 @@ class ScrollView extends React.Component<Props, State> {
     }
 
     const hasStickyHeaders =
-      stickyHeaderIndices && stickyHeaderIndices.length > 0;
+      Array.isArray(stickyHeaderIndices) && stickyHeaderIndices.length > 0;
 
     const contentContainer = (
       <ScrollContentContainerViewClass
@@ -999,14 +1005,15 @@ class ScrollView extends React.Component<Props, State> {
     const DEPRECATED_sendUpdatedChildFrames = !!this.props
       .DEPRECATED_sendUpdatedChildFrames;
 
-    const baseStyle = this.props.horizontal
-      ? styles.baseHorizontal
-      : styles.baseVertical;
+    const baseStyle =
+      this.props.horizontal === true
+        ? styles.baseHorizontal
+        : styles.baseVertical;
     const props = {
       ...this.props,
       alwaysBounceHorizontal,
       alwaysBounceVertical,
-      style: ([baseStyle, this.props.style]: ?Array<any>),
+      style: [baseStyle, this.props.style],
       // Override the onContentSizeChange from props, since this event can
       // bubble up from TextInputs
       onContentSizeChange: null,
@@ -1057,12 +1064,12 @@ class ScrollView extends React.Component<Props, State> {
       pagingEnabled: Platform.select({
         // on iOS, pagingEnabled must be set to false to have snapToInterval / snapToOffsets work
         ios:
-          this.props.pagingEnabled &&
+          this.props.pagingEnabled === true &&
           this.props.snapToInterval == null &&
           this.props.snapToOffsets == null,
         // on Android, pagingEnabled must be set to true to have snapToInterval / snapToOffsets work
         android:
-          this.props.pagingEnabled ||
+          this.props.pagingEnabled === true ||
           this.props.snapToInterval != null ||
           this.props.snapToOffsets != null,
       }),
@@ -1115,10 +1122,6 @@ class ScrollView extends React.Component<Props, State> {
   }
 }
 
-const TypedScrollView = ((ScrollView: any): Class<
-  InternalScrollViewType<Props>,
->);
-
 const styles = StyleSheet.create({
   baseVertical: {
     flexGrow: 1,
@@ -1137,4 +1140,4 @@ const styles = StyleSheet.create({
   },
 });
 
-module.exports = TypedScrollView;
+module.exports = ScrollView;
diff --git a/Libraries/Experimental/WindowedListView.js b/Libraries/Experimental/WindowedListView.js
@@ -208,6 +208,7 @@ class WindowedListView extends React.Component<Props, State> {
     return (
       this._scrollRef &&
       this._scrollRef.getScrollResponder &&
+      // $FlowFixMe - it actually returns ScrollView & ScrollResponder.Mixin
       this._scrollRef.getScrollResponder()
     );
   }

diff --git a/Libraries/Types/CoreEventTypes.js b/Libraries/Types/CoreEventTypes.js
@@ -125,7 +125,8 @@ export type ScrollEvent = SyntheticEvent<
       y: number,
       x: number,
     |}>,
-    zoomScale: number,
+    zoomScale?: number,
+    responderIgnoreScroll?: boolean,
   |}>,
 >;