From 95ac43d3bbff891adce7560e38ae4ff1d9828404 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 27 Aug 2020 19:29:11 -0500 Subject: [PATCH 1/8] Add handling for redirects from getStaticProps/getServerSideProps --- errors/invalid-redirect-gssp.md | 32 ++++++++ .../next/next-server/lib/router/router.ts | 33 +++++++- packages/next/next-server/server/render.tsx | 75 ++++++++++++++++- packages/next/types/index.d.ts | 7 ++ test/integration/gssp-redirect/pages/404.js | 3 + .../gssp-redirect/pages/gsp-blog/[post].js | 39 +++++++++ .../gssp-redirect/pages/gssp-blog/[post].js | 32 ++++++++ .../gssp-redirect/test/index.test.js | 81 +++++++++++++++++++ 8 files changed, 299 insertions(+), 3 deletions(-) create mode 100644 errors/invalid-redirect-gssp.md create mode 100644 test/integration/gssp-redirect/pages/404.js create mode 100644 test/integration/gssp-redirect/pages/gsp-blog/[post].js create mode 100644 test/integration/gssp-redirect/pages/gssp-blog/[post].js create mode 100644 test/integration/gssp-redirect/test/index.test.js diff --git a/errors/invalid-redirect-gssp.md b/errors/invalid-redirect-gssp.md new file mode 100644 index 0000000000000..05db61946d490 --- /dev/null +++ b/errors/invalid-redirect-gssp.md @@ -0,0 +1,32 @@ +# Invalid Redirect getStaticProps/getServerSideProps + +#### Why This Error Occurred + +The `redirect` value returned from your `getStaticProps` or `getServerSideProps` function had invalid values. + +#### Possible Ways to Fix It + +Make sure you return the proper values for the `redirect` value. + +```js +export const getStaticProps = ({ params }) => { + if (params.slug === 'deleted-post') { + return { + redirect: { + permanent: true // or false + destination: '/some-location' + } + } + } + + return { + props: { + // data + } + } +} +``` + +### Useful Links + +- [Data Fetching Documentation](https://nextjs.org/docs/basic-features/data-fetching#getstaticprops-static-generation) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 23ea0bc9ba4e6..2247b530416f3 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -593,7 +593,7 @@ export default class Router implements BaseRouter { as, shallow ) - let { error } = routeInfo + let { error, props, __N_SSG, __N_SSP } = routeInfo Router.events.emit('beforeHistoryChange', as) this.changeState(method, url, as, options) @@ -617,6 +617,36 @@ export default class Router implements BaseRouter { throw error } + // handle redirect on client-transition + if ( + (__N_SSG || __N_SSP) && + props && + (props as any).pageProps && + (props as any).pageProps.__N_REDIRECT + ) { + const destination = (props as any).pageProps.__N_REDIRECT + + // check if destination is internal (resolves to a page) and attempt + // client-navigation if it is falling back to hard navigation if + // it's not + if (destination.startsWith('/')) { + const parsedHref = parseRelativeUrl(destination) + this._resolveHref(parsedHref, pages) + + if (pages.includes(parsedHref.pathname)) { + return this.change( + 'replaceState', + destination, + destination, + options + ) + } + } + + window.location.href = destination + return new Promise(() => {}) + } + if (process.env.__NEXT_SCROLL_RESTORATION) { if (manualScrollRestoration && '_N_X' in options) { window.scrollTo((options as any)._N_X, (options as any)._N_Y) @@ -784,6 +814,7 @@ export default class Router implements BaseRouter { } as any ) ) + routeInfo.props = props this.components[route] = routeInfo return routeInfo diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 5172d948a07a2..62a85923fdff7 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -21,6 +21,8 @@ import { AMP_RENDER_TARGET, SERVER_PROPS_ID, STATIC_PROPS_ID, + PERMANENT_REDIRECT_STATUS, + TEMPORARY_REDIRECT_STATUS, } from '../lib/constants' import { defaultHead } from '../lib/head' import { HeadManagerContext } from '../lib/head-manager-context' @@ -264,6 +266,47 @@ const invalidKeysMsg = (methodName: string, invalidKeys: string[]) => { ) } +type Redirect = { + permanent: boolean + destination: string +} + +function checkRedirectValues(redirect: Redirect, req: IncomingMessage) { + const { destination, permanent } = redirect + let invalidPermanent = typeof permanent !== 'boolean' + let invalidDestination = typeof destination !== 'string' + + if (invalidPermanent || invalidDestination) { + throw new Error( + `Invalid redirect object returned from getStaticProps for ${req.url}\n` + + `Expected${ + invalidPermanent + ? ` \`permanent\` to be boolean but received ${typeof permanent}` + : '' + }${invalidPermanent && invalidDestination ? ' and' : ''}${ + invalidDestination + ? ` \`destinatino\` to be string but received ${typeof destination}` + : '' + }\n` + + `See more info here: https://err.sh/vercel/next.js/invalid-redirect-gssp` + ) + } +} + +function handleRedirect(res: ServerResponse, redirect: Redirect) { + // TODO: this should error if a redirect is returned while prerendering + const statusCode = redirect.permanent + ? PERMANENT_REDIRECT_STATUS + : TEMPORARY_REDIRECT_STATUS + + if (redirect.permanent) { + res.setHeader('Refresh', `0;url=${redirect.destination}`) + } + res.statusCode = statusCode + res.setHeader('Location', redirect.destination) + res.end() +} + export async function renderToHTML( req: IncomingMessage, res: ServerResponse, @@ -534,7 +577,7 @@ export async function renderToHTML( } const invalidKeys = Object.keys(data).filter( - (key) => key !== 'revalidate' && key !== 'props' + (key) => key !== 'revalidate' && key !== 'props' && key !== 'redirect' ) if (invalidKeys.includes('unstable_revalidate')) { @@ -545,6 +588,19 @@ export async function renderToHTML( throw new Error(invalidKeysMsg('getStaticProps', invalidKeys)) } + if (data.redirect && typeof data.redirect === 'object') { + checkRedirectValues(data.redirect, req) + + if (isDataReq) { + data.props = { + __N_REDIRECT: data.redirect.destination, + } + } else { + handleRedirect(res, data.redirect) + return null + } + } + if ( (dev || isBuildTimeSSG) && !isSerializableProps(pathname, 'getStaticProps', data.props) @@ -623,12 +679,27 @@ export async function renderToHTML( throw new Error(GSSP_NO_RETURNED_VALUE) } - const invalidKeys = Object.keys(data).filter((key) => key !== 'props') + const invalidKeys = Object.keys(data).filter( + (key) => key !== 'props' && key !== 'redirect' + ) if (invalidKeys.length) { throw new Error(invalidKeysMsg('getServerSideProps', invalidKeys)) } + if (data.redirect && typeof data.redirect === 'object') { + checkRedirectValues(data.redirect, req) + + if (isDataReq) { + data.props = { + __N_REDIRECT: data.redirect.destination, + } + } else { + handleRedirect(res, data.redirect) + return null + } + } + if ( (dev || isBuildTimeSSG) && !isSerializableProps(pathname, 'getServerSideProps', data.props) diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index 7e1a0bab3799d..d2e0db2eafce7 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -72,6 +72,11 @@ export { NextApiHandler, } +type Redirect = { + permanent: boolean + destination: string +} + export type GetStaticPropsContext = { params?: Q preview?: boolean @@ -80,6 +85,7 @@ export type GetStaticPropsContext = { export type GetStaticPropsResult

