Skip to content

Commit

Permalink
Ensure basePath behavior with GS(S)P redirect (#18988)
Browse files Browse the repository at this point in the history
This ensures we match the `basePath` handling for redirects in `next.config.js` with redirects from `getStaticProps` and `getServerSideProps` and also adds a separate test suite to ensure GS(S)P redirects with `basePath` work correctly

Fixes: #18984
Closes: #18892
  • Loading branch information
ijjk authored Nov 11, 2020
1 parent a429a47 commit 48acc47
Show file tree
Hide file tree
Showing 13 changed files with 791 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,15 @@ const nextServerlessLoader: loader.Loader = function () {
} else if (renderOpts.isRedirect && !_nextData) {
const redirect = {
destination: renderOpts.pageData.pageProps.__N_REDIRECT,
statusCode: renderOpts.pageData.pageProps.__N_REDIRECT_STATUS
statusCode: renderOpts.pageData.pageProps.__N_REDIRECT_STATUS,
basePath: renderOpts.pageData.pageProps.__N_REDIRECT_BASE_PATH
}
const statusCode = getRedirectStatus(redirect)
if ("${basePath}" && redirect.basePath !== false) {
redirect.destination = \`${basePath}\${redirect.destination}\`
}
if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', \`0;url=\${redirect.destination}\`)
}
Expand Down
13 changes: 8 additions & 5 deletions packages/next/lib/load-custom-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,20 @@ export type Rewrite = {
basePath?: false
}

export type Redirect = Rewrite & {
statusCode?: number
permanent?: boolean
}

export type Header = {
source: string
basePath?: false
headers: Array<{ key: string; value: string }>
}

// internal type used for validation (not user facing)
export type Redirect = Rewrite & {
statusCode?: number
permanent?: boolean
destination: string
basePath?: false
}

export const allowedStatusCodes = new Set([301, 302, 303, 307, 308])

export function getRedirectStatus(route: {
Expand Down
9 changes: 7 additions & 2 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -815,10 +815,15 @@ export default class Router implements BaseRouter {
// it's not
if (destination.startsWith('/')) {
const parsedHref = parseRelativeUrl(destination)
this._resolveHref(parsedHref, pages)
this._resolveHref(parsedHref, pages, false)

if (pages.includes(parsedHref.pathname)) {
return this.change(method, destination, destination, options)
const { url: newUrl, as: newAs } = prepareUrlAs(
this,
destination,
destination
)
return this.change(method, newUrl, newAs, options)
}
}

Expand Down
6 changes: 6 additions & 0 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1271,8 +1271,14 @@ export default class Server {
const redirect = {
destination: pageData.pageProps.__N_REDIRECT,
statusCode: pageData.pageProps.__N_REDIRECT_STATUS,
basePath: pageData.pageProps.__N_REDIRECT_BASE_PATH,
}
const statusCode = getRedirectStatus(redirect)
const { basePath } = this.nextConfig

if (basePath && redirect.basePath !== false) {
redirect.destination = `${basePath}${redirect.destination}`
}

if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', `0;url=${redirect.destination}`)
Expand Down
31 changes: 22 additions & 9 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import optimizeAmp from './optimize-amp'
import {
allowedStatusCodes,
getRedirectStatus,
Redirect,
} from '../../lib/load-custom-routes'

function noRouter() {
Expand Down Expand Up @@ -290,18 +291,12 @@ const invalidKeysMsg = (methodName: string, invalidKeys: string[]) => {
)
}

type Redirect = {
permanent: boolean
destination: string
statusCode?: number
}

function checkRedirectValues(
redirect: Redirect,
req: IncomingMessage,
method: 'getStaticProps' | 'getServerSideProps'
) {
const { destination, permanent, statusCode } = redirect
const { destination, permanent, statusCode, basePath } = redirect
let errors: string[] = []

const hasStatusCode = typeof statusCode !== 'undefined'
Expand All @@ -326,6 +321,14 @@ function checkRedirectValues(
)
}

const basePathType = typeof basePath

if (basePathType !== 'undefined' && basePathType !== 'boolean') {
errors.push(
`\`basePath\` should be undefined or a false, received ${basePathType}`
)
}

if (errors.length > 0) {
throw new Error(
`Invalid redirect object returned from ${method} for ${req.url}\n` +
Expand Down Expand Up @@ -657,7 +660,7 @@ export async function renderToHTML(
data.redirect &&
typeof data.redirect === 'object'
) {
checkRedirectValues(data.redirect, req, 'getStaticProps')
checkRedirectValues(data.redirect as Redirect, req, 'getStaticProps')

if (isBuildTimeSSG) {
throw new Error(
Expand All @@ -670,6 +673,9 @@ export async function renderToHTML(
__N_REDIRECT: data.redirect.destination,
__N_REDIRECT_STATUS: getRedirectStatus(data.redirect),
}
if (typeof data.redirect.basePath !== 'undefined') {
;(data as any).props.__N_REDIRECT_BASE_PATH = data.redirect.basePath
}
;(renderOpts as any).isRedirect = true
}

Expand Down Expand Up @@ -791,11 +797,18 @@ export async function renderToHTML(
}

if ('redirect' in data && typeof data.redirect === 'object') {
checkRedirectValues(data.redirect, req, 'getServerSideProps')
checkRedirectValues(
data.redirect as Redirect,
req,
'getServerSideProps'
)
;(data as any).props = {
__N_REDIRECT: data.redirect.destination,
__N_REDIRECT_STATUS: getRedirectStatus(data.redirect),
}
if (typeof data.redirect.basePath !== 'undefined') {
;(data as any).props.__N_REDIRECT_BASE_PATH = data.redirect.basePath
}
;(renderOpts as any).isRedirect = true
}

Expand Down
17 changes: 12 additions & 5 deletions packages/next/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ declare module 'react' {
}
}

export type Redirect =
| {
statusCode: 301 | 302 | 303 | 307 | 308
destination: string
basePath?: false
}
| {
permanent: boolean
destination: string
basePath?: false
}

/**
* `Page` type, use it as a guide to create `pages`.
*/
Expand Down Expand Up @@ -72,11 +84,6 @@ export {
NextApiHandler,
}

type Redirect = {
permanent: boolean
destination: string
}

export type GetStaticPropsContext<Q extends ParsedUrlQuery = ParsedUrlQuery> = {
params?: Q
preview?: boolean
Expand Down
3 changes: 3 additions & 0 deletions test/integration/gssp-redirect-base-path/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
basePath: '/docs',
}
3 changes: 3 additions & 0 deletions test/integration/gssp-redirect-base-path/pages/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function NotFound() {
return <p>oops not found</p>
}
3 changes: 3 additions & 0 deletions test/integration/gssp-redirect-base-path/pages/another.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Another() {
return <p id="another">another Page</p>
}
76 changes: 76 additions & 0 deletions test/integration/gssp-redirect-base-path/pages/gsp-blog/[post].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { useRouter } from 'next/router'

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

if (typeof window === 'undefined') {
if (router.query.post?.startsWith('redir')) {
console.log(router)
throw new Error('render should not occur for redirect')
}
}

if (typeof window !== 'undefined' && !window.initialHref) {
window.initialHref = window.location.href
}

if (router.isFallback) return <p>Loading...</p>

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

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, '/')
}

let permanent = undefined
let statusCode = undefined

if (params.post.includes('statusCode-')) {
permanent = parseInt(
params.post.split('statusCode-').pop().split('-').shift(),
10
)
}

if (params.post.includes('permanent')) {
permanent = true
} else if (!statusCode) {
permanent = false
}

const redirect = {
destination,
permanent,
statusCode,
}

if (params.post.includes('no-basepath-')) {
redirect.basePath = false
}

return { redirect }
}

return {
props: {
params,
},
}
}

