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

Update to not modify req.url for getServerSideProps requests #11637

Merged
merged 3 commits into from
Apr 6, 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
34 changes: 18 additions & 16 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ export default class Server {
.replace(/\.json$/, '')
.replace(/\/index$/, '/')

req.url = pathname
const parsedUrl = parseUrl(pathname, true)
await this.render(
req,
Expand Down Expand Up @@ -794,9 +793,14 @@ export default class Server {

const url: any = req.url

// we allow custom servers to call render for all URLs
// so check if we need to serve a static _next file or not.
// we don't modify the URL for _next/data request but still
// call render so we special case this to prevent an infinite loop
if (
url.match(/^\/_next\//) ||
(this.hasStaticDir && url.match(/^\/static\//))
!query._nextDataReq &&
(url.match(/^\/_next\//) ||
(this.hasStaticDir && url.match(/^\/static\//)))
) {
return this.handleRequest(req, res, parsedUrl)
}
Expand Down Expand Up @@ -926,17 +930,6 @@ export default class Server {
const isDataReq = !!query._nextDataReq
delete query._nextDataReq

// Serverless requests need its URL transformed back into the original
// request path (to emulate lambda behavior in production)
if (isLikeServerless && isDataReq) {
let { pathname } = parseUrl(req.url || '', true)
pathname = !pathname || pathname === '/' ? '/index' : pathname
req.url = formatUrl({
pathname: `/_next/data/${this.buildId}${pathname}.json`,
query,
})
}

let previewData: string | false | object | undefined
let isPreviewMode = false

Expand Down Expand Up @@ -1009,10 +1002,19 @@ export default class Server {
return html
}

// Compute the SPR cache key
const urlPathname = `${parseUrl(req.url || '').pathname!}${
// Compute the iSSG cache key
let urlPathname = `${parseUrl(req.url || '').pathname!}${
query.amp ? '.amp' : ''
}`

// remove /_next/data prefix from urlPathname so it matches
// for direct page visit and /_next/data visit
if (isDataReq && urlPathname.includes(this.buildId)) {
urlPathname = (urlPathname.split(this.buildId).pop() || '/')
.replace(/\.json$/, '')
.replace(/\/index$/, '/')
}

const ssgCacheKey = isPreviewMode
? `__` + nanoid() // Preview mode uses a throw away key to not coalesce preview invokes
: urlPathname
Expand Down
29 changes: 29 additions & 0 deletions test/integration/getserversideprops/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import App from 'next/app'

class MyApp extends App {
static async getInitialProps(ctx) {
const { req, query, pathname, asPath } = ctx.ctx
let pageProps = {}

if (ctx.Component.getInitialProps) {
pageProps = await ctx.Component.getInitialProps(ctx.ctx)
}

return {
appProps: {
url: (req || {}).url,
query,
pathname,
asPath,
},
pageProps,
}
}

render() {
const { Component, pageProps, appProps } = this.props
return <Component {...pageProps} appProps={appProps} />
}
}

export default MyApp
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ export async function getServerSideProps({ params }) {
}
}

export default ({ post, time, params }) => {
export default ({ post, time, params, appProps }) => {
return (
<>
<p>Post: {post}</p>
<span>time: {time}</span>
<div id="params">{JSON.stringify(params)}</div>
<div id="query">{JSON.stringify(useRouter().query)}</div>
<div id="app-query">{JSON.stringify(appProps.query)}</div>
<div id="app-url">{appProps.url}</div>
<Link href="/">
<a id="home">to home</a>
</Link>
Expand Down
4 changes: 3 additions & 1 deletion test/integration/getserversideprops/pages/something.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export async function getServerSideProps({ params, query }) {
}
}

export default ({ world, time, params, random, query }) => {
export default ({ world, time, params, random, query, appProps }) => {
return (
<>
<p>hello: {world}</p>
Expand All @@ -23,6 +23,8 @@ export default ({ world, time, params, random, query }) => {
<div id="params">{JSON.stringify(params)}</div>
<div id="initial-query">{JSON.stringify(query)}</div>
<div id="query">{JSON.stringify(useRouter().query)}</div>
<div id="app-query">{JSON.stringify(appProps.query)}</div>
<div id="app-url">{appProps.url}</div>
<Link href="/">
<a id="home">to home</a>
</Link>
Expand Down
40 changes: 40 additions & 0 deletions test/integration/getserversideprops/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,46 @@ const runTests = (dev = false) => {
expect(data.pageProps.params).toEqual({ path: ['first'] })
})

it('should have original req.url for /_next/data request dynamic page', async () => {
const curUrl = `/_next/data/${buildId}/blog/post-1.json`
const data = await renderViaHTTP(appPort, curUrl)
const { appProps } = JSON.parse(data)

expect(appProps).toEqual({
url: curUrl,
query: { post: 'post-1' },
asPath: curUrl,
pathname: '/blog/[post]',
})
})

it('should have original req.url for /_next/data request', async () => {
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
const curUrl = `/_next/data/${buildId}/something.json`
const data = await renderViaHTTP(appPort, curUrl)
const { appProps } = JSON.parse(data)

expect(appProps).toEqual({
url: curUrl,
query: {},
asPath: curUrl,
pathname: '/something',
})
})

it('should have correct req.url and query for direct visit dynamic page', async () => {
const html = await renderViaHTTP(appPort, '/blog/post-1')
const $ = cheerio.load(html)
expect($('#app-url').text()).toContain('/blog/post-1')
expect(JSON.parse($('#app-query').text())).toEqual({ post: 'post-1' })
})

it('should have correct req.url and query for direct visit', async () => {
const html = await renderViaHTTP(appPort, '/something')
const $ = cheerio.load(html)
expect($('#app-url').text()).toContain('/something')
expect(JSON.parse($('#app-query').text())).toEqual({})
})

it('should return data correctly', async () => {
const data = JSON.parse(
await renderViaHTTP(appPort, `/_next/data/${buildId}/something.json`)
Expand Down