Skip to content

Commit

Permalink
Remove TimerMixin from ListView (facebook#21488)
Browse files Browse the repository at this point in the history
Summary:
Related to facebook#21485

- Remove TimerMixin from ListView

- All flow tests succeed.
- RNTester: <ListView> 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 think a load test would be more relevant:
- Update props multiple times and scroll

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

Differential Revision: D10219088

Pulled By: RSNara

fbshipit-source-id: 946e4fc1319324c5bf4947a2060b18bebb6fc493
  • Loading branch information
exced authored and facebook-github-bot committed Oct 5, 2018
1 parent bd8b942 commit 8ceb158
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions Libraries/Lists/ListView/ListView.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const RCTScrollViewManager = require('NativeModules').ScrollViewManager;
const ScrollView = require('ScrollView');
const ScrollResponder = require('ScrollResponder');
const StaticRenderer = require('StaticRenderer');
const TimerMixin = require('react-timer-mixin');
const View = require('View');
const cloneReferencedElement = require('react-clone-referenced-element');
const createReactClass = require('create-react-class');
Expand Down Expand Up @@ -216,14 +215,15 @@ type Props = $ReadOnly<{|

const ListView = createReactClass({
displayName: 'ListView',
_rafIds: ([]: Array<AnimationFrameID>),
_childFrames: ([]: Array<Object>),
_sentEndForContentLength: (null: ?number),
_scrollComponent: (null: ?React.ElementRef<typeof ScrollView>),
_prevRenderedRowsCount: 0,
_visibleRows: ({}: Object),
scrollProperties: ({}: Object),

mixins: [ScrollResponder.Mixin, TimerMixin],
mixins: [ScrollResponder.Mixin],

statics: {
DataSource: ListViewDataSource,
Expand Down Expand Up @@ -347,16 +347,23 @@ const ListView = createReactClass({
contentLength: null,
offset: 0,
};

this._rafIds = [];
this._childFrames = [];
this._visibleRows = {};
this._prevRenderedRowsCount = 0;
this._sentEndForContentLength = null;
},

componentWillUnmount: function() {
this._rafIds.forEach(cancelAnimationFrame);
this._rafIds = [];
},

componentDidMount: function() {
// do this in animation frame until componentDidMount actually runs after
// the component is laid out
this.requestAnimationFrame(() => {
this._requestAnimationFrame(() => {
this._measureAndUpdateScrollProps();
});
},
Expand Down Expand Up @@ -384,7 +391,7 @@ const ListView = createReactClass({
},

componentDidUpdate: function() {
this.requestAnimationFrame(() => {
this._requestAnimationFrame(() => {
this._measureAndUpdateScrollProps();
});
},
Expand Down Expand Up @@ -535,6 +542,14 @@ const ListView = createReactClass({
* Private methods
*/

_requestAnimationFrame: function(fn: () => void): void {
const rafId = requestAnimationFrame(() => {
this._rafIds.splice(this._rafIds.indexOf(rafId));
fn();
});
this._rafIds.push(rafId);
},

_measureAndUpdateScrollProps: function() {
const scrollComponent = this.getScrollResponder();
if (!scrollComponent || !scrollComponent.getInnerViewNode) {
Expand Down

0 comments on commit 8ceb158

Please sign in to comment.