Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flow-strict] Flow strict-local in InternalScrollViewTypes #22158

Closed
wants to merge 2 commits into from
Closed

[flow-strict] Flow strict-local in InternalScrollViewTypes #22158

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 5, 2018

Issue in focus: #22100

All instances of any were replaced with their respective Flow types.
getScrollResponder() returned this and after going through the official flow documentation for functions , I removed the return type altogether. If this was a wrong approach, please correct me.
Any changes needed will be implemented at the earliest of my convenience (^_^)

Test Plan:

  • yarn prettier
  • npm run lint
  • npm run flow
  • npm run flow-check-android

Release Notes:

[GENERAL][ENHANCEMENT][InternalScrollViewType.js] - Flow Type
[GENERAL][FIXED][deepFreezeAndThrowOnMutationInDev-test.js] - Lint Error

Replaced all the instances of any in InternalScrollViewTypes with the
respective flow type
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. ✅pr adds tests labels Nov 5, 2018
@ghost ghost changed the title Flow strict InternalScrollViewTypes [flow-strict] Flow strict InternalScrollViewTypes Nov 5, 2018
@ghost ghost changed the title [flow-strict] Flow strict InternalScrollViewTypes [flow-strict] Flow strict-local in InternalScrollViewTypes Nov 5, 2018
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 7, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Nov 8, 2018
@ghost
Copy link
Author

ghost commented Nov 13, 2018

@TheSavior
Just wondering which of the internal test failed.
Would make rectifying the error that slipped in here (^_^)

@elicwhite
Copy link
Member

Sorry for not getting back to this, thanks for pinging me.

