Skip to content

Commit

Permalink
Fix basepath router events
Browse files Browse the repository at this point in the history
  • Loading branch information
Janpot committed Jul 4, 2020
1 parent 2a99f59 commit b2c59a5
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 10 deletions.
23 changes: 13 additions & 10 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export default class Router implements BaseRouter {
_wrapApp: (App: ComponentType) => any
isSsr: boolean
isFallback: boolean
_lastStarted?: string

static events: MittEmitter = mitt()

Expand Down Expand Up @@ -396,7 +397,10 @@ export default class Router implements BaseRouter {
}
}

this.abortComponentLoad(as)
this.abortComponentLoad()

const cleanedAs = delBasePath(as)
this._lastStarted = cleanedAs

// If the url change is only related to a hash change
// We should not proceed. We should only change the state.
Expand All @@ -406,10 +410,10 @@ export default class Router implements BaseRouter {
// any time without notice.
if (!options._h && this.onlyAHashChange(as)) {
this.asPath = as
Router.events.emit('hashChangeStart', as)
Router.events.emit('hashChangeStart', cleanedAs)
this.changeState(method, url, as, options)
this.scrollToHash(as)
Router.events.emit('hashChangeComplete', as)
Router.events.emit('hashChangeComplete', cleanedAs)
return resolve(true)
}

Expand Down Expand Up @@ -442,7 +446,6 @@ export default class Router implements BaseRouter {

const route = removePathTrailingSlash(pathname)
const { shallow = false } = options
const cleanedAs = delBasePath(as)

if (isDynamicRoute(route)) {
const { pathname: asPathname } = parse(cleanedAs)
Expand Down Expand Up @@ -476,7 +479,7 @@ export default class Router implements BaseRouter {
}
}

Router.events.emit('routeChangeStart', as)
Router.events.emit('routeChangeStart', cleanedAs)

// If shallow is true and the route exists in the router cache we reuse the previous result
this.getRouteInfo(route, pathname, query, as, shallow).then(
Expand All @@ -487,7 +490,7 @@ export default class Router implements BaseRouter {
return resolve(false)
}

Router.events.emit('beforeHistoryChange', as)
Router.events.emit('beforeHistoryChange', cleanedAs)
this.changeState(method, url, as, options)

if (process.env.NODE_ENV !== 'production') {
Expand All @@ -499,11 +502,11 @@ export default class Router implements BaseRouter {

this.set(route, pathname!, query, cleanedAs, routeInfo).then(() => {
if (error) {
Router.events.emit('routeChangeError', error, as)
Router.events.emit('routeChangeError', error, cleanedAs)
throw error
}

Router.events.emit('routeChangeComplete', as)
Router.events.emit('routeChangeComplete', cleanedAs)
return resolve(true)
})
},
Expand Down Expand Up @@ -862,11 +865,11 @@ export default class Router implements BaseRouter {
})
}

abortComponentLoad(as: string): void {
abortComponentLoad(): void {
if (this.clc) {
const e = new Error('Route Cancelled')
;(e as any).cancelled = true
Router.events.emit('routeChangeError', e, as)
Router.events.emit('routeChangeError', e, this._lastStarted)
this.clc()
this.clc = null
}
Expand Down
37 changes: 37 additions & 0 deletions test/integration/basepath/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { useEffect } from 'react'
import { useRouter } from 'next/router'

let eventLog = []

if (typeof window !== 'undefined') {
// global functions introduced to interface with the test infrastructure
window._clearEventLog = () => {
eventLog = []
}

window._getEventLog = () => {
return eventLog
}
}

function useLoggedEvent(event, serializeArgs = (...args) => args) {
const router = useRouter()
useEffect(() => {
const logEvent = (...args) =>
eventLog.push([event, ...serializeArgs(...args)])
router.events.on(event, logEvent)
return () => router.events.off(event, logEvent)
}, [event, router.events, serializeArgs])
}

export default function MyApp({ Component, pageProps }) {
useLoggedEvent('routeChangeStart')
useLoggedEvent('routeChangeComplete')
useLoggedEvent('routeChangeError', (err, url) => [
err.message,
err.cancelled,
url,
])
useLoggedEvent('beforeHistoryChange')
return <Component {...pageProps} />
}
6 changes: 6 additions & 0 deletions test/integration/basepath/pages/hello.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,11 @@ export default () => (
>
click me for error
</div>
<br />
<Link href="/slow-route">
<a id="slow-route">
<h1>Slow route</h1>
</a>
</Link>
</>
)
10 changes: 10 additions & 0 deletions test/integration/basepath/pages/slow-route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export async function getServerSideProps() {
// we will use this route to simulate a route error by clicking it
// twice in rapid succession
await new Promise((resolve) => setTimeout(resolve, 10000))
return { props: {} }
}

export default function Page() {
return null
}
41 changes: 41 additions & 0 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,47 @@ const runTests = (context, dev = false) => {
}
})

it('should use urls without basepath in router events', async () => {
const browser = await webdriver(context.appPort, '/docs/hello')
try {
await browser.eval('window._clearEventLog()')
await browser
.elementByCss('#other-page-link')
.click()
.waitForElementByCss('#other-page-title')

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toContainEqual(['routeChangeStart', '/other-page'])
expect(eventLog).toContainEqual(['beforeHistoryChange', '/other-page'])
expect(eventLog).toContainEqual(['routeChangeComplete', '/other-page'])
} finally {
await browser.close()
}
})

it('should use urls without basepath in router events for cancelled routes', async () => {
const browser = await webdriver(context.appPort, '/docs/hello')
try {
await browser.eval('window._clearEventLog()')
await browser
.elementByCss('#slow-route')
.click()
.elementByCss('#other-page-link')
.click()
.waitForElementByCss('#other-page-title')

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toContainEqual([
'routeChangeError',
'Route Cancelled',
true,
'/slow-route',
])
} finally {
await browser.close()
}
})

it('should allow URL query strings without refresh', async () => {
const browser = await webdriver(context.appPort, '/docs/hello?query=true')
try {
Expand Down

0 comments on commit b2c59a5

Please sign in to comment.