[createReactClass] remove createReactClass from TimerExample.js#21623
[createReactClass] remove createReactClass from TimerExample.js#21623peaonunes wants to merge 4 commits intofacebook:masterfrom
Conversation
facebook-github-bot
added
the
CLA Signed
RSNara
left a comment
RSNara
left a comment
There was a problem hiding this comment.
Almost ready to merge, but we should make a few changes.
| _rafId = (null: ?AnimationFrameID); | ||
| _intervalId = (null: ?IntervalID); | ||
| _immediateId = (null: ?Object); | ||
| _timerFn = (null: ?() => any); |
There was a problem hiding this comment.
Flow won't correctly infer the types of these props based on the types of what you're assigning to them. We should instead do:
_timeId: ?TimeoutID = null;
_rafId: ?AnimationFrameID = null;
_intervalId: ?IntervalID = null;
_immediateId? Object = null;
_timerFn: ?() => any = null;There was a problem hiding this comment.
Hmm, I see, I remember seeing the way I have done as a suggestion in one of the PRs.
Made your suggested changes on this 66a5b27!
| } | ||
|
|
||
| componentWillUnmount() { | ||
| componentWillUnmount = () => { |
There was a problem hiding this comment.
There's no need to bind component life-cycle hooks. Since React is responsible for calling these hooks, it can make sure that the value of this is correct inside them.
There was a problem hiding this comment.
You are completely right! Here is the change 18ff98b
analysis-bot
left a comment
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
eslintfound some issues.
|
|
||
| var React = require('react'); | ||
| var createReactClass = require('create-react-class'); | ||
| var ReactNative = require('react-native'); |
|
|
||
| var React = require('react'); | ||
| var createReactClass = require('create-react-class'); | ||
| var ReactNative = require('react-native'); |
There was a problem hiding this comment.
no-extra-semi: Unnecessary semicolon.
|
I am still not able to run tests with RNTester, @RSNara , how should we proceed? |
|
@peaonunes It's cool. I'll verify that this works. 😁 |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
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.
|
@peaonunes merged commit c96c93e into |
Summary: Relates to #21581 Removed `createdReactClass` from `TimerExample.js`. Pull Request resolved: facebook/react-native#21623 Reviewed By: hramos Differential Revision: D10341474 Pulled By: RSNara fbshipit-source-id: b4bf6e07fcf0355c89709809fe9a69e447b44e2f
Summary: Relates to facebook#21581 Removed `createdReactClass` from `TimerExample.js`. Pull Request resolved: facebook#21623 Reviewed By: hramos Differential Revision: D10341474 Pulled By: RSNara fbshipit-source-id: b4bf6e07fcf0355c89709809fe9a69e447b44e2f
5 participants
Relates to #21581
Removed
createdReactClassfromTimerExample.js.Test Plan:
Run examples on RNTester
I was expecting to play around and run the tests on RNTester, however I always get stuck while installing the dependencies (it seems I am stucked with this issue #21539. So none of the guides for Mac are working for me :(
Does anyone can help me validating the actual behaviour on RNTester or guide me on fixing my setup?
Release Notes:
[GENERAL] [ENHANCEMENT] [TimerExample.js] - Remove createReactClass