From 62a42252cedc0ffb2189ffdf7004ce1ae7710534 Mon Sep 17 00:00:00 2001
From: Joe Lencioni <joe.lencioni@airbnb.com>
Date: Fri, 6 Jul 2018 10:01:58 -0700
Subject: [PATCH] Defer `handleScroll` in `componentDidUpdate`

I was doing some profiling and I noticed that when Waypoint updates, we
call `this._handleScroll` synchronously, which eventually calls functions
that force layout. Since this happens during render, the layout is
immediately invalidated and then triggered again once rendering is
completed.

We avoid this problem in `componentDidMount` by deferring this work
until the next tick.

I think we should consider making two changes to Waypoint to help
address this:

1. Extend `React.PureComponent` instead of `React.Component`. In the
   place I saw this problem, we were rendering a waypoint with pure
   props, so the update in this case could be completely avoided by
   using PureComponent.

   ```jsx
   <Waypoint
     onEnter={this.onEnterWaypoint}
     scrollableAncestor={this.props.scrollableAncestor}
   />
   ```

2. Defer `this._handleScroll` in `componentDidUpdate` using
   `onNextTick`, like we do in `componentDidMount`. This will still
   require layout, but it will happen around the same time that the
   browser was going to do layout anyway, avoiding the thrash.

This commit implements the second of these two interventions.

Fixes #263
---
 src/waypoint.jsx | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/waypoint.jsx b/src/waypoint.jsx
index 0146444..f2ac7b6 100644
--- a/src/waypoint.jsx
+++ b/src/waypoint.jsx
@@ -42,9 +42,11 @@ export default class Waypoint extends React.Component {
     }
 
     // this._ref may occasionally not be set at this time. To help ensure that
-    // this works smoothly, we want to delay the initial execution until the
-    // next tick.
-    this.cancelInitialTimeout = onNextTick(() => {
+    // this works smoothly and to avoid layout thrashing, we want to delay the
+    // initial execution until the next tick.
+    this.cancelOnNextTick = onNextTick(() => {
+      this.cancelOnNextTick = null;
+
       // Berofe doing anything, we want to check that this._ref is avaliable in Waypoint
       ensureRefIsUsedByChild(this.props.children, this._ref);
 
@@ -87,8 +89,21 @@ export default class Waypoint extends React.Component {
       return;
     }
 
-    // The element may have moved.
-    this._handleScroll(null);
+    // The element may have moved, so we need to recompute its position on the
+    // page. This happens via handleScroll in a way that forces layout to be
+    // computed.
+    //
+    // We want this to be deferred to avoid forcing layout during render, which
+    // causes layout thrashing. And, if we already have this work enqueued, we
+    // can just wait for that to happen instead of enqueueing again.
+    if (this.cancelOnNextTick) {
+      return;
+    }
+
+    this.cancelOnNextTick = onNextTick(() => {
+      this.cancelOnNextTick = null;
+      this._handleScroll(null);
+    });
   }
 
   componentWillUnmount() {
@@ -103,8 +118,8 @@ export default class Waypoint extends React.Component {
       this.resizeEventListenerUnsubscribe();
     }
 
-    if (this.cancelInitialTimeout) {
-      this.cancelInitialTimeout();
+    if (this.cancelOnNextTick) {
+      this.cancelOnNextTick();
     }
   }