= { props: P + redirect?: Redirect revalidate?: number | boolean } @@ -116,6 +122,7 @@ export type GetServerSidePropsContext< export type GetServerSidePropsResult

= { props: P + redirect?: Redirect } export type GetServerSideProps< diff --git a/test/integration/gssp-redirect/pages/404.js b/test/integration/gssp-redirect/pages/404.js new file mode 100644 index 0000000000000..18a28da908b9a --- /dev/null +++ b/test/integration/gssp-redirect/pages/404.js @@ -0,0 +1,3 @@ +export default function NotFound() { + return

oops not found

+} diff --git a/test/integration/gssp-redirect/pages/gsp-blog/[post].js b/test/integration/gssp-redirect/pages/gsp-blog/[post].js new file mode 100644 index 0000000000000..557d8ebce6723 --- /dev/null +++ b/test/integration/gssp-redirect/pages/gsp-blog/[post].js @@ -0,0 +1,39 @@ +export default function Post(props) { + return ( + <> +

getStaticProps

+

{JSON.stringify(props)}

+ + ) +} + +export const getStaticProps = ({ params }) => { + if (params.post.startsWith('redir')) { + let destination = '/404' + + if (params.post.includes('dest-')) { + destination = params.post.split('dest-').pop().replace(/_/g, '/') + } + + return { + redirect: { + destination, + permanent: params.post.includes('permanent'), + }, + } + } + + return { + props: { + params, + random: Math.random(), + }, + } +} + +export const getStaticPaths = () => { + return { + paths: ['first', 'second'].map((post) => ({ params: { post } })), + fallback: true, + } +} diff --git a/test/integration/gssp-redirect/pages/gssp-blog/[post].js b/test/integration/gssp-redirect/pages/gssp-blog/[post].js new file mode 100644 index 0000000000000..56ff2c4d200a7 --- /dev/null +++ b/test/integration/gssp-redirect/pages/gssp-blog/[post].js @@ -0,0 +1,32 @@ +export default function Post(props) { + return ( + <> +

getServerSideProps

+

{JSON.stringify(props)}

+ + ) +} + +export const getServerSideProps = ({ params }) => { + if (params.post.startsWith('redir')) { + let destination = '/404' + + if (params.post.includes('dest-')) { + destination = params.post.split('dest-').pop() + } + + return { + redirect: { + destination, + permanent: params.post.includes('permanent'), + }, + } + } + + return { + props: { + params, + random: Math.random(), + }, + } +} diff --git a/test/integration/gssp-redirect/test/index.test.js b/test/integration/gssp-redirect/test/index.test.js new file mode 100644 index 0000000000000..9eebb58710612 --- /dev/null +++ b/test/integration/gssp-redirect/test/index.test.js @@ -0,0 +1,81 @@ +/* eslint-env jest */ + +import fs from 'fs-extra' +import { join } from 'path' +import { + findPort, + launchApp, + killApp, + nextBuild, + nextStart, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) +const appDir = join(__dirname, '..') +const nextConfig = join(appDir, 'next.config.js') + +let app +let appPort + +const runTests = () => { + it('should apply temporary redirect when visited directly for GSSP page', async () => {}) + + it('should apply permanent redirect when visited directly for GSSP page', async () => {}) + + it('should apply redirect when fallback GSP page is visited directly (internal)', async () => {}) + + it('should apply redirect when fallback GSP page is visited directly (external)', async () => {}) + + it('should apply redirect when GSSP page is navigated to client-side (internal)', async () => {}) + + it('should apply redirect when GSSP page is navigated to client-side (external)', async () => {}) + + it('should apply redirect when GSP page is navigated to client-side (internal)', async () => {}) + + it('should apply redirect when GSP page is navigated to client-side (external)', async () => {}) +} + +describe('GS(S)P Redirect Support', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await fs.remove(join(appDir, '.next')) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) + + describe('serverless mode', () => { + beforeAll(async () => { + await fs.writeFile( + nextConfig, + `module.exports = { + target: 'experimental-serverless-trace' + }` + ) + await fs.remove(join(appDir, '.next')) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(async () => { + await fs.remove(nextConfig) + await killApp(app) + }) + + runTests() + }) +}) From 1665ff5b5fc175468cc6b2202e75b30a97bf8254 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 27 Aug 2020 21:03:02 -0500 Subject: [PATCH 2/8] Update build-output test --- test/integration/build-output/test/index.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 57314c0f64fb8..b954bff8edf12 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -95,16 +95,16 @@ describe('Build Output', () => { expect(indexSize.endsWith('B')).toBe(true) // should be no bigger than 60.2 kb - expect(parseFloat(indexFirstLoad) - 60.2).toBeLessThanOrEqual(0) + expect(parseFloat(indexFirstLoad) - 60.3).toBeLessThanOrEqual(0) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0) expect(err404Size.endsWith('kB')).toBe(true) - expect(parseFloat(err404FirstLoad) - 63.4).toBeLessThanOrEqual(0) + expect(parseFloat(err404FirstLoad) - 63.5).toBeLessThanOrEqual(0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 59.9).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 60).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { From ed7beaf9999a902c2e71230ef27e32774818c500 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 27 Aug 2020 22:03:05 -0500 Subject: [PATCH 3/8] Add tests for GS(S)P redirect behavior --- .../next/next-server/lib/router/router.ts | 44 ++--- .../gssp-redirect/pages/another.js | 3 + .../gssp-redirect/pages/gsp-blog/[post].js | 11 +- .../gssp-redirect/pages/gssp-blog/[post].js | 3 +- test/integration/gssp-redirect/pages/index.js | 3 + .../gssp-redirect/test/index.test.js | 150 +++++++++++++++++- 6 files changed, 181 insertions(+), 33 deletions(-) create mode 100644 test/integration/gssp-redirect/pages/another.js create mode 100644 test/integration/gssp-redirect/pages/index.js diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 2247b530416f3..b6f8978eb0934 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -595,28 +595,6 @@ export default class Router implements BaseRouter { ) let { error, props, __N_SSG, __N_SSP } = routeInfo - Router.events.emit('beforeHistoryChange', as) - this.changeState(method, url, as, options) - - if (process.env.NODE_ENV !== 'production') { - const appComp: any = this.components['/_app'].Component - ;(window as any).next.isPrerendered = - appComp.getInitialProps === appComp.origGetInitialProps && - !(routeInfo.Component as any).getInitialProps - } - - await this.set(route, pathname!, query, cleanedAs, routeInfo).catch( - (e) => { - if (e.cancelled) error = error || e - else throw e - } - ) - - if (error) { - Router.events.emit('routeChangeError', error, cleanedAs) - throw error - } - // handle redirect on client-transition if ( (__N_SSG || __N_SSP) && @@ -647,6 +625,28 @@ export default class Router implements BaseRouter { return new Promise(() => {}) } + Router.events.emit('beforeHistoryChange', as) + this.changeState(method, url, as, options) + + if (process.env.NODE_ENV !== 'production') { + const appComp: any = this.components['/_app'].Component + ;(window as any).next.isPrerendered = + appComp.getInitialProps === appComp.origGetInitialProps && + !(routeInfo.Component as any).getInitialProps + } + + await this.set(route, pathname!, query, cleanedAs, routeInfo).catch( + (e) => { + if (e.cancelled) error = error || e + else throw e + } + ) + + if (error) { + Router.events.emit('routeChangeError', error, cleanedAs) + throw error + } + if (process.env.__NEXT_SCROLL_RESTORATION) { if (manualScrollRestoration && '_N_X' in options) { window.scrollTo((options as any)._N_X, (options as any)._N_Y) diff --git a/test/integration/gssp-redirect/pages/another.js b/test/integration/gssp-redirect/pages/another.js new file mode 100644 index 0000000000000..bcafcefbda828 --- /dev/null +++ b/test/integration/gssp-redirect/pages/another.js @@ -0,0 +1,3 @@ +export default function Another() { + return

another Page

+} diff --git a/test/integration/gssp-redirect/pages/gsp-blog/[post].js b/test/integration/gssp-redirect/pages/gsp-blog/[post].js index 557d8ebce6723..cd3fae7419a73 100644 --- a/test/integration/gssp-redirect/pages/gsp-blog/[post].js +++ b/test/integration/gssp-redirect/pages/gsp-blog/[post].js @@ -1,4 +1,14 @@ +import { useRouter } from 'next/router' + export default function Post(props) { + const router = useRouter() + + if (typeof window !== 'undefined' && !window.initialHref) { + window.initialHref = window.location.href + } + + if (router.isFallback) return

Loading...

+ return ( <>

getStaticProps

@@ -26,7 +36,6 @@ export const getStaticProps = ({ params }) => { return { props: { params, - random: Math.random(), }, } } diff --git a/test/integration/gssp-redirect/pages/gssp-blog/[post].js b/test/integration/gssp-redirect/pages/gssp-blog/[post].js index 56ff2c4d200a7..d01d19f31ae49 100644 --- a/test/integration/gssp-redirect/pages/gssp-blog/[post].js +++ b/test/integration/gssp-redirect/pages/gssp-blog/[post].js @@ -12,7 +12,7 @@ export const getServerSideProps = ({ params }) => { let destination = '/404' if (params.post.includes('dest-')) { - destination = params.post.split('dest-').pop() + destination = params.post.split('dest-').pop().replace(/_/g, '/') } return { @@ -26,7 +26,6 @@ export const getServerSideProps = ({ params }) => { return { props: { params, - random: Math.random(), }, } } diff --git a/test/integration/gssp-redirect/pages/index.js b/test/integration/gssp-redirect/pages/index.js new file mode 100644 index 0000000000000..f204bab747120 --- /dev/null +++ b/test/integration/gssp-redirect/pages/index.js @@ -0,0 +1,3 @@ +export default function Index() { + return

Index Page

+} diff --git a/test/integration/gssp-redirect/test/index.test.js b/test/integration/gssp-redirect/test/index.test.js index 9eebb58710612..b527573828d99 100644 --- a/test/integration/gssp-redirect/test/index.test.js +++ b/test/integration/gssp-redirect/test/index.test.js @@ -1,6 +1,8 @@ /* eslint-env jest */ +import url from 'url' import fs from 'fs-extra' +import webdriver from 'next-webdriver' import { join } from 'path' import { findPort, @@ -8,6 +10,8 @@ import { killApp, nextBuild, nextStart, + fetchViaHTTP, + check, } from 'next-test-utils' jest.setTimeout(1000 * 60 * 2) @@ -18,21 +22,151 @@ let app let appPort const runTests = () => { - it('should apply temporary redirect when visited directly for GSSP page', async () => {}) + it('should apply temporary redirect when visited directly for GSSP page', async () => { + const res = await fetchViaHTTP( + appPort, + '/gssp-blog/redirect-1', + undefined, + { + redirect: 'manual', + } + ) + expect(res.status).toBe(307) + + const { pathname } = url.parse(res.headers.get('location')) + + expect(pathname).toBe('/404') + }) + + it('should apply permanent redirect when visited directly for GSSP page', async () => { + const res = await fetchViaHTTP( + appPort, + '/gssp-blog/redirect-permanent', + undefined, + { + redirect: 'manual', + } + ) + expect(res.status).toBe(308) + + const { pathname } = url.parse(res.headers.get('location')) + + expect(pathname).toBe('/404') + expect(res.headers.get('refresh')).toMatch(/url=\/404/) + }) - it('should apply permanent redirect when visited directly for GSSP page', async () => {}) + it('should apply redirect when fallback GSP page is visited directly (internal dynamic)', async () => { + const browser = await webdriver( + appPort, + '/gsp-blog/redirect-dest-_gsp-blog_first' + ) - it('should apply redirect when fallback GSP page is visited directly (internal)', async () => {}) + await browser.waitForElementByCss('#gsp') - it('should apply redirect when fallback GSP page is visited directly (external)', async () => {}) + const props = JSON.parse(await browser.elementByCss('#props').text()) + expect(props).toEqual({ + params: { + post: 'first', + }, + }) + const initialHref = await browser.eval(() => window.initialHref) + const { pathname } = url.parse(initialHref) + expect(pathname).toBe('/gsp-blog/redirect-dest-_gsp-blog_first') + }) + + it('should apply redirect when fallback GSP page is visited directly (internal normal)', async () => { + const browser = await webdriver(appPort, '/gsp-blog/redirect-dest-_') + + await browser.waitForElementByCss('#index') + + const initialHref = await browser.eval(() => window.initialHref) + const { pathname } = url.parse(initialHref) + expect(pathname).toBe('/gsp-blog/redirect-dest-_') + }) - it('should apply redirect when GSSP page is navigated to client-side (internal)', async () => {}) + it('should apply redirect when fallback GSP page is visited directly (external)', async () => { + const browser = await webdriver(appPort, '/gsp-blog/redirect-dest-_missing') - it('should apply redirect when GSSP page is navigated to client-side (external)', async () => {}) + await check( + () => browser.eval(() => document.documentElement.innerHTML), + /oops not found/ + ) + + const initialHref = await browser.eval(() => window.initialHref) + expect(initialHref).toBe(null) + + const curUrl = await browser.url() + const { pathname } = url.parse(curUrl) + expect(pathname).toBe('/missing') + }) + + it('should apply redirect when GSSP page is navigated to client-side (internal dynamic)', async () => { + const browser = await webdriver( + appPort, + '/gssp-blog/redirect-dest-_gssp-blog_first' + ) + + await browser.waitForElementByCss('#gssp') + + const props = JSON.parse(await browser.elementByCss('#props').text()) + expect(props).toEqual({ + params: { + post: 'first', + }, + }) + }) - it('should apply redirect when GSP page is navigated to client-side (internal)', async () => {}) + it('should apply redirect when GSSP page is navigated to client-side (internal normal)', async () => { + const browser = await webdriver(appPort, '/') - it('should apply redirect when GSP page is navigated to client-side (external)', async () => {}) + await browser.eval(`(function () { + window.next.router.push('/gssp-blog/redirect-dest-_another') + })()`) + await browser.waitForElementByCss('#another') + }) + + it('should apply redirect when GSSP page is navigated to client-side (external)', async () => { + const browser = await webdriver(appPort, '/') + + await browser.eval(`(function () { + window.next.router.push('/gssp-blog/redirect-dest-_gssp-blog_first') + })()`) + await browser.waitForElementByCss('#gssp') + + const props = JSON.parse(await browser.elementByCss('#props').text()) + + expect(props).toEqual({ + params: { + post: 'first', + }, + }) + }) + + it('should apply redirect when GSP page is navigated to client-side (internal)', async () => { + const browser = await webdriver(appPort, '/') + + await browser.eval(`(function () { + window.next.router.push('/gsp-blog/redirect-dest-_another') + })()`) + await browser.waitForElementByCss('#another') + }) + + it('should apply redirect when GSP page is navigated to client-side (external)', async () => { + const browser = await webdriver(appPort, '/') + + await browser.eval(`(function () { + window.next.router.push('/gsp-blog/redirect-dest-_gsp-blog_first') + })()`) + await browser.waitForElementByCss('#gsp') + + const props = JSON.parse(await browser.elementByCss('#props').text()) + + expect(props).toEqual({ + params: { + post: 'first', + }, + }) + }) } describe('GS(S)P Redirect Support', () => { From b8154389456f16256de84e3be2ede2bc763e3b56 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 28 Aug 2020 10:45:41 -0500 Subject: [PATCH 4/8] Add error for redirect during prerendering --- errors/gsp-redirect-during-prerender.md | 13 +++++++ packages/next/next-server/server/render.tsx | 7 ++++ .../gssp-redirect/test/index.test.js | 38 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 errors/gsp-redirect-during-prerender.md diff --git a/errors/gsp-redirect-during-prerender.md b/errors/gsp-redirect-during-prerender.md new file mode 100644 index 0000000000000..964d7bf1db1ff --- /dev/null +++ b/errors/gsp-redirect-during-prerender.md @@ -0,0 +1,13 @@ +# Redirect During getStaticProps Prerendering + +#### Why This Error Occurred + +The `redirect` value was returned from `getStaticProps` during prerendering which is invalid. + +#### Possible Ways to Fix It + +Remove any paths that result in a redirect from being prerendered in `getStaticPaths` and enable `fallback: true` to handle redirecting for these pages. + +### Useful Links + +- [Data Fetching Documentation](https://nextjs.org/docs/basic-features/data-fetching#getstaticprops-static-generation) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 62a85923fdff7..5e548b2410d53 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -591,6 +591,13 @@ export async function renderToHTML( if (data.redirect && typeof data.redirect === 'object') { checkRedirectValues(data.redirect, req) + if (isBuildTimeSSG) { + throw new Error( + `\`redirect\` can not be returned from getStaticProps during prerendering (${req.url})\n` + + `See more info here: https://err.sh/next.js/gsp-redirect-during-prerender` + ) + } + if (isDataReq) { data.props = { __N_REDIRECT: data.redirect.destination, diff --git a/test/integration/gssp-redirect/test/index.test.js b/test/integration/gssp-redirect/test/index.test.js index b527573828d99..b3b04074e0fc1 100644 --- a/test/integration/gssp-redirect/test/index.test.js +++ b/test/integration/gssp-redirect/test/index.test.js @@ -212,4 +212,42 @@ describe('GS(S)P Redirect Support', () => { runTests() }) + + it('should error for redirect during prerendering', async () => { + await fs.mkdirp(join(appDir, 'pages/invalid')) + await fs.writeFile( + join(appDir, 'pages', 'invalid', '[slug].js'), + ` + export default function Post(props) { + return "hi" + } + + export const getStaticProps = ({ params }) => { + return { + redirect: { + permanent: true, + destination: '/another' + } + } + } + + export const getStaticPaths = () => { + return { + paths: ['first', 'second'].map((slug) => ({ params: { slug } })), + fallback: true, + } + } + ` + ) + const { stdout, stderr } = await nextBuild(appDir, undefined, { + stdout: true, + stderr: true, + }) + const output = stdout + stderr + await fs.remove(join(appDir, 'pages/invalid')) + + expect(output).toContain( + '`redirect` can not be returned from getStaticProps during prerendering' + ) + }) }) From c2a4407ad5cbbc44149146f9f2f61133befd74b6 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 31 Aug 2020 10:13:58 -0500 Subject: [PATCH 5/8] Update todo and rename redirect key --- packages/next/next-server/server/render.tsx | 28 +++++++++++-------- packages/next/types/index.d.ts | 4 +-- .../gssp-redirect/pages/gsp-blog/[post].js | 2 +- .../gssp-redirect/pages/gssp-blog/[post].js | 2 +- .../gssp-redirect/test/index.test.js | 2 +- yarn.lock | 7 +++-- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 5e548b2410d53..51274610c0d17 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -294,7 +294,6 @@ function checkRedirectValues(redirect: Redirect, req: IncomingMessage) { } function handleRedirect(res: ServerResponse, redirect: Redirect) { - // TODO: this should error if a redirect is returned while prerendering const statusCode = redirect.permanent ? PERMANENT_REDIRECT_STATUS : TEMPORARY_REDIRECT_STATUS @@ -577,7 +576,8 @@ export async function renderToHTML( } const invalidKeys = Object.keys(data).filter( - (key) => key !== 'revalidate' && key !== 'props' && key !== 'redirect' + (key) => + key !== 'revalidate' && key !== 'props' && key !== 'unstable_redirect' ) if (invalidKeys.includes('unstable_revalidate')) { @@ -588,8 +588,11 @@ export async function renderToHTML( throw new Error(invalidKeysMsg('getStaticProps', invalidKeys)) } - if (data.redirect && typeof data.redirect === 'object') { - checkRedirectValues(data.redirect, req) + if ( + data.unstable_redirect && + typeof data.unstable_redirect === 'object' + ) { + checkRedirectValues(data.unstable_redirect, req) if (isBuildTimeSSG) { throw new Error( @@ -600,10 +603,10 @@ export async function renderToHTML( if (isDataReq) { data.props = { - __N_REDIRECT: data.redirect.destination, + __N_REDIRECT: data.unstable_redirect.destination, } } else { - handleRedirect(res, data.redirect) + handleRedirect(res, data.unstable_redirect) return null } } @@ -687,22 +690,25 @@ export async function renderToHTML( } const invalidKeys = Object.keys(data).filter( - (key) => key !== 'props' && key !== 'redirect' + (key) => key !== 'props' && key !== 'unstable_redirect' ) if (invalidKeys.length) { throw new Error(invalidKeysMsg('getServerSideProps', invalidKeys)) } - if (data.redirect && typeof data.redirect === 'object') { - checkRedirectValues(data.redirect, req) + if ( + data.unstable_redirect && + typeof data.unstable_redirect === 'object' + ) { + checkRedirectValues(data.unstable_redirect, req) if (isDataReq) { data.props = { - __N_REDIRECT: data.redirect.destination, + __N_REDIRECT: data.unstable_redirect.destination, } } else { - handleRedirect(res, data.redirect) + handleRedirect(res, data.unstable_redirect) return null } } diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index d2e0db2eafce7..bcc0720abddeb 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -85,7 +85,7 @@ export type GetStaticPropsContext = { export type GetStaticPropsResult

= { props: P - redirect?: Redirect + unstable_redirect?: Redirect revalidate?: number | boolean } @@ -122,7 +122,7 @@ export type GetServerSidePropsContext< export type GetServerSidePropsResult

= { props: P - redirect?: Redirect + unstable_redirect?: Redirect } export type GetServerSideProps< diff --git a/test/integration/gssp-redirect/pages/gsp-blog/[post].js b/test/integration/gssp-redirect/pages/gsp-blog/[post].js index cd3fae7419a73..18166abd0b48e 100644 --- a/test/integration/gssp-redirect/pages/gsp-blog/[post].js +++ b/test/integration/gssp-redirect/pages/gsp-blog/[post].js @@ -26,7 +26,7 @@ export const getStaticProps = ({ params }) => { } return { - redirect: { + unstable_redirect: { destination, permanent: params.post.includes('permanent'), }, diff --git a/test/integration/gssp-redirect/pages/gssp-blog/[post].js b/test/integration/gssp-redirect/pages/gssp-blog/[post].js index d01d19f31ae49..4c6af348a5564 100644 --- a/test/integration/gssp-redirect/pages/gssp-blog/[post].js +++ b/test/integration/gssp-redirect/pages/gssp-blog/[post].js @@ -16,7 +16,7 @@ export const getServerSideProps = ({ params }) => { } return { - redirect: { + unstable_redirect: { destination, permanent: params.post.includes('permanent'), }, diff --git a/test/integration/gssp-redirect/test/index.test.js b/test/integration/gssp-redirect/test/index.test.js index b3b04074e0fc1..bdbb842bd51c6 100644 --- a/test/integration/gssp-redirect/test/index.test.js +++ b/test/integration/gssp-redirect/test/index.test.js @@ -224,7 +224,7 @@ describe('GS(S)P Redirect Support', () => { export const getStaticProps = ({ params }) => { return { - redirect: { + unstable_redirect: { permanent: true, destination: '/another' } diff --git a/yarn.lock b/yarn.lock index 2267aa0f15a34..43baaffaca82d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7959,9 +7959,10 @@ finalhandler@~1.1.2: statuses "~1.5.0" unpipe "~1.0.0" -finally-polyfill@0.1.0: - version "0.1.0" - resolved "https://registry.yarnpkg.com/finally-polyfill/-/finally-polyfill-0.1.0.tgz#2a17b16581d9477db16a703c7b79a898ac0b7d50" +finally-polyfill@0.2.0: + version "0.2.0" + resolved "https://registry.yarnpkg.com/finally-polyfill/-/finally-polyfill-0.2.0.tgz#1b34c6e555a6c1603d2ae046e2e176d08687bfdb" + integrity sha512-3w46w5Vo4TRtk5jrLT3c8ITGxnPJhMAg3Ogbj4nmgL6thNep9+UgBgk+IRVmRpZDbwNkR7tyGsE3S3J4Qt2zVw== find-cache-dir@3.3.1, find-cache-dir@^3.0.0, find-cache-dir@^3.3.1: version "3.3.1" From a3b394a3b8a21e251d09ef5106a7899802b6d15e Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 7 Sep 2020 11:56:40 +0200 Subject: [PATCH 6/8] Change type of unstable_redirect return --- packages/next/types/index.d.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index bcc0720abddeb..3d89a5b430741 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -83,11 +83,14 @@ export type GetStaticPropsContext = { previewData?: any } -export type GetStaticPropsResult

= { - props: P - unstable_redirect?: Redirect - revalidate?: number | boolean -} +export type GetStaticPropsResult

= + | { + props: P + revalidate?: number | boolean + } + | { + unstable_redirect: Redirect + } export type GetStaticProps< P extends { [key: string]: any } = { [key: string]: any }, @@ -120,10 +123,13 @@ export type GetServerSidePropsContext< previewData?: any } -export type GetServerSidePropsResult

= { - props: P - unstable_redirect?: Redirect -} +export type GetServerSidePropsResult

= + | { + props: P + } + | { + unstable_redirect: Redirect + } export type GetServerSideProps< P extends { [key: string]: any } = { [key: string]: any }, From 461c52abe9db4e483c9a2279eaca6cb9c3f44f4c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 7 Sep 2020 13:06:21 -0500 Subject: [PATCH 7/8] Update type --- packages/next/types/index.d.ts | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index a11dbdd4f27ab..1c047e889c81b 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -83,14 +83,11 @@ export type GetStaticPropsContext = { previewData?: any } -export type GetStaticPropsResult

= - | { - props: P - revalidate?: number | boolean - } - | { - unstable_redirect: Redirect - } +export type GetStaticPropsResult

= { + props?: P + revalidate?: number | boolean + unstable_redirect?: Redirect +} export type GetStaticProps< P extends { [key: string]: any } = { [key: string]: any }, @@ -125,13 +122,10 @@ export type GetServerSidePropsContext< previewData?: any } -export type GetServerSidePropsResult

= - | { - props: P - } - | { - unstable_redirect: Redirect - } +export type GetServerSidePropsResult

= { + props?: P + unstable_redirect?: Redirect +} export type GetServerSideProps< P extends { [key: string]: any } = { [key: string]: any }, From e3bb8463a00926b4c0e69f6c2dfe68250e690e82 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 7 Sep 2020 14:33:59 -0500 Subject: [PATCH 8/8] Update size tests --- test/integration/build-output/test/index.test.js | 6 +++--- test/integration/size-limit/test/index.test.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index b954bff8edf12..5acf77f1072f2 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -95,16 +95,16 @@ describe('Build Output', () => { expect(indexSize.endsWith('B')).toBe(true) // should be no bigger than 60.2 kb - expect(parseFloat(indexFirstLoad) - 60.3).toBeLessThanOrEqual(0) + expect(parseFloat(indexFirstLoad) - 60.4).toBeLessThanOrEqual(0) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0) expect(err404Size.endsWith('kB')).toBe(true) - expect(parseFloat(err404FirstLoad) - 63.5).toBeLessThanOrEqual(0) + expect(parseFloat(err404FirstLoad) - 63.6).toBeLessThanOrEqual(0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 60).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 60.1).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 464ad0a02be65..6bc2f88f42233 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -80,7 +80,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 278 * 1024 + const delta = responseSizesBytes - 279 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target })