From e5e71b94c49331d60d4b05401fe644fccd0e7791 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 30 Dec 2020 21:27:52 -0600 Subject: [PATCH 1/4] Add isReady field on router --- docs/api-reference/next/router.md | 1 + packages/next/client/router.ts | 1 + .../next/next-server/lib/router/router.ts | 24 +++- packages/next/next-server/server/render.tsx | 5 + .../router-is-ready/pages/auto-export.js | 18 +++ test/integration/router-is-ready/pages/gip.js | 26 +++++ test/integration/router-is-ready/pages/gsp.js | 28 +++++ .../integration/router-is-ready/pages/gssp.js | 28 +++++ .../router-is-ready/pages/invalid.js | 15 +++ .../router-is-ready/test/index.test.js | 103 ++++++++++++++++++ 10 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 test/integration/router-is-ready/pages/auto-export.js create mode 100644 test/integration/router-is-ready/pages/gip.js create mode 100644 test/integration/router-is-ready/pages/gsp.js create mode 100644 test/integration/router-is-ready/pages/gssp.js create mode 100644 test/integration/router-is-ready/pages/invalid.js create mode 100644 test/integration/router-is-ready/test/index.test.js diff --git a/docs/api-reference/next/router.md b/docs/api-reference/next/router.md index 65d1318884587..f87df048ab663 100644 --- a/docs/api-reference/next/router.md +++ b/docs/api-reference/next/router.md @@ -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`: diff --git a/packages/next/client/router.ts b/packages/next/client/router.ts index cff79525bd50f..093a310f3b60d 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -40,6 +40,7 @@ const urlPropertyFields = [ 'locale', 'locales', 'defaultLocale', + 'isReady', ] const routerEvents = [ 'routeChangeStart', diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 98813725816a3..f552e97761009 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -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' @@ -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 } @@ -416,6 +424,7 @@ export default class Router implements BaseRouter { locales?: string[] defaultLocale?: string domainLocales?: DomainLocales + isReady: boolean static events: MittEmitter = mitt() @@ -487,8 +496,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 @@ -499,6 +507,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 @@ -650,6 +664,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 // `` consistent. diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 74e57abde7097..b31b26460b3f4 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -97,6 +97,11 @@ class ServerRouter implements NextRouter { this.locales = locales this.defaultLocale = defaultLocale } + + get isReady(): any { + return noRouter() + } + push(): any { noRouter() } diff --git a/test/integration/router-is-ready/pages/auto-export.js b/test/integration/router-is-ready/pages/auto-export.js new file mode 100644 index 0000000000000..f325029ae5880 --- /dev/null +++ b/test/integration/router-is-ready/pages/auto-export.js @@ -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 ( + <> +

auto-export page

+ + ) +} diff --git a/test/integration/router-is-ready/pages/gip.js b/test/integration/router-is-ready/pages/gip.js new file mode 100644 index 0000000000000..861fb516946b9 --- /dev/null +++ b/test/integration/router-is-ready/pages/gip.js @@ -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 ( + <> +

gssp page

+

{JSON.stringify(props)}

+ + ) +} + +Page.getInitialProps = () => { + return { + hello: 'world', + random: Math.random(), + } +} diff --git a/test/integration/router-is-ready/pages/gsp.js b/test/integration/router-is-ready/pages/gsp.js new file mode 100644 index 0000000000000..b2bf1b290ab01 --- /dev/null +++ b/test/integration/router-is-ready/pages/gsp.js @@ -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 ( + <> +

gsp page

+

{JSON.stringify(props)}

+ + ) +} + +export const getStaticProps = () => { + return { + props: { + hello: 'world', + random: Math.random(), + }, + } +} diff --git a/test/integration/router-is-ready/pages/gssp.js b/test/integration/router-is-ready/pages/gssp.js new file mode 100644 index 0000000000000..8a103d094a2df --- /dev/null +++ b/test/integration/router-is-ready/pages/gssp.js @@ -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 ( + <> +

gssp page

+

{JSON.stringify(props)}

+ + ) +} + +export const getServerSideProps = () => { + return { + props: { + hello: 'world', + random: Math.random(), + }, + } +} diff --git a/test/integration/router-is-ready/pages/invalid.js b/test/integration/router-is-ready/pages/invalid.js new file mode 100644 index 0000000000000..d5c2d696cdd2c --- /dev/null +++ b/test/integration/router-is-ready/pages/invalid.js @@ -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 ( + <> +

invalid page

+

{JSON.stringify(props)}

+ + ) +} diff --git a/test/integration/router-is-ready/test/index.test.js b/test/integration/router-is-ready/test/index.test.js new file mode 100644 index 0000000000000..322eb144967ef --- /dev/null +++ b/test/integration/router-is-ready/test/index.test.js @@ -0,0 +1,103 @@ +/* eslint-env jest */ + +import { join } from 'path' +import webdriver from 'next-webdriver' +import { + findPort, + launchApp, + killApp, + nextStart, + nextBuild, + File, + hasRedbox, + getRedboxHeader, +} 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) { + if (isDev) { + it('should show error when accessing isReady in render on server', async () => { + invalidPage.replace('// console', 'console') + const browser = await webdriver(appPort, '/invalid') + + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toContain( + `No router instance found. you should only use "next/router" inside the client side of your app` + ) + invalidPage.restore() + }) + } + + 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() + }) +}) From 7fb0f680930cfaccaff9ab49c5bb80ee93d6f9d8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 30 Dec 2020 22:10:39 -0600 Subject: [PATCH 2/4] Update test --- test/integration/build-output/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 2b7498188d963..fb5968c81b628 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -104,7 +104,7 @@ describe('Build Output', () => { expect(parseFloat(err404FirstLoad)).toBeCloseTo(65.3, 1) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 61.8).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 61.9).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { From 1e37c6af51dfe30760f918003ce7345a933ae524 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 30 Dec 2020 22:41:07 -0600 Subject: [PATCH 3/4] Disable server isReady error --- packages/next/next-server/server/render.tsx | 10 ++++++---- .../router-is-ready/test/index.test.js | 15 --------------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index b31b26460b3f4..12842393bcf7f 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -72,8 +72,10 @@ class ServerRouter implements NextRouter { events: any isFallback: boolean locale?: string + isReady: boolean locales?: string[] defaultLocale?: string + // TODO: Remove in the next major version, as this would mean the user is adding event listeners in server-side `render` method static events: MittEmitter = mitt() @@ -82,6 +84,7 @@ class ServerRouter implements NextRouter { query: ParsedUrlQuery, as: string, { isFallback }: { isFallback: boolean }, + isReady: boolean, basePath: string, locale?: string, locales?: string[], @@ -96,10 +99,7 @@ class ServerRouter implements NextRouter { this.locale = locale this.locales = locales this.defaultLocale = defaultLocale - } - - get isReady(): any { - return noRouter() + this.isReady = isReady } push(): any { @@ -528,6 +528,7 @@ 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, @@ -535,6 +536,7 @@ export async function renderToHTML( { isFallback: isFallback, }, + routerIsReady, basePath, renderOpts.locale, renderOpts.locales, diff --git a/test/integration/router-is-ready/test/index.test.js b/test/integration/router-is-ready/test/index.test.js index 322eb144967ef..b2a9509e52a02 100644 --- a/test/integration/router-is-ready/test/index.test.js +++ b/test/integration/router-is-ready/test/index.test.js @@ -9,8 +9,6 @@ import { nextStart, nextBuild, File, - hasRedbox, - getRedboxHeader, } from 'next-test-utils' jest.setTimeout(1000 * 60 * 1) @@ -21,19 +19,6 @@ const appDir = join(__dirname, '../') const invalidPage = new File(join(appDir, 'pages/invalid.js')) function runTests(isDev) { - if (isDev) { - it('should show error when accessing isReady in render on server', async () => { - invalidPage.replace('// console', 'console') - const browser = await webdriver(appPort, '/invalid') - - expect(await hasRedbox(browser)).toBe(true) - expect(await getRedboxHeader(browser)).toContain( - `No router instance found. you should only use "next/router" inside the client side of your app` - ) - invalidPage.restore() - }) - } - it('isReady should be true immediately for getInitialProps page', async () => { const browser = await webdriver(appPort, '/gip') expect(await browser.eval('window.isReadyValues')).toEqual([true]) From 9bc3846ea6d471f0d0d51b4f6e3c53b2c8ad482f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 31 Dec 2020 10:23:35 -0600 Subject: [PATCH 4/4] Update size-limit --- test/integration/size-limit/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 4a761552ca505..a2bc467eec594 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -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) }) })