Skip to content

Commit 38863ab

Browse files
committed
perf(scroll): prevent unneeded forced synchronous layout on update
In the case where new items are added or the component is otherwise rerendered, this avoids an expensive relayout in getScroll() where the result would not change. In a production application I see as much as 9ms on unneeded synchronous layout when this happens.
1 parent 0e3799e commit 38863ab

2 files changed

Lines changed: 38 additions & 14 deletions

File tree

react-list.es6

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ module.exports = class ReactList extends Component {
8080
const {from, size} = this.constrain(initialIndex, 0, itemsPerRow, props);
8181
this.state = {from, size, itemsPerRow};
8282
this.cache = {};
83+
this.cachedScroll = null;
8384
this.prevPrevState = {};
8485
this.unstable = false;
8586
this.updateCounter = 0;
@@ -91,8 +92,8 @@ module.exports = class ReactList extends Component {
9192
}
9293

9394
componentDidMount() {
94-
this.updateFrame = this.updateFrame.bind(this);
95-
window.addEventListener('resize', this.updateFrame);
95+
this.updateFrameAndClearCache = this.updateFrameAndClearCache.bind(this);
96+
window.addEventListener('resize', this.updateFrameAndClearCache);
9697
this.updateFrame(this.scrollTo.bind(this, this.props.initialIndex));
9798
}
9899

@@ -123,8 +124,8 @@ module.exports = class ReactList extends Component {
123124
}
124125

125126
componentWillUnmount() {
126-
window.removeEventListener('resize', this.updateFrame);
127-
this.scrollParent.removeEventListener('scroll', this.updateFrame, PASSIVE);
127+
window.removeEventListener('resize', this.updateFrameAndClearCache);
128+
this.scrollParent.removeEventListener('scroll', this.updateFrameAndClearCache, PASSIVE);
128129
this.scrollParent.removeEventListener('mousewheel', NOOP, PASSIVE);
129130
}
130131

@@ -154,6 +155,8 @@ module.exports = class ReactList extends Component {
154155
}
155156

156157
getScroll() {
158+
// Cache scroll position as this causes a forced synchronous layout.
159+
if (typeof this.cachedScroll === 'number') return this.cachedScroll;
157160
const {scrollParent} = this;
158161
const {axis} = this.props;
159162
const scrollKey = SCROLL_START_KEYS[axis];
@@ -166,7 +169,8 @@ module.exports = class ReactList extends Component {
166169
const max = this.getScrollSize() - this.getViewportSize();
167170
const scroll = Math.max(0, Math.min(actual, max));
168171
const el = this.getEl();
169-
return this.getOffset(scrollParent) + scroll - this.getOffset(el);
172+
this.cachedScroll = this.getOffset(scrollParent) + scroll - this.getOffset(el);
173+
return this.cachedScroll;
170174
}
171175

172176
setScroll(offset) {
@@ -245,6 +249,12 @@ module.exports = class ReactList extends Component {
245249
return {itemSize, itemsPerRow};
246250
}
247251

252+
// Called by 'scroll' and 'resize' events, clears scroll position cache.
253+
updateFrameAndClearCache(cb) {
254+
this.cachedScroll = null;
255+
return this.updateFrame(cb);
256+
}
257+
248258
updateFrame(cb) {
249259
this.updateScrollParent();
250260
if (typeof cb != 'function') cb = NOOP;
@@ -260,10 +270,12 @@ module.exports = class ReactList extends Component {
260270
this.scrollParent = this.getScrollParent();
261271
if (prev === this.scrollParent) return;
262272
if (prev) {
263-
prev.removeEventListener('scroll', this.updateFrame);
273+
prev.removeEventListener('scroll', this.updateFrameAndClearCache);
264274
prev.removeEventListener('mousewheel', NOOP);
265275
}
266-
this.scrollParent.addEventListener('scroll', this.updateFrame, PASSIVE);
276+
this.scrollParent.addEventListener('scroll', this.updateFrameAndClearCache, PASSIVE);
277+
// You have to attach mousewheel listener to the scrollable element.
278+
// Just an empty listener. After that onscroll events will be fired synchronously.
267279
this.scrollParent.addEventListener('mousewheel', NOOP, PASSIVE);
268280
}
269281

react-list.js

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@
131131

132132
_this.state = { from: from, size: size, itemsPerRow: itemsPerRow };
133133
_this.cache = {};
134+
_this.cachedScroll = null;
134135
_this.prevPrevState = {};
135136
_this.unstable = false;
136137
_this.updateCounter = 0;
@@ -150,8 +151,8 @@
150151
}, {
151152
key: 'componentDidMount',
152153
value: function componentDidMount() {
153-
this.updateFrame = this.updateFrame.bind(this);
154-
window.addEventListener('resize', this.updateFrame);
154+
this.updateFrameAndClearCache = this.updateFrameAndClearCache.bind(this);
155+
window.addEventListener('resize', this.updateFrameAndClearCache);
155156
this.updateFrame(this.scrollTo.bind(this, this.props.initialIndex));
156157
}
157158
}, {
@@ -186,8 +187,8 @@
186187
}, {
187188
key: 'componentWillUnmount',
188189
value: function componentWillUnmount() {
189-
window.removeEventListener('resize', this.updateFrame);
190-
this.scrollParent.removeEventListener('scroll', this.updateFrame, PASSIVE);
190+
window.removeEventListener('resize', this.updateFrameAndClearCache);
191+
this.scrollParent.removeEventListener('scroll', this.updateFrameAndClearCache, PASSIVE);
191192
this.scrollParent.removeEventListener('mousewheel', NOOP, PASSIVE);
192193
}
193194
}, {
@@ -228,6 +229,8 @@
228229
}, {
229230
key: 'getScroll',
230231
value: function getScroll() {
232+
// Cache scroll position as this causes a forced synchronous layout.
233+
if (typeof this.cachedScroll === 'number') return this.cachedScroll;
231234
var scrollParent = this.scrollParent;
232235
var axis = this.props.axis;
233236

@@ -240,7 +243,8 @@
240243
var max = this.getScrollSize() - this.getViewportSize();
241244
var scroll = Math.max(0, Math.min(actual, max));
242245
var el = this.getEl();
243-
return this.getOffset(scrollParent) + scroll - this.getOffset(el);
246+
this.cachedScroll = this.getOffset(scrollParent) + scroll - this.getOffset(el);
247+
return this.cachedScroll;
244248
}
245249
}, {
246250
key: 'setScroll',
@@ -331,6 +335,12 @@
331335
++itemsPerRow;
332336
}return { itemSize: itemSize, itemsPerRow: itemsPerRow };
333337
}
338+
}, {
339+
key: 'updateFrameAndClearCache',
340+
value: function updateFrameAndClearCache(cb) {
341+
this.cachedScroll = null;
342+
return this.updateFrame(cb);
343+
}
334344
}, {
335345
key: 'updateFrame',
336346
value: function updateFrame(cb) {
@@ -352,10 +362,12 @@
352362
this.scrollParent = this.getScrollParent();
353363
if (prev === this.scrollParent) return;
354364
if (prev) {
355-
prev.removeEventListener('scroll', this.updateFrame);
365+
prev.removeEventListener('scroll', this.updateFrameAndClearCache);
356366
prev.removeEventListener('mousewheel', NOOP);
357367
}
358-
this.scrollParent.addEventListener('scroll', this.updateFrame, PASSIVE);
368+
this.scrollParent.addEventListener('scroll', this.updateFrameAndClearCache, PASSIVE);
369+
// You have to attach mousewheel listener to the scrollable element.
370+
// Just an empty listener. After that onscroll events will be fired synchronously.
359371
this.scrollParent.addEventListener('mousewheel', NOOP, PASSIVE);
360372
}
361373
}, {

0 commit comments

Comments
 (0)