Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added flag to identify shallow router events #19802

Merged
merged 4 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions docs/api-reference/next/router.md
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,13 @@ export default function Page() {

You can listen to different events happening inside the Next.js Router. Here's a list of supported events:

- `routeChangeStart(url)` - Fires when a route starts to change
- `routeChangeComplete(url)` - Fires when a route changed completely
- `routeChangeError(err, url)` - Fires when there's an error when changing routes, or a route load is cancelled
- `routeChangeStart(url, { shallow })` - Fires when a route starts to change
- `routeChangeComplete(url, { shallow })` - Fires when a route changed completely
- `routeChangeError(err, url, { shallow })` - Fires when there's an error when changing routes, or a route load is cancelled
- `err.cancelled` - Indicates if the navigation was cancelled
- `beforeHistoryChange(url)` - Fires just before changing the browser's history
- `hashChangeStart(url)` - Fires when the hash will change but not the page
- `hashChangeComplete(url)` - Fires when the hash has changed but not the page
- `beforeHistoryChange(url, { shallow })` - Fires just before changing the browser's history
- `hashChangeStart(url, { shallow })` - Fires when the hash will change but not the page
- `hashChangeComplete(url, { shallow })` - Fires when the hash has changed but not the page

> **Note:** Here `url` is the URL shown in the browser, including the [`basePath`](/docs/api-reference/next.config.js/basepath.md).

Expand All @@ -332,8 +332,12 @@ export default function MyApp({ Component, pageProps }) {
const router = useRouter()

useEffect(() => {
const handleRouteChange = (url) => {
console.log('App is changing to: ', url)
const handleRouteChange = (url, { shallow }) => {
console.log(
`App is changing to ${url} ${
shallow ? 'with' : 'without'
} shallow routing`
)
}

router.events.on('routeChangeStart', handleRouteChange)
Expand Down
51 changes: 35 additions & 16 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ import resolveRewrites from './utils/resolve-rewrites'
import { getRouteMatcher } from './utils/route-matcher'
import { getRouteRegex } from './utils/route-regex'

interface RouteProperties {
shallow: boolean
}

interface TransitionOptions {
shallow?: boolean
locale?: string | false
Expand Down Expand Up @@ -652,8 +656,11 @@ export default class Router implements BaseRouter {
performance.mark('routeChange')
}

const { shallow = false } = options
const routeProps = { shallow }

if (this._inFlightRoute) {
this.abortComponentLoad(this._inFlightRoute)
this.abortComponentLoad(this._inFlightRoute, routeProps)
}

as = addBasePath(
Expand All @@ -677,12 +684,12 @@ export default class Router implements BaseRouter {
// any time without notice.
if (!(options as any)._h && this.onlyAHashChange(cleanedAs)) {
this.asPath = cleanedAs
Router.events.emit('hashChangeStart', as)
Router.events.emit('hashChangeStart', as, routeProps)
// TODO: do we need the resolved href when only a hash change?
this.changeState(method, url, as, options)
this.scrollToHash(cleanedAs)
this.notify(this.components[this.route])
Router.events.emit('hashChangeComplete', as)
Router.events.emit('hashChangeComplete', as, routeProps)
return true
}

Expand Down Expand Up @@ -727,7 +734,6 @@ export default class Router implements BaseRouter {
}

let route = removePathTrailingSlash(pathname)
const { shallow = false } = options

// we need to resolve the as value using rewrites for dynamic SSG
// pages to allow building the data URL correctly
Expand Down Expand Up @@ -827,15 +833,15 @@ export default class Router implements BaseRouter {
}
}

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

try {
const routeInfo = await this.getRouteInfo(
route,
pathname,
query,
as,
shallow
routeProps
)
let { error, props, __N_SSG, __N_SSP } = routeInfo

Expand Down Expand Up @@ -869,7 +875,7 @@ export default class Router implements BaseRouter {
return new Promise(() => {})
}

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

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

if (error) {
Router.events.emit('routeChangeError', error, cleanedAs)
Router.events.emit('routeChangeError', error, cleanedAs, routeProps)
throw error
}

Expand All @@ -902,7 +908,7 @@ export default class Router implements BaseRouter {
document.documentElement.lang = this.locale
}
}
Router.events.emit('routeChangeComplete', as)
Router.events.emit('routeChangeComplete', as, routeProps)

return true
} catch (err) {
Expand Down Expand Up @@ -954,6 +960,7 @@ export default class Router implements BaseRouter {
pathname: string,
query: ParsedUrlQuery,
as: string,
routeProps: RouteProperties,
loadErrorFail?: boolean
): Promise<CompletePrivateRouteInfo> {
if (err.cancelled) {
Expand All @@ -962,7 +969,7 @@ export default class Router implements BaseRouter {
}

if (isAssetError(err) || loadErrorFail) {
Router.events.emit('routeChangeError', err, as)
Router.events.emit('routeChangeError', err, as, routeProps)

// If we can't load the page it could be one of following reasons
// 1. Page doesn't exists
Expand Down Expand Up @@ -1034,7 +1041,14 @@ export default class Router implements BaseRouter {

return routeInfo
} catch (routeInfoErr) {
return this.handleRouteInfoError(routeInfoErr, pathname, query, as, true)
return this.handleRouteInfoError(
routeInfoErr,
pathname,
query,
as,
routeProps,
true
)
}
}

Expand All @@ -1043,13 +1057,13 @@ export default class Router implements BaseRouter {
pathname: string,
query: any,
as: string,
shallow: boolean = false
routeProps: RouteProperties
): Promise<PrivateRouteInfo> {
try {
const existingRouteInfo: PrivateRouteInfo | undefined = this.components[
route
]
if (shallow && existingRouteInfo && this.route === route) {
if (routeProps.shallow && existingRouteInfo && this.route === route) {
return existingRouteInfo
}

Expand Down Expand Up @@ -1108,7 +1122,7 @@ export default class Router implements BaseRouter {
this.components[route] = routeInfo
return routeInfo
} catch (err) {
return this.handleRouteInfoError(err, pathname, query, as)
return this.handleRouteInfoError(err, pathname, query, as, routeProps)
}
}

Expand Down Expand Up @@ -1351,9 +1365,14 @@ export default class Router implements BaseRouter {
})
}

abortComponentLoad(as: string): void {
abortComponentLoad(as: string, routeProps: RouteProperties): void {
if (this.clc) {
Router.events.emit('routeChangeError', buildCancellationError(), as)
Router.events.emit(
'routeChangeError',
buildCancellationError(),
as,
routeProps
)
this.clc()
this.clc = null
}
Expand Down
4 changes: 2 additions & 2 deletions test/integration/basepath/pages/_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ function useLoggedEvent(event, serializeArgs = (...args) => args) {
}, [event, router.events, serializeArgs])
}

function serializeErrorEventArgs(err, url) {
return [err.message, err.cancelled, url]
function serializeErrorEventArgs(err, url, properties) {
return [err.message, err.cancelled, url, properties]
}

export default function MyApp({ Component, pageProps }) {
Expand Down
33 changes: 22 additions & 11 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -924,9 +924,9 @@ const runTests = (dev = false) => {

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toEqual([
['routeChangeStart', `${basePath}/other-page`],
['beforeHistoryChange', `${basePath}/other-page`],
['routeChangeComplete', `${basePath}/other-page`],
['routeChangeStart', `${basePath}/other-page`, { shallow: false }],
['beforeHistoryChange', `${basePath}/other-page`, { shallow: false }],
['routeChangeComplete', `${basePath}/other-page`, { shallow: false }],
])
} finally {
await browser.close()
Expand All @@ -941,8 +941,12 @@ const runTests = (dev = false) => {

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toEqual([
['hashChangeStart', `${basePath}/hello#some-hash`],
['hashChangeComplete', `${basePath}/hello#some-hash`],
['hashChangeStart', `${basePath}/hello#some-hash`, { shallow: false }],
[
'hashChangeComplete',
`${basePath}/hello#some-hash`,
{ shallow: false },
],
])
} finally {
await browser.close()
Expand All @@ -962,11 +966,17 @@ const runTests = (dev = false) => {

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toEqual([
['routeChangeStart', `${basePath}/slow-route`],
['routeChangeError', 'Route Cancelled', true, `${basePath}/slow-route`],
['routeChangeStart', `${basePath}/other-page`],
['beforeHistoryChange', `${basePath}/other-page`],
['routeChangeComplete', `${basePath}/other-page`],
['routeChangeStart', `${basePath}/slow-route`, { shallow: false }],
[
'routeChangeError',
'Route Cancelled',
true,
`${basePath}/slow-route`,
{ shallow: false },
],
['routeChangeStart', `${basePath}/other-page`, { shallow: false }],
['beforeHistoryChange', `${basePath}/other-page`, { shallow: false }],
['routeChangeComplete', `${basePath}/other-page`, { shallow: false }],
])
} finally {
await browser.close()
Expand All @@ -983,12 +993,13 @@ const runTests = (dev = false) => {

const eventLog = await browser.eval('window._getEventLog()')
expect(eventLog).toEqual([
['routeChangeStart', `${basePath}/error-route`],
['routeChangeStart', `${basePath}/error-route`, { shallow: false }],
[
'routeChangeError',
'Failed to load static props',
null,
`${basePath}/error-route`,
{ shallow: false },
],
])
} finally {
Expand Down