Make .events work even when router is not initialized (#4874)

Followup of https://github.com/zeit/next.js/issues/4863#issuecomment-408920755

client/on-demand-entries-client.js

@@ -5,7 +5,7 @@ import fetch from 'unfetch'
 
 export default ({assetPrefix}) => {
   Router.ready(() => {
-    Router.router.events.on('routeChangeComplete', ping)
+    Router.events.on('routeChangeComplete', ping)
   })
 
   async function ping () {

lib/router/index.js

@@ -15,10 +15,17 @@ const SingletonRouter = {
 
 // Create public properties and methods of the router in the SingletonRouter
 const urlPropertyFields = ['pathname', 'route', 'query', 'asPath']
-const propertyFields = ['components', 'events']
+const propertyFields = ['components']
 const routerEvents = ['routeChangeStart', 'beforeHistoryChange', 'routeChangeComplete', 'routeChangeError', 'hashChangeStart', 'hashChangeComplete']
 const coreMethodFields = ['push', 'replace', 'reload', 'back', 'prefetch', 'beforePopState']
 
+// Events is a static property on the router, the router doesn't have to be initialized to use it
+Object.defineProperty(SingletonRouter, 'events', {
+  get () {
+    return _Router.events
+  }
+})
+
 propertyFields.concat(urlPropertyFields).forEach((field) => {
   // Here we need to use Object.defineProperty because, we need to return
   // the property assigned to the actual router
@@ -41,7 +48,7 @@ coreMethodFields.forEach((field) => {
 
 routerEvents.forEach((event) => {
   SingletonRouter.ready(() => {
-    SingletonRouter.router.events.on(event, (...args) => {
+    _Router.events.on(event, (...args) => {
       const eventField = `on${event.charAt(0).toUpperCase()}${event.substring(1)}`
       if (SingletonRouter[eventField]) {
         try {
@@ -136,6 +143,9 @@ export function makePublicRouterInstance (router) {
     instance[property] = router[property]
   }
 
+  // Events is a static property on the router, the router doesn't have to be initialized to use it
+  instance.events = _Router.events
+
   propertyFields.forEach((field) => {
     // Here we need to use Object.defineProperty because, we need to return
     // the property assigned to the actual router

lib/router/router.js

@@ -15,6 +15,8 @@ const historyMethodWarning = execOnce((method) => {
 })
 
 export default class Router {
+  static events = new EventEmitter()
+
   constructor (pathname, query, as, { initialProps, pageLoader, App, Component, ErrorComponent, err } = {}) {
     // represents the current component key
     this.route = toRoute(pathname)
@@ -30,8 +32,9 @@ export default class Router {
 
     this.components['/_app'] = { Component: App }
 
-    // Handling Router Events
+    // Backwards compat for Router.router.events
-    this.events = new EventEmitter()
+    // TODO: Should be remove the following major version as it was never documented
+    this.events = Router.events
 
     this.pageLoader = pageLoader
     this.prefetchQueue = new PQueue({ concurrency: 2 })
@@ -110,7 +113,7 @@ export default class Router {
     // This makes sure we only use pathname + query + hash, to mirror `asPath` coming from the server.
     const as = window.location.pathname + window.location.search + window.location.hash
 
-    this.events.emit('routeChangeStart', url)
+    Router.events.emit('routeChangeStart', url)
     const routeInfo = await this.getRouteInfo(route, pathname, query, as)
     const { error } = routeInfo
 
@@ -121,11 +124,11 @@ export default class Router {
     this.notify(routeInfo)
 
     if (error) {
-      this.events.emit('routeChangeError', error, url)
+      Router.events.emit('routeChangeError', error, url)
       throw error
     }
 
-    this.events.emit('routeChangeComplete', url)
+    Router.events.emit('routeChangeComplete', url)
   }
 
   back () {
@@ -157,10 +160,10 @@ export default class Router {
     // If the url change is only related to a hash change
     // We should not proceed. We should only change the state.
     if (this.onlyAHashChange(as)) {
-      this.events.emit('hashChangeStart', as)
+      Router.events.emit('hashChangeStart', as)
       this.changeState(method, url, as)
       this.scrollToHash(as)
-      this.events.emit('hashChangeComplete', as)
+      Router.events.emit('hashChangeComplete', as)
       return true
     }
 
@@ -178,7 +181,7 @@ export default class Router {
     const { shallow = false } = options
     let routeInfo = null
 
-    this.events.emit('routeChangeStart', as)
+    Router.events.emit('routeChangeStart', as)
 
     // If shallow === false and other conditions met, we reuse the
     // existing routeInfo for this route.
@@ -195,18 +198,18 @@ export default class Router {
       return false
     }
 
-    this.events.emit('beforeHistoryChange', as)
+    Router.events.emit('beforeHistoryChange', as)
     this.changeState(method, url, as, options)
     const hash = window.location.hash.substring(1)
 
     this.set(route, pathname, query, as, { ...routeInfo, hash })
 
     if (error) {
-      this.events.emit('routeChangeError', error, as)
+      Router.events.emit('routeChangeError', error, as)
       throw error
     }
 
-    this.events.emit('routeChangeComplete', as)
+    Router.events.emit('routeChangeComplete', as)
     return true
   }
 
@@ -400,7 +403,7 @@ export default class Router {
 
   abortComponentLoad (as) {
     if (this.componentLoadCancel) {
-      this.events.emit('routeChangeError', new Error('Route Cancelled'), as)
+      Router.events.emit('routeChangeError', new Error('Route Cancelled'), as)
       this.componentLoadCancel()
       this.componentLoadCancel = null
     }

test/integration/with-router/components/header-nav.js

@@ -1,5 +1,5 @@
 import * as React from 'react'
-import { withRouter } from 'next/router'
+import Router, { withRouter } from 'next/router'
 import Link from 'next/link'
 
 const pages = {
@@ -12,20 +12,40 @@ class HeaderNav extends React.Component {
     super()
 
     this.state = {
-      activeURL: router.asPath
+      activeURL: router.asPath,
+      activeURLTopLevelRouterDeprecatedBehavior: router.asPath,
+      activeURLTopLevelRouter: router.asPath
     }
 
     this.handleRouteChange = this.handleRouteChange.bind(this)
+    this.handleRouteChangeTopLevelRouter = this.handleRouteChangeTopLevelRouter.bind(this)
+    this.handleRouteChangeTopLevelRouterDeprecatedBehavior = this.handleRouteChangeTopLevelRouterDeprecatedBehavior.bind(this)
   }
 
   componentDidMount () {
+    Router.onRouteChangeComplete = this.handleRouteChangeTopLevelRouterDeprecatedBehavior
+    Router.events.on('routeChangeComplete', this.handleRouteChangeTopLevelRouter)
     this.props.router.events.on('routeChangeComplete', this.handleRouteChange)
   }
 
   componentWillUnmount () {
+    Router.onRouteChangeComplete = null
+    Router.events.on('routeChangeComplete', this.handleRouteChangeTopLevelRouter)
     this.props.router.events.off('routeChangeComplete', this.handleRouteChange)
   }
 
+  handleRouteChangeTopLevelRouterDeprecatedBehavior (url) {
+    this.setState({
+      activeURLTopLevelRouterDeprecatedBehavior: url
+    })
+  }
+
+  handleRouteChangeTopLevelRouter (url) {
+    this.setState({
+      activeURLTopLevelRouter: url
+    })
+  }
+
   handleRouteChange (url) {
     this.setState({
       activeURL: url
@@ -38,7 +58,7 @@ class HeaderNav extends React.Component {
         {
           Object.keys(pages).map(url => (
             <Link href={url} key={url} prefetch>
-              <a className={this.state.activeURL === url ? 'active' : ''}>
+              <a className={`${this.state.activeURL === url ? 'active' : ''} ${this.state.activeURLTopLevelRouter === url ? 'active-top-level-router' : ''} ${this.state.activeURLTopLevelRouterDeprecatedBehavior === url ? 'active-top-level-router-deprecated-behavior' : ''}`}>
                 { pages[url] }
               </a>
             </Link>

test/integration/with-router/test/index.test.js

@@ -30,7 +30,7 @@ describe('withRouter', () => {
 
   afterAll(() => stopApp(server))
 
-  it('allows observation of navigation events', async () => {
+  it('allows observation of navigation events using withRouter', async () => {
     const browser = await webdriver(appPort, '/a')
     await browser.waitForElementByCss('#page-a')
 
@@ -45,4 +45,36 @@ describe('withRouter', () => {
 
     browser.close()
   })
+
+  it('allows observation of navigation events using top level Router', async () => {
+    const browser = await webdriver(appPort, '/a')
+    await browser.waitForElementByCss('#page-a')
+
+    let activePage = await browser.elementByCss('.active-top-level-router').text()
+    expect(activePage).toBe('Foo')
+
+    await browser.elementByCss('button').click()
+    await browser.waitForElementByCss('#page-b')
+
+    activePage = await browser.elementByCss('.active-top-level-router').text()
+    expect(activePage).toBe('Bar')
+
+    browser.close()
+  })
+
+  it('allows observation of navigation events using top level Router deprecated behavior', async () => {
+    const browser = await webdriver(appPort, '/a')
+    await browser.waitForElementByCss('#page-a')
+
+    let activePage = await browser.elementByCss('.active-top-level-router-deprecated-behavior').text()
+    expect(activePage).toBe('Foo')
+
+    await browser.elementByCss('button').click()
+    await browser.waitForElementByCss('#page-b')
+
+    activePage = await browser.elementByCss('.active-top-level-router-deprecated-behavior').text()
+    expect(activePage).toBe('Bar')
+
+    browser.close()
+  })
 })