fix: avoid using the problematic beforeunload event (#344)

* fix: avoid using the problematic beforeunload event * Account for pageshow happenning after initialisation * Add tests for pageshow/pagehide handling * refactor: use page-lifecycle library * Remove this._hasSetScrollRestoration * simplify logic for saving/restoring scroll restoration * reset scrollRestoration before each test * Fix initialization of variable to happen before it is used * correct assertion * cover case of page initialisation hapenning after scroll ehavior initialisation * update comment * dedupe deps * format Co-authored-by: Jimmy Jia <tesrin@gmail.com>
taion · Mar 11, 2020 · 6a7a8b4 · 6a7a8b4
1 parent e4c9af4
commit 6a7a8b4
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 22 deletions.
diff --git a/package.json b/package.json
@@ -47,7 +47,8 @@
   "homepage": "https://github.com/taion/scroll-behavior#readme",
   "dependencies": {
     "dom-helpers": "^3.4.0",
-    "invariant": "^2.2.4"
+    "invariant": "^2.2.4",
+    "page-lifecycle": "^0.1.2"
   },
   "devDependencies": {
     "@4c/babel-preset": "^7.3.3",

diff --git a/src/index.js b/src/index.js
@@ -6,6 +6,7 @@ import scrollLeft from 'dom-helpers/query/scrollLeft';
 import scrollTop from 'dom-helpers/query/scrollTop';
 import requestAnimationFrame from 'dom-helpers/util/requestAnimationFrame';
 import invariant from 'invariant';
+import PageLifecycle from 'page-lifecycle/dist/lifecycle.es5';
 
 import { isMobileSafari } from './utils';
 
@@ -22,32 +23,27 @@ export default class ScrollBehavior {
     this._stateStorage = stateStorage;
     this._getCurrentLocation = getCurrentLocation;
     this._shouldUpdateScroll = shouldUpdateScroll;
+    this._oldScrollRestoration = null;
 
     // This helps avoid some jankiness in fighting against the browser's
     // default scroll behavior on `POP` transitions.
     /* istanbul ignore else: Travis browsers all support this */
-    if (
-      'scrollRestoration' in window.history &&
-      // Unfortunately, Safari on iOS freezes for 2-6s after the user swipes to
-      // navigate through history with scrollRestoration being 'manual', so we
-      // need to detect this browser and exclude it from the following code
-      // until this bug is fixed by Apple.
-      !isMobileSafari()
-    ) {
-      this._oldScrollRestoration = window.history.scrollRestoration;
-      try {
-        window.history.scrollRestoration = 'manual';
-
-        // Scroll restoration persists across page reloads. We want to reset
-        // this to the original value, so that we can let the browser handle
-        // restoring the initial scroll position on server-rendered pages.
-        on(window, 'beforeunload', this._restoreScrollRestoration);
-      } catch (e) {
-        this._oldScrollRestoration = null;
+    this._setScrollRestoration();
+
+    // Scroll restoration persists across page reloads. We want to reset
+    // this to the original value, so that we can let the browser handle
+    // restoring the initial scroll position on server-rendered pages.
+    PageLifecycle.addEventListener('statechange', ({ newState }) => {
+      if (
+        newState === 'terminated' ||
+        newState === 'frozen' ||
+        newState === 'discarded'
+      ) {
+        this._restoreScrollRestoration();
+      } else {
+        this._setScrollRestoration();
       }
-    } else {
-      this._oldScrollRestoration = null;
-    }
+    });
 
     this._saveWindowPositionHandle = null;
     this._checkWindowScrollHandle = null;
@@ -151,6 +147,28 @@ export default class ScrollBehavior {
     });
   }
 
+  _setScrollRestoration = () => {
+    if (this._oldScrollRestoration) {
+      // It's possible that we already set the scroll restoration
+      return;
+    }
+    if (
+      'scrollRestoration' in window.history &&
+      // Unfortunately, Safari on iOS freezes for 2-6s after the user swipes to
+      // navigate through history with scrollRestoration being 'manual', so we
+      // need to detect this browser and exclude it from the following code
+      // until this bug is fixed by Apple.
+      !isMobileSafari()
+    ) {
+      this._oldScrollRestoration = window.history.scrollRestoration;
+      try {
+        window.history.scrollRestoration = 'manual';
+      } catch (e) {
+        this._oldScrollRestoration = null;
+      }
+    }
+  };
+
   _restoreScrollRestoration = () => {
     /* istanbul ignore if: not supported by any browsers on Travis */
     if (this._oldScrollRestoration) {
@@ -159,6 +177,7 @@ export default class ScrollBehavior {
       } catch (e) {
         /* silence */
       }
+      this._oldScrollRestoration = null;
     }
   };
 

diff --git a/test/ScrollBehavior.test.js b/test/ScrollBehavior.test.js
@@ -3,8 +3,11 @@ import scrollLeft from 'dom-helpers/query/scrollLeft';
 import scrollTop from 'dom-helpers/query/scrollTop';
 import createBrowserHistory from 'history/lib/createBrowserHistory';
 import createHashHistory from 'history/lib/createHashHistory';
+import PageLifecycle from 'page-lifecycle/dist/lifecycle.es5';
+import sinon from 'sinon';
 
 import { createHashHistoryWithoutKey } from './histories';
+import { setEventListener, triggerEvent } from './mockPageLifecycle';
 import {
   withRoutes,
   withScrollElement,
@@ -22,10 +25,48 @@ describe('ScrollBehavior', () => {
     describe(createHistory.name, () => {
       let unlisten;
 
+      beforeEach(() => {
+        window.history.scrollRestoration = 'auto';
+      });
+
       afterEach(() => {
         if (unlisten) {
           unlisten();
         }
+        sinon.restore();
+        setEventListener();
+      });
+
+      it('sets/restores/resets scrollRestoration on freeze/resume', done => {
+        sinon.replace(PageLifecycle, 'addEventListener', setEventListener);
+        const history = withScroll(createHistory());
+        expect(window.history.scrollRestoration).to.equal('auto');
+        unlisten = run(history, [
+          () => {
+            expect(window.history.scrollRestoration).to.equal('manual');
+            triggerEvent('frozen', 'hidden');
+            expect(window.history.scrollRestoration).to.equal('manual');
+            triggerEvent('hidden', 'frozen');
+            expect(window.history.scrollRestoration).to.equal('auto');
+            triggerEvent('frozen', 'hidden');
+            expect(window.history.scrollRestoration).to.equal('manual');
+            done();
+          },
+        ]);
+      });
+
+      it('sets/restores scrollRestoration on termination', done => {
+        sinon.replace(PageLifecycle, 'addEventListener', setEventListener);
+        expect(window.history.scrollRestoration).to.equal('auto');
+        const history = withScroll(createHistory());
+        unlisten = run(history, [
+          () => {
+            expect(window.history.scrollRestoration).to.equal('manual');
+            triggerEvent('hidden', 'terminated');
+            expect(window.history.scrollRestoration).to.equal('auto');
+            done();
+          },
+        ]);
       });
 
       describe('default behavior', () => {

diff --git a/test/mockPageLifecycle.js b/test/mockPageLifecycle.js
@@ -0,0 +1,14 @@
+let listener;
+
+export const setEventListener = (eventType, callback) => {
+  listener = callback;
+};
+
+export const triggerEvent = (oldState, newState) => {
+  if (listener) {
+    const event = new Event('statechange');
+    event.newState = newState;
+    event.oldState = oldState;
+    listener(event);
+  }
+};
diff --git a/yarn.lock b/yarn.lock
@@ -5561,6 +5561,11 @@ p-try@^2.0.0:
   resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6"
   integrity sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==
 
+page-lifecycle@^0.1.2:
+  version "0.1.2"
+  resolved "https://registry.yarnpkg.com/page-lifecycle/-/page-lifecycle-0.1.2.tgz#f17a083c082bd5ababddd77f1025a4b1c8808012"
+  integrity sha512-+3uccYgL0CXG0KSXRxZi4uc2E6mqFWV5HqiJJgcnaJCiS0LqiuJ4vB420N21NFuLvuvLB4Jr5drgQ2NXAXF9Iw==
+
 pako@~1.0.5:
   version "1.0.10"
   resolved "https://registry.yarnpkg.com/pako/-/pako-1.0.10.tgz#4328badb5086a426aa90f541977d4955da5c9732"