diff --git a/client/index.js b/client/index.js index 9941f6004230..0e0438b7736f 100644 --- a/client/index.js +++ b/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 diff --git a/lib/app.js b/lib/app.js index 979ed7bd42a5..5ff939653c4a 100644 --- a/lib/app.js +++ b/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
@@ -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) diff --git a/lib/router/router.js b/lib/router/router.js index f72a1629066b..16ba8848d332 100644 --- a/lib/router/router.js +++ b/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(/\/$/, '') || '/' } diff --git a/lib/utils.js b/lib/utils.js index d5ba6c5f301b..2831c0a465ff 100644 --- a/lib/utils.js +++ b/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) +} diff --git a/test/integration/basic/pages/nav/hash-changes.js b/test/integration/basic/pages/nav/hash-changes.js new file mode 100644 index 000000000000..99699796c5a3 --- /dev/null +++ b/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 ( +
+ + Via Link + + Via A + + Page URL + +

COUNT: {this.props.count}

+
+ ) + } +} diff --git a/test/integration/basic/test/client-navigation.js b/test/integration/basic/test/client-navigation.js index eeea8078bd14..96435c941c54 100644 --- a/test/integration/basic/test/client-navigation.js +++ b/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() + }) + }) + }) }) } diff --git a/test/integration/basic/test/index.test.js b/test/integration/basic/test/index.test.js index 33de82ba1d4e..d88ace289f69 100644 --- a/test/integration/basic/test/index.test.js +++ b/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))