export const getStaticPaths = () => {
return {
paths: ['first', 'second'].map((post) => ({ params: { post } })),
fallback: true,
}
}
63 changes: 63 additions & 0 deletions test/integration/gssp-redirect-base-path/pages/gssp-blog/[post].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { useRouter } from 'next/router'

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

if (typeof window === 'undefined') {
if (router.query.post?.startsWith('redir')) {
console.log(router)
throw new Error('render should not occur for redirect')
}
}

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

export const getServerSideProps = ({ params }) => {
if (params.post.startsWith('redir')) {
let destination = '/404'

if (params.post.includes('dest-')) {
destination = params.post.split('dest-').pop().replace(/_/g, '/')
}

let permanent = undefined
let statusCode = undefined

if (params.post.includes('statusCode-')) {
statusCode = parseInt(
params.post.split('statusCode-').pop().split('-').shift(),
10
)
}

if (params.post.includes('permanent')) {
permanent = true
} else if (!statusCode) {
permanent = false
}

const redirect = {
destination,
permanent,
statusCode,
}

if (params.post.includes('no-basepath-')) {
redirect.basePath = false
}

return { redirect }
}

return {
props: {
params,
},
}
}
7 changes: 7 additions & 0 deletions test/integration/gssp-redirect-base-path/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Index() {
if (typeof window !== 'undefined' && !window.initialHref) {
window.initialHref = window.location.href
}

return <p id="index">Index Page</p>
}
Loading

0 comments on commit 48acc47

Please sign in to comment.