[createReactClass] Converting ProgressbarAndroid #21600

gugakatsi · 2018-10-09T11:27:37Z

[Android] [RNTester] converted ProgressbarAndroid.js to class
but you own build is failing

relates to #21581

facebook-github-bot · 2018-10-09T11:27:43Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

analysis-bot

Code analysis results:

eslint found some issues.

analysis-bot · 2018-10-09T11:28:55Z

RNTester/js/ProgressBarAndroidExample.android.js

-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {
+  displayName = 'MovingBar';
+  _intervalID = IntervalID ? IntervalID : null;


no-undef: 'IntervalID' is not defined.

analysis-bot · 2018-10-09T11:28:55Z

RNTester/js/ProgressBarAndroidExample.android.js

-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {
+  displayName = 'MovingBar';
+  _intervalID = IntervalID ? IntervalID : null;


no-undef: 'IntervalID' is not defined.

analysis-bot · 2018-10-09T11:28:55Z

RNTester/js/ProgressBarAndroidExample.android.js

-  },
+  state = {
+    progress: 0,
+  }


prettier/prettier: Insert ;

pull-bot · 2018-10-09T11:29:01Z

	Warnings
⚠️	📋 Test Plan - This PR appears to be missing a Test Plan.
⚠️	📋 Release Notes - This PR appears to be missing Release Notes.

Generated by 🚫 dangerJS

elicwhite · 2018-10-09T18:05:33Z

RNTester/js/ProgressBarAndroidExample.android.js

-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {
+  displayName = 'MovingBar';
+  _intervalID = IntervalID ? IntervalID : null;


I don't think this is valid. I think it should be

_intervalID = (null: ?IntervalID)

@gugakatsi IntervalID is a type (and not a JavaScript object). Since JavaScript doesn't understand Flow types, we can't assign Flow types to JavaScript properties, which is why this is invalid. Also, what we really want is what @TheSavior suggested.

elicwhite · 2018-10-09T18:05:52Z

RNTester/js/ProgressBarAndroidExample.android.js

-var MovingBar = createReactClass({
-  displayName: 'MovingBar',
-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {


Can you make this an exact type so that people don't pass additional props? {||}

@gugakatsi It looks like MovingBar just forwards the props to ProgressBar. So, MovingBarProps should probably be defined like this:

// Get the props of ProgressBar type ProgressBarProps = React.ElementConfig<typeof ProgressBar> // 1. Remove the `progress` prop from ProgressBarProp // 2. Whenever you do C = $Diff<A, B>, C won't be read only (regardless of if A is read only). So, we have to explicitly call $ReadOnly on the result from 1. type MovingBarProps = $ReadOnly<$Diff<ProgressBarProps, { progress: $ElementType<ProgressBarProps, 'progress'> }>>

RSNara

This is almost ready to land, but we should fix a few things.

RSNara · 2018-10-09T18:30:52Z

RNTester/js/ProgressBarAndroidExample.android.js

-var MovingBar = createReactClass({
-  displayName: 'MovingBar',
-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {


@gugakatsi It looks like MovingBar just forwards the props to ProgressBar. So, MovingBarProps should probably be defined like this:

// Get the props of ProgressBar type ProgressBarProps = React.ElementConfig<typeof ProgressBar> // 1. Remove the `progress` prop from ProgressBarProp // 2. Whenever you do C = $Diff<A, B>, C won't be read only (regardless of if A is read only). So, we have to explicitly call $ReadOnly on the result from 1. type MovingBarProps = $ReadOnly<$Diff<ProgressBarProps, { progress: $ElementType<ProgressBarProps, 'progress'> }>>

RSNara · 2018-10-09T18:33:42Z

RNTester/js/ProgressBarAndroidExample.android.js

-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {
+  displayName = 'MovingBar';
+  _intervalID = IntervalID ? IntervalID : null;


@gugakatsi IntervalID is a type (and not a JavaScript object). Since JavaScript doesn't understand Flow types, we can't assign Flow types to JavaScript properties, which is why this is invalid. Also, what we really want is what @TheSavior suggested.

RSNara · 2018-10-09T18:35:31Z

RNTester/js/ProgressBarAndroidExample.android.js

-  displayName: 'MovingBar',
-  _intervalID: (null: ?IntervalID),
+class MovingBar extends React.Component<{}> {
+  displayName = 'MovingBar';


I think this should be:

static displayName = 'MovingBar';

But since we have a babel plugin to automatically generate the displayName from components, we don't really need this. So, you should get rid of this line entirely.

@RSNara thanks for helping me out , will fix pull request soon , learned a lot.

gugakatsi · 2018-10-09T18:50:26Z

@RSNara @TheSavior thanks for helping and understanding , your replies helped me lot , will fix it in few hrs.

RSNara · 2018-10-19T21:44:03Z

@gugakatsi, ping. It's been some time since there was activity on this PR. Do you still have time to work on this PR?

elicwhite · 2018-10-19T22:22:31Z

Also, we can't import and make the fixes to this PR ourselves because you haven't signed the CLA.

RSNara · 2018-10-29T19:11:20Z

Due to inactivity, I'm closing this PR.

Summary: Related to #21581 This PR was already opened here #21600 but seems to be inactive. Remove createReactClass from ProgressBarAndroidExample. - `yarn run flow` && `yarn run flow-check-android` succeed. - RNTester app ProgressBarAndroidExample on Android. [GENERAL] [ENHANCEMENT] [ProgressBarAndroidExample.android.js] - rm createReactClass Pull Request resolved: #21874 Reviewed By: TheSavior Differential Revision: D12827689 Pulled By: RSNara fbshipit-source-id: 46c70ea67dddf5d928fe936a28ef4a0a929d127f

Summary: Related to #21581 This PR was already opened here facebook/react-native#21600 but seems to be inactive. Remove createReactClass from ProgressBarAndroidExample. - `yarn run flow` && `yarn run flow-check-android` succeed. - RNTester app ProgressBarAndroidExample on Android. [GENERAL] [ENHANCEMENT] [ProgressBarAndroidExample.android.js] - rm createReactClass Pull Request resolved: facebook/react-native#21874 Reviewed By: TheSavior Differential Revision: D12827689 Pulled By: RSNara fbshipit-source-id: 46c70ea67dddf5d928fe936a28ef4a0a929d127f

Summary: Related to facebook#21581 This PR was already opened here facebook#21600 but seems to be inactive. Remove createReactClass from ProgressBarAndroidExample. - `yarn run flow` && `yarn run flow-check-android` succeed. - RNTester app ProgressBarAndroidExample on Android. [GENERAL] [ENHANCEMENT] [ProgressBarAndroidExample.android.js] - rm createReactClass Pull Request resolved: facebook#21874 Reviewed By: TheSavior Differential Revision: D12827689 Pulled By: RSNara fbshipit-source-id: 46c70ea67dddf5d928fe936a28ef4a0a929d127f

converted to class

b72a1ff

analysis-bot suggested changes Oct 9, 2018

View reviewed changes

elicwhite reviewed Oct 9, 2018

View reviewed changes

elicwhite changed the title ~~converted to class issue #21581~~ [CreateReactClass] Converting ProgressbarAndroid Oct 9, 2018

RSNara mentioned this pull request Oct 9, 2018

[createReactClass] Remove createReactClass from thr ProgressBarAndroidExample.android.js #21604

Closed

3 tasks

RSNara requested changes Oct 9, 2018

View reviewed changes

RSNara changed the title ~~[CreateReactClass] Converting ProgressbarAndroid~~ [createReactClass] Converting ProgressbarAndroid Oct 9, 2018

react-native-bot added Platform: Android Android applications. 🔶Components labels Oct 9, 2018

exced mentioned this pull request Oct 20, 2018

[createReactClass] Remove createReactClass from ProgressBarAndroidExample #21874

Closed

andrew-pyle mentioned this pull request Oct 26, 2018

Convert components off of createReactClass #21581

Closed

15 tasks

RSNara closed this Oct 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[createReactClass] Converting ProgressbarAndroid #21600

[createReactClass] Converting ProgressbarAndroid #21600

gugakatsi commented Oct 9, 2018 •

edited by elicwhite

Loading

facebook-github-bot commented Oct 9, 2018

analysis-bot left a comment

analysis-bot Oct 9, 2018

analysis-bot Oct 9, 2018

analysis-bot Oct 9, 2018

pull-bot commented Oct 9, 2018

elicwhite Oct 9, 2018

RSNara Oct 9, 2018

elicwhite Oct 9, 2018

RSNara Oct 9, 2018 •

edited

Loading

RSNara left a comment

RSNara Oct 9, 2018 •

edited

Loading

RSNara Oct 9, 2018

RSNara Oct 9, 2018

gugakatsi Oct 9, 2018

gugakatsi commented Oct 9, 2018

RSNara commented Oct 19, 2018 •

edited

Loading

elicwhite commented Oct 19, 2018

RSNara commented Oct 29, 2018

[createReactClass] Converting ProgressbarAndroid #21600

[createReactClass] Converting ProgressbarAndroid #21600

Conversation

gugakatsi commented Oct 9, 2018 • edited by elicwhite Loading

facebook-github-bot commented Oct 9, 2018

analysis-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pull-bot commented Oct 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RSNara Oct 9, 2018 • edited Loading

Choose a reason for hiding this comment

RSNara left a comment

Choose a reason for hiding this comment

RSNara Oct 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gugakatsi commented Oct 9, 2018

RSNara commented Oct 19, 2018 • edited Loading

elicwhite commented Oct 19, 2018

RSNara commented Oct 29, 2018

gugakatsi commented Oct 9, 2018 •

edited by elicwhite

Loading

RSNara Oct 9, 2018 •

edited

Loading

RSNara Oct 9, 2018 •

edited

Loading

RSNara commented Oct 19, 2018 •

edited

Loading