Skip to content

Conversation

@exced
Copy link
Contributor

@exced exced commented Oct 15, 2018

Related to #21488
Disclaimer: I made this PR.

I think there's some requestAnimationFrame events that are not cleared on unmount because of bad use of splice method.

Test Plan:

  • All flow tests succeed.
  • RNTester: iOS (this change should only affect iOS because calculateChildFrames is iOS only)
    Show perf monitor, show ListView* screen, start scrolling. UI frame Rate is used at the beginning. When scrolling there is no drop in FPS rate.
  • TODO: I'll write a load test for ListView

Release Notes:

[GENERAL] [ENHANCEMENT] [ListView.js] - rm TimerMixin

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 15, 2018
Copy link
Contributor

@RSNara RSNara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@exced Can we use filter here as well?

@exced
Copy link
Contributor Author

exced commented Oct 15, 2018

@RSNara Yep for sure, I find it clearer !

Copy link
Contributor

@RSNara RSNara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick changes! 😁

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 15, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@react-native-bot
Copy link
Collaborator

@exced merged commit 70b5eb3 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Oct 16, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 16, 2018
kelset pushed a commit that referenced this pull request Oct 23, 2018
Summary:
Related to #21488
Disclaimer: I made this PR.

I think there's some requestAnimationFrame events that are not cleared on unmount because of bad use of `splice` method.

- All flow tests succeed.
- RNTester: iOS (this change should only affect iOS because calculateChildFrames is iOS only)
Show perf monitor, show ListView* screen, start scrolling. UI frame Rate is used at the beginning. When scrolling there is no drop in FPS rate.
- TODO: I'll write a load test for ListView

[GENERAL] [ENHANCEMENT] [ListView.js] - rm TimerMixin
Pull Request resolved: #21802

Differential Revision: D10391812

Pulled By: RSNara

fbshipit-source-id: 49f0b0a4641ec29bcb4cc04bd3bafb42b3842b69
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
Related to facebook#21488
Disclaimer: I made this PR.

I think there's some requestAnimationFrame events that are not cleared on unmount because of bad use of `splice` method.

- All flow tests succeed.
- RNTester: iOS (this change should only affect iOS because calculateChildFrames is iOS only)
Show perf monitor, show ListView* screen, start scrolling. UI frame Rate is used at the beginning. When scrolling there is no drop in FPS rate.
- TODO: I'll write a load test for ListView

[GENERAL] [ENHANCEMENT] [ListView.js] - rm TimerMixin
Pull Request resolved: facebook#21802

Differential Revision: D10391812

Pulled By: RSNara

fbshipit-source-id: 49f0b0a4641ec29bcb4cc04bd3bafb42b3842b69
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants