[createReactClass, TimerMixin] remove createReactClass and TImerMixin from TimerTest.js #21748
Conversation
facebook-github-bot
added
the
CLA Signed
| _interval = -1; | ||
| _timeoutID: ?TimeoutID = null; | ||
| // $FlowFixMe | ||
| _intervalId: ?IntervalID = -1; |
There was a problem hiding this comment.
// $FlowFixMe
_intervalId: ?IntervalID = -1;I've had issues with Flow here and on the contructor.displayName
because null or undefined is
incompatible with IntervalID,
and number is incompatible with IntervalID
There was a problem hiding this comment.
I commented below how we can fix this.
| {/* $FlowFixMe(>=0.54.0 site=react_native_fb,react_native_oss) This | ||
| * comment suppresses an error found when Flow v0.54 was deployed. | ||
| * To see the error delete this comment and run Flow. */ | ||
| this.constructor.displayName + ': \n'} |
There was a problem hiding this comment.
IIRC, the displayName is added on to our components in a babel transform. Flow doesn't know anything about that, so that's probably why it's raising this error.
I think there're three fixes:
- Change
this.constructor.displayNametothis.constructor.name. (Both should be equivalent). - Redundantly define the
displayNamestatic property in this class. - Hardcode 'TimerTest' instead of
this.constructor.displayName.
Your call. 😄
|
|
||
| _incrementInterval() { | ||
| if (this.state.count > 3) { | ||
| throw new Error('interval incremented past end.'); | ||
| } | ||
| if (this.state.count === 3) { | ||
| this.clearInterval(this._interval); | ||
| clearInterval(this._intervalId); |
There was a problem hiding this comment.
The signature of clearInterval is this:
function clearInterval(interval?: IntervalID) {So clearInterval expects an argument that is either an IntervalID, undefined, or not present at all. You're passing in a value that's ?IntervalID, which is equivalent to IntervalID, undefined, or null. (The bottom line is that clearInterval does not accept null as an argument). Therefore, If you guard this function call with a null check, your flow error should go away.
if (this.state.count === 3) {
if (this._intervalId != null) {
clearInterval(this._intervalId);
}
}| _interval = -1; | ||
| _timeoutID: ?TimeoutID = null; | ||
| // $FlowFixMe | ||
| _intervalId: ?IntervalID = -1; |
There was a problem hiding this comment.
I commented below how we can fix this.
| @@ -11,98 +11,106 @@ | |||
| 'use strict'; | |||
|
|
|||
| const React = require('react'); | |||
| const createReactClass = require('create-react-class'); | |||
| const ReactNative = require('react-native'); | |||
| /* $FlowFixMe(>=0.54.0 site=react_native_oss) This comment suppresses an error | |||
There was a problem hiding this comment.
Let's get rid of all $FlowFixMes in this file.
| }, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| container: { |
There was a problem hiding this comment.
Let's get rid of:
TimersTest.displayName = 'TimersTest';If we want to define static properties on TimerTest, we should do it using the static keyword inside the TimerTest class. That way, flow will be able to analyze the static property.
There was a problem hiding this comment.
RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Thanks again @RSNara |
) Summary: Related to facebook#21485 (comment) Remove createReactClass and TimerMixin from IntegrationTests/TimersTest.js Pull Request resolved: facebook#21748 Reviewed By: TheSavior Differential Revision: D10366418 Pulled By: RSNara fbshipit-source-id: f7e9a1b62a17cd23374997f99dbfe239172b614f
Related to #21485 (comment)
Remove createReactClass and TimerMixin from IntegrationTests/TimersTest.js
Test Plan:
Flow tests succeed
Release Notes:
[GENERAL] [ENHANCEMENT] [IntegrationTests/TimersTest.js] Remove createReactClass and TimerMixin