Add better hash URL support. (#1250)

* Add better hash URL support.
1. Add scrolling to given id related to hash
2. Hash changes won't trigger getInitialProps

* Add some comments.

* Fix tests.

* Add some test cases.

Author: arunoda

client/index.js

@@ -5,7 +5,7 @@ import HeadManager from './head-manager'
 import { createRouter } from '../lib/router'
 import App from '../lib/app'
 import evalScript from '../lib/eval-script'
-import { loadGetInitialProps } from '../lib/utils'
+import { loadGetInitialProps, getURL } from '../lib/utils'
 
 const {
   __NEXT_DATA__: {
@@ -15,14 +15,15 @@ const {
     err,
     pathname,
     query
-  }
+  },
+  location
 } = window
 
 const Component = evalScript(component).default
 const ErrorComponent = evalScript(errorComponent).default
 let lastAppProps
 
-export const router = createRouter(pathname, query, {
+export const router = createRouter(pathname, query, getURL(), {
   Component,
   ErrorComponent,
   err
@@ -34,11 +35,12 @@ const container = document.getElementById('__next')
 export default (onError) => {
   const emitter = new EventEmitter()
 
-  router.subscribe(({ Component, props, err }) => {
-    render({ Component, props, err, emitter }, onError)
+  router.subscribe(({ Component, props, hash, err }) => {
+    render({ Component, props, err, hash, emitter }, onError)
   })
 
-  render({ Component, props, err, emitter }, onError)
+  const hash = location.hash.substring(1)
+  render({ Component, props, hash, err, emitter }, onError)
 
   return emitter
 }
@@ -57,7 +59,7 @@ async function renderErrorComponent (err) {
   await doRender({ Component: ErrorComponent, props, err })
 }
 
-async function doRender ({ Component, props, err, emitter }) {
+async function doRender ({ Component, props, hash, err, emitter }) {
   if (!props && Component &&
     Component !== ErrorComponent &&
     lastAppProps.Component === ErrorComponent) {
@@ -73,7 +75,7 @@ async function doRender ({ Component, props, err, emitter }) {
   Component = Component || lastAppProps.Component
   props = props || lastAppProps.props
 
-  const appProps = { Component, props, err, router, headManager }
+  const appProps = { Component, props, hash, err, router, headManager }
   // lastAppProps has to be set before ReactDom.render to account for ReactDom throwing an error.
   lastAppProps = appProps
 

lib/app.js

@@ -17,8 +17,8 @@ export default class App extends Component {
   }
 
   render () {
-    const { Component, props, err, router } = this.props
-    const containerProps = { Component, props, router }
+    const { Component, props, hash, err, router } = this.props
+    const containerProps = { Component, props, hash, router }
 
     return <div>
       <Container {...containerProps} />
@@ -29,6 +29,24 @@ export default class App extends Component {
 }
 
 class Container extends Component {
+  componentDidMount () {
+    this.scrollToHash()
+  }
+
+  componentDidUpdate () {
+    this.scrollToHash()
+  }
+
+  scrollToHash () {
+    const { hash } = this.props
+    const el = document.getElementById(hash)
+    if (el) {
+      // If we call scrollIntoView() in here without a setTimeout
+      // it won't scroll properly.
+      setTimeout(() => el.scrollIntoView(), 0)
+    }
+  }
+
   shouldComponentUpdate (nextProps) {
     // need this check not to rerender component which has already thrown an error
     return !shallowEquals(this.props, nextProps)

lib/router/router.js

@@ -3,7 +3,7 @@ import { EventEmitter } from 'events'
 import evalScript from '../eval-script'
 import shallowEquals from '../shallow-equals'
 import PQueue from '../p-queue'
-import { loadGetInitialProps, getLocationOrigin } from '../utils'
+import { loadGetInitialProps, getURL } from '../utils'
 import { _notifyBuildIdMismatch } from './'
 import fetch from 'unfetch'
 
@@ -26,7 +26,7 @@ if (typeof window !== 'undefined' && typeof navigator.serviceWorker !== 'undefin
 }
 
 export default class Router extends EventEmitter {
-  constructor (pathname, query, { Component, ErrorComponent, err } = {}) {
+  constructor (pathname, query, as, { Component, ErrorComponent, err } = {}) {
     super()
     // represents the current component key
     this.route = toRoute(pathname)
@@ -41,6 +41,7 @@ export default class Router extends EventEmitter {
     this.ErrorComponent = ErrorComponent
     this.pathname = pathname
     this.query = query
+    this.as = as
     this.subscriptions = new Set()
     this.componentLoadCancel = null
     this.onPopState = this.onPopState.bind(this)
@@ -66,42 +67,12 @@ export default class Router extends EventEmitter {
       // Actually, for (1) we don't need to nothing. But it's hard to detect that event.
       // So, doing the following for (1) does no harm.
       const { pathname, query } = this
-      this.replace(format({ pathname, query }), getURL())
+      this.changeState('replaceState', format({ pathname, query }), getURL())
       return
     }
 
     const { url, as } = e.state
-    const { pathname, query } = parse(url, true)
-
-    this.abortComponentLoad(as)
-
-    if (!this.urlIsNew(pathname, query)) {
-      this.emit('routeChangeStart', as)
-      this.emit('routeChangeComplete', as)
-      return
-    }
-
-    const route = toRoute(pathname)
-
-    this.emit('routeChangeStart', as)
-    const {
-      data,
-      props,
-      error
-    } = await this.getRouteInfo(route, pathname, query, as)
-
-    if (error && error.cancelled) {
-      return
-    }
-
-    this.route = route
-    this.set(pathname, query, { ...data, props })
-
-    if (error) {
-      this.emit('routeChangeError', error, as)
-    } else {
-      this.emit('routeChangeComplete', as)
-    }
+    this.replace(url, as)
   }
 
   update (route, Component) {
@@ -160,6 +131,13 @@ export default class Router extends EventEmitter {
     this.abortComponentLoad(as)
     const { pathname, query } = parse(url, true)
 
+    // If the url change is only related to a hash change
+    // We should not proceed. We should only replace the state.
+    if (this.onlyAHashChange(as)) {
+      this.changeState('replaceState', url, as)
+      return
+    }
+
     // If asked to change the current URL we should reload the current page
     // (not location.reload() but reload getInitalProps and other Next.js stuffs)
     // We also need to set the method = replaceState always
@@ -180,9 +158,10 @@ export default class Router extends EventEmitter {
     }
 
     this.changeState(method, url, as)
+    const hash = window.location.hash.substring(1)
 
     this.route = route
-    this.set(pathname, query, { ...data, props })
+    this.set(pathname, query, as, { ...data, props, hash })
 
     if (error) {
       this.emit('routeChangeError', error, as)
@@ -228,12 +207,33 @@ export default class Router extends EventEmitter {
     return routeInfo
   }
 
-  set (pathname, query, data) {
+  set (pathname, query, as, data) {
     this.pathname = pathname
     this.query = query
+    this.as = as
     this.notify(data)
   }
 
+  onlyAHashChange (as) {
+    if (!this.as) return false
+    const [ oldUrlNoHash ] = this.as.split('#')
+    const [ newUrlNoHash, newHash ] = as.split('#')
+
+    // If the urls are change, there's more than a hash change
+    if (oldUrlNoHash !== newUrlNoHash) {
+      return false
+    }
+
+    // If there's no hash in the new url, we can't consider it as a hash change
+    if (!newHash) {
+      return false
+    }
+
+    // Now there's a hash in the new URL.
+    // We don't need to worry about the old hash.
+    return true
+  }
+
   urlIsNew (pathname, query) {
     return this.pathname !== pathname || !shallowEquals(query, this.query)
   }
@@ -346,12 +346,6 @@ export default class Router extends EventEmitter {
   }
 }
 
-function getURL () {
-  const { href } = window.location
-  const origin = getLocationOrigin()
-  return href.substring(origin.length)
-}
-
 function toRoute (path) {
   return path.replace(/\/$/, '') || '/'
 }

lib/utils.js

@@ -58,3 +58,9 @@ export function getLocationOrigin () {
   const { protocol, hostname, port } = window.location
   return `${protocol}//${hostname}${port ? ':' + port : ''}`
 }
+
+export function getURL () {
+  const { href } = window.location
+  const origin = getLocationOrigin()
+  return href.substring(origin.length)
+}

test/integration/basic/pages/nav/hash-changes.js

@@ -0,0 +1,28 @@
+import React, { Component } from 'react'
+import Link from 'next/link'
+
+let count = 0
+
+export default class SelfReload extends Component {
+  static getInitialProps ({ res }) {
+    if (res) return { count: 0 }
+    count += 1
+
+    return { count }
+  }
+
+  render () {
+    return (
+      <div id='hash-changes-page'>
+        <Link href='#via-link'>
+          <a id='via-link'>Via Link</a>
+        </Link>
+        <a href='#via-a' id='via-a'>Via A</a>
+        <Link href='/nav/hash-changes'>
+          <a id='page-url'>Page URL</a>
+        </Link>
+        <p>COUNT: {this.props.count}</p>
+      </div>
+    )
+  }
+}

test/integration/basic/test/client-navigation.js

@@ -119,5 +119,65 @@ export default (context, render) => {
         await browser.close()
       })
     })
+
+    describe('with hash changes', () => {
+      describe('when hash change via Link', () => {
+        it('should not run getInitialProps', async () => {
+          const browser = await webdriver(context.appPort, '/nav/hash-changes')
+
+          const counter = await browser
+            .elementByCss('#via-link').click()
+            .elementByCss('p').text()
+
+          expect(counter).toBe('COUNT: 0')
+
+          await browser.close()
+        })
+      })
+
+      describe('when hash change via A tag', () => {
+        it('should not run getInitialProps', async () => {
+          const browser = await webdriver(context.appPort, '/nav/hash-changes')
+
+          const counter = await browser
+            .elementByCss('#via-a').click()
+            .elementByCss('p').text()
+
+          expect(counter).toBe('COUNT: 0')
+
+          await browser.close()
+        })
+      })
+
+      describe('when hash get removed', () => {
+        it('should not run getInitialProps', async () => {
+          const browser = await webdriver(context.appPort, '/nav/hash-changes')
+
+          const counter = await browser
+            .elementByCss('#via-a').click()
+            .elementByCss('#page-url').click()
+            .elementByCss('p').text()
+
+          expect(counter).toBe('COUNT: 1')
+
+          await browser.close()
+        })
+      })
+
+      describe('when hash changed to a different hash', () => {
+        it('should not run getInitialProps', async () => {
+          const browser = await webdriver(context.appPort, '/nav/hash-changes')
+
+          const counter = await browser
+            .elementByCss('#via-a').click()
+            .elementByCss('#via-link').click()
+            .elementByCss('p').text()
+
+          expect(counter).toBe('COUNT: 0')
+
+          await browser.close()
+        })
+      })
+    })
   })
 }

test/integration/basic/test/index.test.js

@@ -45,7 +45,8 @@ describe('Basic Features', () => {
       renderViaHTTP(context.appPort, '/nav'),
       renderViaHTTP(context.appPort, '/nav/about'),
       renderViaHTTP(context.appPort, '/nav/querystring'),
-      renderViaHTTP(context.appPort, '/nav/self-reload')
+      renderViaHTTP(context.appPort, '/nav/self-reload'),
+      renderViaHTTP(context.appPort, '/nav/hash-changes')
     ])
   })
   afterAll(() => stopApp(context.server))