fix scroll behaviour + [wip] cypress tests for scroll behaviour

integration-tests/production-runtime/cypress/integration/production.js

@@ -5,9 +5,49 @@ describe(`Production build tests`, () => {
     cy.visit(`/`).waitForRouteChange()
   })
 
-  it(`should navigate back after a reload`, () => {
-    cy.getTestElement(`page2`)
+  it(`should restore scroll position only when going back in history`, () => {
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForRouteChange()
+
+    cy.scrollTo(`bottom`)
+
+    // allow ScrollContext to update scroll position store
+    // it uses requestAnimationFrame so wait a bit to allow
+    // it to store scroll position
+    cy.wait(500)
+
+    cy.getTestElement(`below-the-fold`)
+      .click()
+      .waitForRouteChange()
+
+    // after going back we expect page will
+    // be restore previous scroll position
+    cy.go(`back`).waitForRouteChange()
+
+    cy.window().then(win => {
+      expect(win.scrollY).not.to.eq(0, 0)
+    })
+
+    cy.go(`forward`).waitForRouteChange()
+
+    // after clicking link we expect page will be scrolled to top
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForRouteChange()
+
+    cy.window().then(win => {
+      expect(win.scrollY).to.eq(0, 0)
+    })
+
+    // reset to index page
+    cy.getTestElement(`index-link`)
       .click()
+      .waitForRouteChange()
+  })
+
+  it(`should navigate back after a reload`, () => {
+    cy.getTestElement(`page2`).click()
 
     cy.waitForRouteChange()
       .location(`pathname`)

integration-tests/production-runtime/package.json

@@ -5,10 +5,10 @@
   "author": "Kyle Mathews <[email protected]>",
   "dependencies": {
     "cypress": "^3.1.0",
-    "gatsby": "next",
-    "gatsby-plugin-manifest": "next",
-    "gatsby-plugin-offline": "next",
-    "gatsby-plugin-react-helmet": "next",
+    "gatsby": "latest",
+    "gatsby-plugin-manifest": "latest",
+    "gatsby-plugin-offline": "latest",
+    "gatsby-plugin-react-helmet": "latest",
     "react": "^16.4.2",
     "react-dom": "^16.4.2",
     "react-helmet": "^5.2.0"

integration-tests/production-runtime/src/components/header.js

@@ -18,6 +18,7 @@ const Header = ({ siteTitle }) => (
     >
       <h1 style={{ margin: 0 }}>
         <Link
+          data-testid="index-link"
           to="/"
           style={{
             color: `white`,

integration-tests/production-runtime/src/pages/index.js

@@ -8,8 +8,15 @@ const IndexPage = () => (
     <h1>Hi people</h1>
     <p>Welcome to your new Gatsby site.</p>
     <p>Now go build something great.</p>
-    <Link to="/page-2/"><span data-testid="page2">Go to page 2</span></Link>
-    <Link to="/page-3/" data-testid="404">To non-existent page</Link>
+    <Link to="/page-2/">
+      <span data-testid="page2">Go to page 2</span>
+    </Link>
+    <Link to="/page-3/" data-testid="404">
+      To non-existent page
+    </Link>
+    <Link to="/long-page/" data-testid="long-page">
+      To long page
+    </Link>
   </Layout>
 )
 

integration-tests/production-runtime/src/pages/long-page.js

@@ -0,0 +1,17 @@
+import React from 'react'
+import { Link } from 'gatsby'
+
+import Layout from '../components/layout'
+
+const LongPage = () => (
+  <Layout>
+    <h1>Hi from the long page</h1>
+    <p>Welcome to long page</p>
+    <div style={{ height: `200vh` }} />
+    <Link to="/" data-testid="below-the-fold">
+      Go back to the homepage
+    </Link>
+  </Layout>
+)
+
+export default LongPage

packages/gatsby/cache-dir/navigation.js

@@ -1,3 +1,5 @@
+import React from "react"
+import PropTypes from "prop-types"
 import loader, { setApiRunnerForLoader } from "./loader"
 import redirects from "./redirects.json"
 import { apiRunner } from "./api-runner-browser"
@@ -139,4 +141,39 @@ function init() {
   maybeRedirect(window.location.pathname)
 }
 
-export { init, shouldUpdateScroll, onRouteUpdate, onPreRouteUpdate }
+// Fire on(Pre)RouteUpdate APIs
+class RouteUpdates extends React.Component {
+  constructor(props) {
+    super(props)
+    onPreRouteUpdate(props.location)
+  }
+
+  componentDidMount() {
+    onRouteUpdate(this.props.location)
+  }
+
+  componentDidUpdate(prevProps, prevState, shouldFireRouteUpdate) {
+    if (shouldFireRouteUpdate) {
+      onRouteUpdate(this.props.location)
+    }
+  }
+
+  getSnapshotBeforeUpdate(prevProps) {
+    if (this.props.location.pathname !== prevProps.location.pathname) {
+      onPreRouteUpdate(this.props.location)
+      return true
+    }
+
+    return false
+  }
+
+  render() {
+    return this.props.children
+  }
+}
+
+RouteUpdates.propTypes = {
+  location: PropTypes.object.isRequired,
+}
+
+export { init, shouldUpdateScroll, RouteUpdates }

packages/gatsby/cache-dir/page-renderer.js

@@ -2,41 +2,9 @@ import React, { createElement } from "react"
 import PropTypes from "prop-types"
 import { publicLoader } from "./loader"
 import { apiRunner } from "./api-runner-browser"
-import { onRouteUpdate, onPreRouteUpdate } from "./navigation"
 
-// Renders page and fire on(Pre)RouteUpdate APIs
+// Renders page
 class PageRenderer extends React.Component {
-  constructor(props) {
-    super(props)
-    if (props.isMain) {
-      onPreRouteUpdate(props.location)
-    }
-  }
-
-  componentDidMount() {
-    if (this.props.isMain) {
-      onRouteUpdate(this.props.location)
-    }
-  }
-
-  componentDidUpdate(prevProps, prevState, shouldFireRouteUpdate) {
-    if (this.props.isMain && shouldFireRouteUpdate) {
-      onRouteUpdate(this.props.location)
-    }
-  }
-
-  getSnapshotBeforeUpdate(prevProps, prevState) {
-    if (this.props.isMain) {
-      if (this.props.location.pathname !== prevProps.location.pathname) {
-        onPreRouteUpdate(this.props.location)
-        return true
-      }
-
-      return false
-    }
-    return null
-  }
-
   render() {
     const props = {
       ...this.props,
@@ -73,7 +41,6 @@ PageRenderer.propTypes = {
   pageResources: PropTypes.object.isRequired,
   data: PropTypes.object,
   pageContext: PropTypes.object.isRequired,
-  isMain: PropTypes.bool,
 }
 
 export default PageRenderer

packages/gatsby/cache-dir/production-app.js

@@ -5,7 +5,11 @@ import { Router, navigate } from "@reach/router"
 import { match } from "@reach/router/lib/utils"
 import { ScrollContext } from "gatsby-react-router-scroll"
 import domReady from "domready"
-import { shouldUpdateScroll, init as navigationInit } from "./navigation"
+import {
+  shouldUpdateScroll,
+  init as navigationInit,
+  RouteUpdates,
+} from "./navigation"
 import emitter from "./emitter"
 window.___emitter = emitter
 import PageRenderer from "./page-renderer"
@@ -43,22 +47,23 @@ apiRunnerAsync(`onClientEntry`).then(() => {
       // if not, add that.
 
       return (
-        <ScrollContext
-          location={location}
-          shouldUpdateScroll={shouldUpdateScroll}
-        >
-          <EnsureResources location={location}>
-            {({ pageResources, location }) => (
-              <PageRenderer
-                {...this.props}
+        <EnsureResources location={location}>
+          {({ pageResources, location }) => (
+            <RouteUpdates location={location}>
+              <ScrollContext
                 location={location}
-                pageResources={pageResources}
-                {...pageResources.json}
-                isMain
-              />
-            )}
-          </EnsureResources>
-        </ScrollContext>
+                shouldUpdateScroll={shouldUpdateScroll}
+              >
+                <PageRenderer
+                  {...this.props}
+                  location={location}
+                  pageResources={pageResources}
+                  {...pageResources.json}
+                />
+              </ScrollContext>
+            </RouteUpdates>
+          )}
+        </EnsureResources>
       )
     }
   }

packages/gatsby/cache-dir/root.js

@@ -1,7 +1,11 @@
 import React, { createElement } from "react"
 import { Router } from "@reach/router"
 import { ScrollContext } from "gatsby-react-router-scroll"
-import { shouldUpdateScroll, init as navigationInit } from "./navigation"
+import {
+  shouldUpdateScroll,
+  init as navigationInit,
+  RouteUpdates,
+} from "./navigation"
 import { apiRunner } from "./api-runner-browser"
 import syncRequires from "./sync-requires"
 import pages from "./pages.json"
@@ -46,45 +50,40 @@ navigationInit()
 class RouteHandler extends React.Component {
   render() {
     let { location } = this.props
-    let child
 
     // check if page exists - in dev pages are sync loaded, it's safe to use
     // loader.getPage
     let page = loader.getPage(location.pathname)
 
     if (page) {
-      child = (
+      return (
         <EnsureResources location={location}>
           {locationAndPageResources => (
-            <JSONStore
-              pages={pages}
-              {...this.props}
-              {...locationAndPageResources}
-              isMain
-            />
+            <RouteUpdates location={location}>
+              <ScrollContext
+                location={location}
+                shouldUpdateScroll={shouldUpdateScroll}
+              >
+                <JSONStore
+                  pages={pages}
+                  {...this.props}
+                  {...locationAndPageResources}
+                />
+              </ScrollContext>
+            </RouteUpdates>
           )}
         </EnsureResources>
       )
     } else {
       const dev404Page = pages.find(p => /^\/dev-404-page\/$/.test(p.path))
-      child = createElement(
+      return createElement(
         syncRequires.components[dev404Page.componentChunkName],
         {
           pages,
           ...this.props,
         }
       )
     }
-
-    return (
-      <ScrollContext
-        location={location}
-        history={this.props.history}
-        shouldUpdateScroll={shouldUpdateScroll}
-      >
-        {child}
-      </ScrollContext>
-    )
   }
 }