Skip to content

Add timer usage for es6 #5401

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add timer usage for es6 #5401

wants to merge 1 commit into from
+3 −1

Conversation

sunnylqm
Copy link
Contributor

No description provided.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @christopherdro, @vjeux and @sahrens to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 19, 2016
@brentvatne
Copy link
Collaborator

While this is technically correct I'm not a fan of manually doing this on every component that needs setTimeout or requestAnimationFrame or setInterval. At Exponent we use react-mixin on es6 classes for TimerMixin: https://github.com/brigand/react-mixin - cc @ide

@Kureev
Copy link
Contributor

Kureev commented Mar 20, 2016

@sunnylqm can you please update your PR according to the changes @brentvatne proposed?

@facebook-github-bot
Copy link
Contributor

@sunnylqm updated the pull request.

@sunnylqm
Copy link
Contributor Author

@Kureev updated

@Kureev
Copy link
Contributor

Kureev commented Mar 22, 2016

Thanks, @sunnylqm!

Can you also rebase this branch by the current master?

@sunnylqm
Copy link
Contributor Author

@Kureev I think I've rebased...?

@Kureev
Copy link
Contributor

Kureev commented Mar 22, 2016

@sunnylqm

image

@Kureev
Copy link
Contributor

Kureev commented Mar 22, 2016

@sunnylqm if you'll have problems with the merge, it's also possible to close this PR and create a new one from the latest master (don't forget to make a reference to this one).

@sunnylqm
Copy link
Contributor Author

@Kureev OK, done!

@Kureev
Copy link
Contributor

Kureev commented Mar 22, 2016

@vjeux @sahrens @sahrens what would be a next action on this PR? It's currently up to date with the master and seems to be a valuable doc change. Can we merge it in?

@brentvatne
Copy link
Collaborator

@sunnylqm - there is a typo here: "exsiting" should be "existing"

@facebook-github-bot
Copy link
Contributor

@sunnylqm updated the pull request.

@brentvatne
Copy link
Collaborator

I would change...

However, TimerMixin and any other mixins can not be used on ES6. You need to use some helper like react-mixin to migarate existing mixins to ES6 classes.

to...

Keep in mind that if you use ES6 classes for your React components there is no built-in API for mixins. To use TimerMixin with ES6 classes, we recommend react-mixin.

@facebook-github-bot
Copy link
Contributor

@sunnylqm updated the pull request.

@sunnylqm
add note for mixins on es6
a7d5bd9 
i#Update Timers.md
@facebook-github-bot
Copy link
Contributor

@sunnylqm updated the pull request.

@Kureev
Copy link
Contributor

Kureev commented Mar 26, 2016

@brentvatne done!

@satya164
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 7289222 Mar 26, 2016
@sunnylqm sunnylqm deleted the patch-11 branch March 26, 2016 14:03
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
@sunnylqm @zebulgar
Add timer usage for es6
d190fdf 
Summary: Closes facebook#5401

Differential Revision: D3102058

fb-gh-sync-id: 953a69a2a50b7177a4b0ee01972f5d32d5476f5c
fbshipit-source-id: 953a69a2a50b7177a4b0ee01972f5d32d5476f5c
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sunnylqm @facebook-github-bot @brentvatne @Kureev @satya164