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

Add isReady field on router #20628

Merged
merged 8 commits into from
Dec 31, 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
1 change: 1 addition & 0 deletions docs/api-reference/next/router.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The following is the definition of the `router` object returned by both [`useRou
- `locale`: `String` - The active locale (if enabled).
- `locales`: `String[]` - All supported locales (if enabled).
- `defaultLocale`: `String` - The current default locale (if enabled).
- `isReady`: `boolean` - Whether the router fields are updated client-side and ready for use. Should only be used inside of `useEffect` methods and not for conditionally rendering on the server.

Additionally, the following methods are also included inside `router`:

Expand Down
1 change: 1 addition & 0 deletions packages/next/client/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const urlPropertyFields = [
'locale',
'locales',
'defaultLocale',
'isReady',
]
const routerEvents = [
'routeChangeStart',
Expand Down
24 changes: 22 additions & 2 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
loadGetInitialProps,
NextPageContext,
ST,
NEXT_DATA,
} from '../utils'
import { isDynamicRoute } from './utils/is-dynamic'
import { parseRelativeUrl } from './utils/parse-relative-url'
Expand All @@ -33,6 +34,13 @@ import resolveRewrites from './utils/resolve-rewrites'
import { getRouteMatcher } from './utils/route-matcher'
import { getRouteRegex } from './utils/route-regex'

declare global {
interface Window {
/* prod */
__NEXT_DATA__: NEXT_DATA
}
}

interface RouteProperties {
shallow: boolean
}
Expand Down Expand Up @@ -454,6 +462,7 @@ export default class Router implements BaseRouter {
locales?: string[]
defaultLocale?: string
domainLocales?: DomainLocales
isReady: boolean

private _idx: number = 0

Expand Down Expand Up @@ -527,8 +536,7 @@ export default class Router implements BaseRouter {
// if auto prerendered and dynamic route wait to update asPath
// until after mount to prevent hydration mismatch
this.asPath =
// @ts-ignore this is temporarily global (attached to window)
isDynamicRoute(pathname) && __NEXT_DATA__.autoExport ? pathname : as
isDynamicRoute(pathname) && self.__NEXT_DATA__.autoExport ? pathname : as
this.basePath = basePath
this.sub = subscription
this.clc = null
Expand All @@ -539,6 +547,12 @@ export default class Router implements BaseRouter {

this.isFallback = isFallback

this.isReady = !!(
self.__NEXT_DATA__.gssp ||
self.__NEXT_DATA__.gip ||
!self.location.search
)

if (process.env.__NEXT_I18N_SUPPORT) {
this.locale = locale
this.locales = locales
Expand Down Expand Up @@ -707,6 +721,12 @@ export default class Router implements BaseRouter {
return false
}

// for static pages with query params in the URL we delay
// marking the router ready until after the query is updated
if ((options as any)._h) {
this.isReady = true
}

// Default to scroll reset behavior unless explicitly specified to be
// `false`! This makes the behavior between using `Router#push` and a
// `<Link />` consistent.
Expand Down
6 changes: 6 additions & 0 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class ServerRouter implements NextRouter {
events: any
isFallback: boolean
locale?: string
isReady: boolean
locales?: string[]
defaultLocale?: string
domainLocales?: DomainLocales
Expand All @@ -83,6 +84,7 @@ class ServerRouter implements NextRouter {
query: ParsedUrlQuery,
as: string,
{ isFallback }: { isFallback: boolean },
isReady: boolean,
basePath: string,
locale?: string,
locales?: string[],
Expand All @@ -98,8 +100,10 @@ class ServerRouter implements NextRouter {
this.locale = locale
this.locales = locales
this.defaultLocale = defaultLocale
this.isReady = isReady
this.domainLocales = domainLocales
}

push(): any {
noRouter()
}
Expand Down Expand Up @@ -526,13 +530,15 @@ export async function renderToHTML(

// url will always be set
const asPath: string = renderOpts.resolvedAsPath || (req.url as string)
const routerIsReady = !!(getServerSideProps || hasPageGetInitialProps)
const router = new ServerRouter(
pathname,
query,
asPath,
{
isFallback: isFallback,
},
routerIsReady,
basePath,
renderOpts.locale,
renderOpts.locales,
Expand Down
4 changes: 2 additions & 2 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ describe('Build Output', () => {
expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad)).toBeCloseTo(65.3, 1)
expect(parseFloat(err404FirstLoad)).toBeCloseTo(65.4, 1)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(61.9, 1)
expect(parseFloat(sharedByAll)).toBeCloseTo(62, 1)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down
18 changes: 18 additions & 0 deletions test/integration/router-is-ready/pages/auto-export.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { useRouter } from 'next/router'

export default function Page(props) {
const router = useRouter()

if (typeof window !== 'undefined') {
if (!window.isReadyValues) {
window.isReadyValues = []
}
window.isReadyValues.push(router.isReady)
}

return (
<>
<p id="auto-export">auto-export page</p>
</>
)
}
26 changes: 26 additions & 0 deletions test/integration/router-is-ready/pages/gip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { useRouter } from 'next/router'

export default function Page(props) {
const router = useRouter()

if (typeof window !== 'undefined') {
if (!window.isReadyValues) {
window.isReadyValues = []
}
window.isReadyValues.push(router.isReady)
}

return (
<>
<p id="gssp">gssp page</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

Page.getInitialProps = () => {
return {
hello: 'world',
random: Math.random(),
}
}
28 changes: 28 additions & 0 deletions test/integration/router-is-ready/pages/gsp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { useRouter } from 'next/router'

export default function Page(props) {
const router = useRouter()

if (typeof window !== 'undefined') {
if (!window.isReadyValues) {
window.isReadyValues = []
}
window.isReadyValues.push(router.isReady)
}

return (
<>
<p id="gsp">gsp page</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export const getStaticProps = () => {
return {
props: {
hello: 'world',
random: Math.random(),
},
}
}
28 changes: 28 additions & 0 deletions test/integration/router-is-ready/pages/gssp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { useRouter } from 'next/router'

export default function Page(props) {
const router = useRouter()

if (typeof window !== 'undefined') {
if (!window.isReadyValues) {
window.isReadyValues = []
}
window.isReadyValues.push(router.isReady)
}

return (
<>
<p id="gssp">gssp page</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export const getServerSideProps = () => {
return {
props: {
hello: 'world',
random: Math.random(),
},
}
}
15 changes: 15 additions & 0 deletions test/integration/router-is-ready/pages/invalid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { useRouter } from 'next/router'

export default function Page(props) {
// eslint-disable-next-line
const router = useRouter()

// console.log(router.isReady)

return (
<>
<p id="invalid">invalid page</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}
88 changes: 88 additions & 0 deletions test/integration/router-is-ready/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/* eslint-env jest */

import { join } from 'path'
import webdriver from 'next-webdriver'
import {
findPort,
launchApp,
killApp,
nextStart,
nextBuild,
File,
} from 'next-test-utils'

jest.setTimeout(1000 * 60 * 1)

let app
let appPort
const appDir = join(__dirname, '../')
const invalidPage = new File(join(appDir, 'pages/invalid.js'))

function runTests(isDev) {
it('isReady should be true immediately for getInitialProps page', async () => {
const browser = await webdriver(appPort, '/gip')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})

it('isReady should be true immediately for getInitialProps page with query', async () => {
const browser = await webdriver(appPort, '/gip?hello=world')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})

it('isReady should be true immediately for getServerSideProps page', async () => {
const browser = await webdriver(appPort, '/gssp')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})

it('isReady should be true immediately for getServerSideProps page with query', async () => {
const browser = await webdriver(appPort, '/gssp?hello=world')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})

it('isReady should be true immediately for auto-export page without query', async () => {
const browser = await webdriver(appPort, '/auto-export')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})

it('isReady should be true after query update for auto-export page with query', async () => {
const browser = await webdriver(appPort, '/auto-export?hello=world')
expect(await browser.eval('window.isReadyValues')).toEqual([false, true])
})

it('isReady should be true after query update for getStaticProps page with query', async () => {
const browser = await webdriver(appPort, '/gsp?hello=world')
expect(await browser.eval('window.isReadyValues')).toEqual([false, true])
})

it('isReady should be true immediately for getStaticProps page without query', async () => {
const browser = await webdriver(appPort, '/gsp')
expect(await browser.eval('window.isReadyValues')).toEqual([true])
})
}

describe('router.isReady', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(async () => {
await killApp(app)
invalidPage.restore()
})

runTests(true)
})

describe('production mode', () => {
beforeAll(async () => {
await nextBuild(appDir)

appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})
})
2 changes: 1 addition & 1 deletion test/integration/size-limit/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ describe('Production response size', () => {
const delta = responseSizesBytes / 1024

// Expected difference: < 0.5
expect(delta).toBeCloseTo(281.5, 0)
expect(delta).toBeCloseTo(282, 0)
})
})