[createReactClass] remove createReactClass from ScrollViewTestModule.js

[sopranolinist]

Related to issue #21581
Removed createReactClass from ReactAndroid/src/androidTest/js/ScrollViewTestModule.js

Test Plan:

• npm run prettier
• npm run flow-check-ios
• npm run flow-check-android

Release Notes:

[GENERAL] [ENHANCEMENT] [ScrollViewTestModule.js] - rm createReactClass

[RSNara]

The PR looks mostly good, but I think we should change a few things. 🤔

[RSNara]

nit: We should just use class properties for this.

class ScrollViewTestApp extends React.Component {
  scrollView = React.createRef();
  //...

This way, we won't have to implement our own constructor.

[RSNara]

React components are instantiated with arguments (the first of which is just props). To avoid potential problems, we should just forward them all to React.Component constructor.

class ScrollViewTestApp extends React.Component {
  constructor(...args) {
    super(...args);
    // ...
  }
  // ...
}

[RSNara]

Classes that extend React.Component do not call getInitialState to get their initial state. With the current configuration, this.state will be null inside the render() method, which will raise an exception given the current implementation of that method. Instead of implementing getInitialState, you should create the state and assign it to this.state inside the constructor. Since we support class property assignment, we should just do this:

class ScrollViewTestApp extends React.Component {
  scrollView = React.createRef();
  state = getInitialState();

[RSNara]

All functions need to meet two conditions before you should consider binding them (i.e: using this syntax):

1. You're using this inside the function.
2. You're passing the function by reference to component's prop or as a callback to some function.

In this case, since we're not using this inside onScroll, we don't actually need to bind this function. In fact, since we're not using this, we could leave onScroll as is (i.e: Implemented outside the ScrollViewTestApp class).

Let's just move this back to where it was before.

[RSNara]

Let's move this back to where it was before.

[RSNara]

I don't think there's a reason for these functions to be attached to the component instance. So, after we move the functions out of the ScrollViewTestApp class, lets change this to:

  <Item
    // ...
    onScroll={onScroll}
    onScrollBeginDrag={onScrollBeginDrag}
    onScrollEndDrag={onScrollEndDrag}
   // ...

[RSNara]

class HorizontalScrollViewTestApp extends React.Component {
  scrollView = React.createRef();

[RSNara]

There's no need to duplicate this function.

[RSNara]

class HorizontalScrollViewTestApp extends React.Component {
  scrollView = React.createRef();
  state = getInitialState()

[RSNara]

  <ScrollView
     horizontal={true}
     onScroll={onScroll}

[RSNara]

This file should also be flow-typed. But it's up to you if you want to tackle that here, or if you want to tackle that at all. 😛

[RSNara]

I should have caught this earlier, but when you create a ref using React.createRef, you have to access what it's referring to using ref.current. So, this should be something like:

const { scrollView: { current: scrollView } } = this;
if (scrollView == null) {
  return;
}
scrollView.scrollTo(destY, destX);

But don't worry about it. 😄 I'll make the fix on my end.

[RSNara]

Looks good! Thanks for making all those changes. 😁

[facebook-github-bot]

RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

@JenLindsay merged commit bbfff90 into facebook:master.