These are the flow errors:

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

               v-----------------------------------------------------------
  242|         scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  243|           ReactNative.findNodeHandle(this._submitButton),
  244|           INPUT_FIELD_OFFSET,
  245|           true,
  246|         );
       --------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `this._scrollViewRef.getScrollResponder().scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

             v------------------
  365|       this._scrollViewRef
  366|         .getScrollResponder()
  367|         .scrollResponderScrollNativeHandleToKeyboard(
     :
  370|           preventNegativeScrollOffset,
  371|         );
       --------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `this._scrollViewRef.getScrollResponder().scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

             v------------------
  334|       this._scrollViewRef
  335|         .getScrollResponder()
  336|         .scrollResponderScrollNativeHandleToKeyboard(
     :
  339|           preventNegativeScrollOffset,
  340|         );
       --------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

             v-----------------------------------------------------------
  280|       scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  281|         ReactNative.findNodeHandle(this._inputRef),
  282|         /* $FlowFixMe This issue was
     :
  285|         true,
  286|       );
       ------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

           v-----------------------------------------------------------
  475|     scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  476|       inputNodeHandle,
  477|       Platform.select({
     :
  481|       true,
  482|     );
       ----^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollView.getScrollResponder().scrollResponderScrollTo` because property `scrollResponderScrollTo` is missing in undefined [1].

                   v--------------------------------------------------------
  511|             scrollView.getScrollResponder().scrollResponderScrollTo({
  512|               x: 0,
  513|               y: Math.min(questionY, maxScrollOffset),
  514|               animated: true,
  515|             });
       -------------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

           v-----------------------------------------------------------
  200|     scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  201|       inputNodeHandle,
  202|       Platform.select({
     :
  206|       true,
  207|     );
       ----^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

                 v-----------------------------------------------------------
  234|           scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  235|             inputNodeHandle,
  236|             BASE_SCROLL_OFFSET,
  237|             true,
  238|           ),
       ----------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot create `MyComponent` element because undefined [1] is incompatible with `InternalScrollViewType` [2] in property `scrollResponder`.

                     v-------------------
  363|               <MyComponent
  364|                 errorCodes={this._errorCodes}
  365|                 questions={questions}
     :
  367|                 onNext={this._onNextPage}
  368|               />
       ---------------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]
  ProductCode.js:
  19|   scrollResponder: ScrollView,
                         ^^^^^^^^^^ [2]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot create `MyComponent` element because undefined [1] is incompatible with `InternalScrollViewType` [2] in property `scrollResponder`.

                     v--------------------------
  401|               <MyComponent
  402|                 contextContent={redacted}
  403|                 contextContentStyle={this.getContextContentStyle()}
     :
  409|                 onNext={this._onNextPage}
  410|               />
       ---------------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]
  ProductCode.js:
  34|   scrollResponder: ScrollView,
                         ^^^^^^^^^^ [2]

ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` with `ReactNative.findNodeHandle(...)` bound to `nodeHandle` because null or undefined [1] is incompatible with number [2].

  ProductCode.js:
  77|             ReactNative.findNodeHandle(this._component),
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

References:
  react-native/Libraries/Renderer/shims/ReactNativeTypes.js:133:43
  133|   findNodeHandle(componentOrHandle: any): ?number,
                                                 ^^^^^^^ [1]
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:34:17
  34|     nodeHandle: number,
                      ^^^^^^ [2]

ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` with `ReactNative.findNodeHandle(...)` bound to `nodeHandle` because null or undefined [1] is incompatible with number [2].

  ProductCode.js:
  91|               ReactNative.findNodeHandle(this._component),
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

References:
  react-native/Libraries/Renderer/shims/ReactNativeTypes.js:133:43
  133|   findNodeHandle(componentOrHandle: any): ?number,
                                                 ^^^^^^^ [1]
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:34:17
  34|     nodeHandle: number,
                      ^^^^^^ [2]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

                    v-----------------------------------------------------------
  91|               scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  92|                 ReactNative.findNodeHandle(inputRef),
  93|                 /* $FlowFixMe This issue
    :
  96|                 true,
  97|               );
      --------------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollResponder.scrollResponderScrollNativeHandleToKeyboard` because property `scrollResponderScrollNativeHandleToKeyboard` is missing in undefined [1].

            v-----------------------------------------------------------
  74|       scrollResponder.scrollResponderScrollNativeHandleToKeyboard(
  75|         inputNodeHandle,
  76|         44 + 20 + 10,
  77|         true,
  78|       );
      ------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `scrollView.getScrollResponder().scrollResponderScrollTo` because property `scrollResponderScrollTo` is missing in undefined [1].

                     v--------------------------------------------------------
  318|               scrollView.getScrollResponder().scrollResponderScrollTo({
  319|                 x: 0,
  320|                 y: Math.min(inputY, maxScrollOffset),
  321|                 animated: true,
  322|               });
       ---------------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

Error ---------------------------------------------------------------------------
ProductCode.js:
Cannot call `this._scrollViewRef.getScrollResponder().scrollTo` because property `scrollTo` is missing in undefined [1].

               v--------------------------------------------------
  202|         this._scrollViewRef.getScrollResponder().scrollTo({
  203|           y: Math.max(
  204|             this.state.commentsBottom -
     :
  210|           ),
  211|         });
       ---------^

References:
  react-native/Libraries/Components/ScrollView/InternalScrollViewType.js:29:23
  29|   getScrollResponder() {}
                            ^ [1]

@ghost
Copy link
Author

ghost commented Nov 14, 2018

@TheSavior,
After a bit of digging, I found getScrollResapender returns a ScrollView object from this line but the ScrollView is based on createReactClass whose alternate I think is still being worked out by the guys there.
I'm not sure strict-local can be taken care of, as of now.

@elicwhite
Copy link
Member

Thanks for digging. It isn't critical that we jump to strict-local. I mostly just care that we make this better than it was. :)

What do you think about changing your PR to only include the changes you are confident in without using strict-local. Then we can merge the PR and improve it even further later when other files it depends on are improved.

Added return type of getScrollResponder as any.
@ghost
Copy link
Author

ghost commented Nov 18, 2018

@TheSavior,
Sorry for not getting back earlier.
I have dialed back the return type of getScrollResponder to any so now it can return the ScrollView object which takes care of all the internal errors as all of them were related to this single change.
The changes were added in a02fcb8 but I haven't squashed the two commits as one. Please let me know if this is necessary and I'll do it in a jiffy.
Thank you (^_^)

@ghost
Copy link
Author

ghost commented Nov 18, 2018

#22301 Removes InternalScrollViewTypes.js completely so this PR becomes redundant after that. Should I go ahead and close this @TheSavior ?

@ghost ghost closed this Nov 23, 2018
@hramos hramos added Merged This PR has been merged. and removed Import Failed labels Mar 8, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: ScrollView Component: View Flow Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants