From 47cb4cf850b27c36f8839ad915bb0f14cc92910c Mon Sep 17 00:00:00 2001 From: kaykdm <34934746+kaykdm@users.noreply.github.com> Date: Tue, 26 Jan 2021 03:26:32 +0900 Subject: [PATCH] Fix aspath for getInitialProps (#20572) Fixes: https://github.com/vercel/next.js/issues/20370 > AsPath is incorrect on Server if you use rewrites and getInitialProps. On the server, asPath is the rewritten asPath while on the client asPath ist as given in the request URL. The same issue was used to happen on `getServersideProps`, but it was fixed in this PR (https://github.com/vercel/next.js/pull/17121). `getInitialProps` needs same fix except when the target is serverless, which has correct `asPath` value. Additional tests have been added in the `getInitialProps` suite to ensure correct asPath with rewrites. --- .../next/next-server/server/next-server.ts | 30 +++---- .../getinitialprops/next.config.js | 11 +++ .../getinitialprops/pages/blog/[post].js | 16 ++++ .../getinitialprops/pages/index.js | 3 + .../getinitialprops/pages/normal.js | 1 + .../getinitialprops/test/index.test.js | 81 +++++++++++++++++++ 6 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 test/integration/getinitialprops/next.config.js create mode 100644 test/integration/getinitialprops/pages/blog/[post].js create mode 100644 test/integration/getinitialprops/pages/index.js create mode 100644 test/integration/getinitialprops/pages/normal.js create mode 100644 test/integration/getinitialprops/test/index.test.js diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ea06715f21d6a08..382e8c28a230701 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1347,11 +1347,12 @@ export default class Server { typeof components.Component === 'object' && typeof (components.Component as any).renderReqToHTML === 'function' const isSSG = !!components.getStaticProps - const isServerProps = !!components.getServerSideProps + const hasServerProps = !!components.getServerSideProps const hasStaticPaths = !!components.getStaticPaths + const hasGetInitialProps = !!(components.Component as any).getInitialProps // Toggle whether or not this is a Data request - const isDataReq = !!query._nextDataReq && (isSSG || isServerProps) + const isDataReq = !!query._nextDataReq && (isSSG || hasServerProps) delete query._nextDataReq // we need to ensure the status code if /404 is visited directly @@ -1379,7 +1380,7 @@ export default class Server { let previewData: string | false | object | undefined let isPreviewMode = false - if (isServerProps || isSSG) { + if (hasServerProps || isSSG) { previewData = tryGetPreviewData(req, res, this.renderOpts.previewProps) isPreviewMode = previewData !== false } @@ -1603,17 +1604,18 @@ export default class Server { locale, locales, defaultLocale, - // For getServerSideProps we need to ensure we use the original URL + // For getServerSideProps and getInitialProps we need to ensure we use the original URL // and not the resolved URL to prevent a hydration mismatch on // asPath - resolvedAsPath: isServerProps - ? formatUrl({ - // we use the original URL pathname less the _next/data prefix if - // present - pathname: `${urlPathname}${hadTrailingSlash ? '/' : ''}`, - query: origQuery, - }) - : resolvedUrl, + resolvedAsPath: + hasServerProps || hasGetInitialProps + ? formatUrl({ + // we use the original URL pathname less the _next/data prefix if + // present + pathname: `${urlPathname}${hadTrailingSlash ? '/' : ''}`, + query: origQuery, + }) + : resolvedUrl, } renderResult = await renderToHTML( @@ -1720,7 +1722,7 @@ export default class Server { let resHtml = html const revalidateOptions = - !this.renderOpts.dev || (isServerProps && !isDataReq) + !this.renderOpts.dev || (hasServerProps && !isDataReq) ? { private: isPreviewMode, stateful: !isSSG, @@ -1731,7 +1733,7 @@ export default class Server { if ( !isResSent(res) && !isNotFound && - (isSSG || isDataReq || isServerProps) + (isSSG || isDataReq || hasServerProps) ) { if (isRedirect && !isDataReq) { await handleRedirect(pageData) diff --git a/test/integration/getinitialprops/next.config.js b/test/integration/getinitialprops/next.config.js new file mode 100644 index 000000000000000..41cc8e0d0e5eeb8 --- /dev/null +++ b/test/integration/getinitialprops/next.config.js @@ -0,0 +1,11 @@ +module.exports = { + // replace me + async rewrites() { + return [ + { + source: '/blog/post/:pid', + destination: '/blog/:pid', + }, + ] + }, +} diff --git a/test/integration/getinitialprops/pages/blog/[post].js b/test/integration/getinitialprops/pages/blog/[post].js new file mode 100644 index 000000000000000..6af9a8728a7ab2d --- /dev/null +++ b/test/integration/getinitialprops/pages/blog/[post].js @@ -0,0 +1,16 @@ +import React from 'react' +import { useRouter } from 'next/router' + +const Post = () => { + const router = useRouter() + + return ( + <> +
{router.asPath}
+ + ) +} + +Post.getInitialProps = () => ({ hello: 'hi' }) + +export default Post diff --git a/test/integration/getinitialprops/pages/index.js b/test/integration/getinitialprops/pages/index.js new file mode 100644 index 000000000000000..6a1fd4b88fb6f59 --- /dev/null +++ b/test/integration/getinitialprops/pages/index.js @@ -0,0 +1,3 @@ +const page = () => 'hello from sub id' +page.getInitialProps = () => ({ hello: 'hi' }) +export default page diff --git a/test/integration/getinitialprops/pages/normal.js b/test/integration/getinitialprops/pages/normal.js new file mode 100644 index 000000000000000..75ad8dfee1722d9 --- /dev/null +++ b/test/integration/getinitialprops/pages/normal.js @@ -0,0 +1 @@ +export default () =>

a normal page

diff --git a/test/integration/getinitialprops/test/index.test.js b/test/integration/getinitialprops/test/index.test.js new file mode 100644 index 000000000000000..a2e7445899228c7 --- /dev/null +++ b/test/integration/getinitialprops/test/index.test.js @@ -0,0 +1,81 @@ +import cheerio from 'cheerio' +import { join } from 'path' +import { + findPort, + launchApp, + killApp, + nextStart, + nextBuild, + renderViaHTTP, + File, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 5) +let app +let appPort +const appDir = join(__dirname, '..') +const nextConfig = new File(join(appDir, 'next.config.js')) + +const runTests = () => { + it('should have gip in __NEXT_DATA__', async () => { + const html = await renderViaHTTP(appPort, '/') + const $ = cheerio.load(html) + expect(JSON.parse($('#__NEXT_DATA__').text()).gip).toBe(true) + }) + + it('should not have gip in __NEXT_DATA__ for non-GIP page', async () => { + const html = await renderViaHTTP(appPort, '/normal') + const $ = cheerio.load(html) + expect('gip' in JSON.parse($('#__NEXT_DATA__').text())).toBe(false) + }) + + it('should have correct router.asPath for direct visit dynamic page', async () => { + const html = await renderViaHTTP(appPort, '/blog/1') + const $ = cheerio.load(html) + expect($('#as-path').text()).toBe('/blog/1') + }) + + it('should have correct router.asPath for direct visit dynamic page rewrite direct', async () => { + const html = await renderViaHTTP(appPort, '/blog/post/1') + const $ = cheerio.load(html) + expect($('#as-path').text()).toBe('/blog/post/1') + }) +} + +describe('getInitialProps', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) + + describe('serverless mode', () => { + beforeAll(async () => { + await nextConfig.replace('// replace me', `target: 'serverless', `) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(async () => { + await killApp(app) + nextConfig.restore() + }) + + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) +})