From e2ab55ad23fe5d46ee48d760a273316b7eb35f98 Mon Sep 17 00:00:00 2001
From: nkzawa <naoyuki.kanezawa@gmail.com>
Date: Sat, 8 Oct 2016 14:12:51 +0900
Subject: [PATCH] fix client/router

---
 client/next.js   |   2 +-
 client/router.js | 283 +++++++++++++++++++++++------------------------
 lib/app.js       |  13 ++-
 lib/link.js      |  22 ++--
 server/render.js |   2 +-
 5 files changed, 156 insertions(+), 166 deletions(-)

diff --git a/client/next.js b/client/next.js
index e22b577188..7abddbf4b4 100644
--- a/client/next.js
+++ b/client/next.js
@@ -12,7 +12,7 @@ const {
 const App = app ? evalScript(app).default : DefaultApp
 const Component = evalScript(component).default
 
-const router = new Router({ Component, props })
+const router = new Router({ Component })
 const headManager = new HeadManager()
 const container = document.getElementById('__next')
 const appProps = { Component, props, router, headManager }
diff --git a/client/router.js b/client/router.js
index a517f012ee..7df774a155 100644
--- a/client/router.js
+++ b/client/router.js
@@ -4,119 +4,105 @@ import shallowEquals from './shallow-equals'
 
 export default class Router {
   constructor (initialData) {
-    this.subscriptions = []
-
-    const id = createUid()
-    const route = toRoute(location.pathname)
-
-    this.currentRoute = route
-    this.currentComponentData = { ...initialData, id }
+    // represents the current component key
+    this.route = toRoute(location.pathname)
 
     // set up the component cache (by route keys)
-    this.components = { [route]: initialData }
-
-    // in order for `e.state` to work on the `onpopstate` event
-    // we have to register the initial route upon initialization
-    this.replace(id, getURL())
+    this.components = { [this.route]: initialData }
 
+    this.subscriptions = new Set()
+    this.componentLoadCancel = null
     this.onPopState = this.onPopState.bind(this)
-    window.addEventListener('unload', () => {})
+
     window.addEventListener('popstate', this.onPopState)
   }
 
   onPopState (e) {
     this.abortComponentLoad()
-    const cur = this.currentComponentData.id
-    const url = getURL()
-    const { fromComponent, route } = e.state || {}
-    if (fromComponent && cur && fromComponent === cur) {
-      // if the component has not changed due
-      // to the url change, it means we only
-      // need to notify the subscriber about
-      // the URL change
-      this.set(url)
-    } else {
-      this.fetchComponent(route || url, (err, data) => {
-        if (err) {
-          // the only way we can appropriately handle
-          // this failure is deferring to the browser
-          // since the URL has already changed
-          location.reload()
-        } else {
-          this.currentRoute = route || toRoute(location.pathname)
-          this.currentComponentData = data
-          this.set(url)
-        }
-      })
-    }
+
+    const route = (e.state || {}).route || toRoute(location.pathname)
+
+    Promise.resolve()
+    .then(async () => {
+      const data = await this.fetchComponent(route)
+      let props
+      if (route !== this.route) {
+        props = await this.getInitialProps(data.Component)
+      }
+
+      this.route = route
+      this.set(getURL(), { ...data, props })
+    })
+    .catch((err) => {
+      if (err.cancelled) return
+
+      // the only way we can appropriately handle
+      // this failure is deferring to the browser
+      // since the URL has already changed
+      location.reload()
+    })
   }
 
-  update (route, data) {
+  async update (route, data) {
     data.Component = evalScript(data.component).default
     delete data.component
     this.components[route] = data
-    if (route === this.currentRoute) {
-      let cancelled = false
-      const cancel = () => { cancelled = true }
-      this.componentLoadCancel = cancel
-      getInitialProps(data, (err, dataWithProps) => {
-        if (cancel === this.componentLoadCancel) {
-          this.componentLoadCancel = false
-        }
-        if (cancelled) return
-        if (err) throw err
-        this.currentComponentData = dataWithProps
-        this.notify()
-      })
-    }
-  }
 
-  goTo (url, fn) {
-    this.change('pushState', null, url, fn)
+    if (route === this.route) {
+      let props
+      try {
+        props = await this.getInitialProps(data.Component)
+      } catch (err) {
+        if (err.cancelled) return false
+        throw err
+      }
+      this.notify({ ...data, props })
+    }
+    return true
   }
 
   back () {
     history.back()
   }
 
-  push (fromComponent, url, fn) {
-    this.change('pushState', fromComponent, url, fn)
+  push (route, url) {
+    return this.change('pushState', route, url)
   }
 
-  replace (id, url, fn) {
-    this.change('replaceState', id, url, fn)
+  replace (route, url) {
+    return this.change('replaceState', route, url)
   }
 
-  change (method, id, url, fn) {
+  async change (method, route, url) {
+    if (!route) route = toRoute(parse(url).pathname)
+
     this.abortComponentLoad()
 
-    const set = (id) => {
-      const state = id ? { fromComponent: id, route: this.currentRoute } : {}
-      history[method](state, null, url)
-      this.set(url)
-      if (fn) fn(null)
+    let data
+    let props
+    try {
+      data = await this.fetchComponent(route)
+      if (route !== this.route) {
+        props = await this.getInitialProps(data.Component)
+      }
+    } catch (err) {
+      if (err.cancelled) return false
+      throw err
     }
 
-    if (this.currentComponentData && id !== this.currentComponentData.id) {
-      this.fetchComponent(url, (err, data) => {
-        if (!err) {
-          this.currentRoute = toRoute(url)
-          this.currentComponentData = data
-          set(data.id)
-        }
-        if (fn) fn(err, data)
-      })
-    } else {
-      set(id)
-    }
+    history[method]({ route }, null, url)
+    this.route = route
+    this.set(url, { ...data, props })
+    return true
   }
 
-  set (url) {
+  set (url, data) {
     const parsed = parse(url, true)
+
     if (this.urlIsNew(parsed)) {
       this.pathname = parsed.pathname
       this.query = parsed.query
-      this.notify()
+      this.notify(data)
     }
   }
 
@@ -124,52 +110,66 @@ export default class Router {
     return this.pathname !== pathname || !shallowEquals(query, this.query)
   }
 
-  fetchComponent (url, fn) {
-    const { pathname } = parse(url)
-    const route = toRoute(pathname)
+  async fetchComponent (url) {
+    const route = toRoute(parse(url).pathname)
+
+    let data = this.components[route]
+    if (data) return data
 
+    let cancel
     let cancelled = false
-    let componentXHR = null
-    const cancel = () => {
-      cancelled = true
 
-      if (componentXHR && componentXHR.abort) {
-        componentXHR.abort()
+    const componentUrl = toJSONUrl(route)
+    data = await new Promise((resolve, reject) => {
+      this.componentLoadCancel = cancel = () => {
+        cancelled = true
+        if (componentXHR.abort) componentXHR.abort()
+
+        const err = new Error('Cancelled')
+        err.cancelled = true
+        reject(err)
       }
-    }
 
-    if (this.components[route]) {
-      const data = this.components[route]
-      getInitialProps(data, (err, dataWithProps) => {
-        if (cancel === this.componentLoadCancel) {
-          this.componentLoadCancel = false
-        }
-        if (cancelled) return
-        fn(err, dataWithProps)
+      const componentXHR = loadComponent(componentUrl, (err, data) => {
+        if (err) return reject(err)
+        resolve(data)
       })
-      this.componentLoadCancel = cancel
-      return
+    })
+
+    if (cancel === this.componentLoadCancel) {
+      this.componentLoadCancel = null
     }
 
-    const componentUrl = toJSONUrl(route)
+    // we update the cache even if cancelled
+    if (data) this.components[route] = data
 
-    componentXHR = loadComponent(componentUrl, (err, data) => {
-      if (cancel === this.componentLoadCancel) {
-        this.componentLoadCancel = false
-      }
-      if (err) {
-        if (!cancelled) fn(err)
-      } else {
-        const d = { ...data, id: createUid() }
-        // we update the cache even if cancelled
-        if (!this.components[route]) {
-          this.components[route] = d
-        }
-        if (!cancelled) fn(null, d)
-      }
-    })
+    if (cancelled) {
+      const err = new Error('Cancelled')
+      err.cancelled = true
+      throw err
+    }
 
+    return data
+  }
+
+  async getInitialProps (Component) {
+    let cancelled = false
+    const cancel = () => { cancelled = true }
     this.componentLoadCancel = cancel
+
+    const props = await (Component.getInitialProps ? Component.getInitialProps({}) : {})
+
+    if (cancel === this.componentLoadCancel) {
+      this.componentLoadCancel = null
+    }
+
+    if (cancelled) {
+      const err = new Error('Cancelled')
+      err.cancelled = true
+      throw err
+    }
+
+    return props
   }
 
   abortComponentLoad () {
@@ -179,44 +179,44 @@ export default class Router {
     }
   }
 
-  notify () {
-    this.subscriptions.forEach(fn => fn())
+  notify (data) {
+    this.subscriptions.forEach((fn) => fn(data))
   }
 
   subscribe (fn) {
-    this.subscriptions.push(fn)
-    return () => {
-      const i = this.subscriptions.indexOf(fn)
-      if (~i) this.subscriptions.splice(i, 1)
-    }
+    this.subscriptions.add(fn)
+    return () => this.subscriptions.delete(fn)
   }
 }
 
-// every route finishing in `/test/` becomes `/test`
+function getURL () {
+  return location.pathname + (location.search || '') + (location.hash || '')
+}
 
-export function toRoute (path) {
+function toRoute (path) {
   return path.replace(/\/$/, '') || '/'
 }
 
-export function toJSONUrl (route) {
+function toJSONUrl (route) {
   return ('/' === route ? '/index' : route) + '.json'
 }
 
-export function loadComponent (url, fn) {
+function loadComponent (url, fn) {
   return loadJSON(url, (err, data) => {
-    if (err && fn) fn(err)
-    const { component, props } = data
-    const Component = evalScript(component).default
-    getInitialProps({ Component, props }, fn)
-  })
-}
+    if (err) return fn(err)
 
-function getURL () {
-  return location.pathname + (location.search || '') + (location.hash || '')
-}
+    const { component } = data
+
+    let module
+    try {
+      module = evalScript(component)
+    } catch (err) {
+      return fn(err)
+    }
 
-function createUid () {
-  return Math.floor(Math.random() * 1e16)
+    const Component = module.default || module
+    fn(null, { Component })
+  })
 }
 
 function loadJSON (url, fn) {
@@ -234,21 +234,10 @@ function loadJSON (url, fn) {
     fn(null, data)
   }
   xhr.onerror = () => {
-    if (fn) fn(new Error('XHR failed. Status: ' + xhr.status))
+    fn(new Error('XHR failed. Status: ' + xhr.status))
   }
   xhr.open('GET', url)
   xhr.send()
 
   return xhr
 }
-
-function getInitialProps (data, fn) {
-  const { Component: { getInitialProps } } = data
-  if (getInitialProps) {
-    Promise.resolve(getInitialProps({}))
-    .then((props) => fn(null, { ...data, props }))
-    .catch(fn)
-  } else {
-    fn(null, data)
-  }
-}
diff --git a/lib/app.js b/lib/app.js
index 5084b9b4da..53b6c78d3e 100644
--- a/lib/app.js
+++ b/lib/app.js
@@ -24,9 +24,10 @@ export default class App extends Component {
   componentDidMount () {
     const { router } = this.props
 
-    this.close = router.subscribe(() => {
+    this.close = router.subscribe((data) => {
+      const props = data.props || this.state.props
       const state = propsToState({
-        ...router.currentComponentData,
+        ...data,
         router
       })
 
@@ -55,13 +56,15 @@ export default class App extends Component {
 
 function propsToState (props) {
   const { Component, router } = props
+  const { route } = router
   const url = {
     query: router.query,
     pathname: router.pathname,
     back: () => router.back(),
-    goTo: (url, fn) => router.goTo(url, fn),
-    push: (url, fn) => router.push(Component, url, fn),
-    replace: (url, fn) => router.replace(Component, url, fn)
+    push: (url) => router.push(route, url),
+    pushTo: (url) => router.push(null, url),
+    replace: (url) => router.replace(route, url),
+    replaceTo: (url) => router.replace(null, url)
   }
 
   return {
diff --git a/lib/link.js b/lib/link.js
index e7c2775599..ccdebc10f0 100644
--- a/lib/link.js
+++ b/lib/link.js
@@ -27,15 +27,13 @@ export default class Link extends Component {
     e.preventDefault()
 
     // straight up redirect
-    this.context.router.goTo(href, (err) => {
-      if (err) {
-        if (this.props.onError) this.props.onError(err)
-        return
-      }
-
-      if (false !== scroll) {
-        window.scrollTo(0, 0)
-      }
+    this.context.router.push(null, href)
+    .then((success) => {
+      if (!success) return
+      if (false !== scroll) window.scrollTo(0, 0)
+    })
+    .catch((err) => {
+      if (this.props.onError) this.props.onError(err)
     })
   }
 
@@ -45,15 +43,15 @@ export default class Link extends Component {
         onClick: this.linkClicked
       }
 
-      const isChildAnchor = child && 'a' === child.type
+      const isAnchor = child && 'a' === child.type
 
       // if child does not specify a href, specify it
       // so that repetition is not needed by the user
-      if (!isChildAnchor || !('href' in child.props)) {
+      if (!isAnchor || !('href' in child.props)) {
         props.href = this.props.href
       }
 
-      if (isChildAnchor) {
+      if (isAnchor) {
         return React.cloneElement(child, props)
       } else {
         return <a {...props}>{child}</a>
diff --git a/server/render.js b/server/render.js
index a478c10e0a..ccec1d0aa0 100644
--- a/server/render.js
+++ b/server/render.js
@@ -35,7 +35,7 @@ export async function render (path, req, res, { dir = process.cwd(), dev = false
     html,
     head,
     css,
-    data: { component },
+    data: { component, props },
     hotReload: false,
     dev
   })
-- 
GitLab