Skip to content

Commit

Permalink
fix: ensure that postponed requests can be resumed in minimal mode
Browse files Browse the repository at this point in the history
  • Loading branch information
wyattjoh committed Nov 4, 2023
1 parent 54a0e05 commit ec49f8b
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 94 deletions.
98 changes: 37 additions & 61 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// @ts-expect-error internal field not publicly exposed
isExperimentalCompile: this.nextConfig.experimental.isExperimentalCompile,
experimental: {
ppr: this.nextConfig.experimental.ppr === true,
ppr:
this.enabledDirectories.app &&
this.nextConfig.experimental.ppr === true,
},
}

Expand Down Expand Up @@ -611,50 +613,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
return false
}

protected handleNextPostponedRequest: RouteHandler = async (
req,
_res,
parsedUrl
) => {
if (
!parsedUrl.pathname ||
!this.normalizers.postponed.match(parsedUrl.pathname) ||
req.method !== 'POST'
) {
return false
}

parsedUrl.pathname = this.normalizers.postponed.normalize(
parsedUrl.pathname,
true
)

if (req.url) {
const parsed = parseUrl(req.url)
parsed.pathname = parsedUrl.pathname
req.url = formatUrl(parsed)
}

// If we've already read the postponed body, then we don't need to do so
// again.
let postponed = getRequestMeta(req, 'postponed') ?? ''
if (postponed) return false

// Read the body in chunks. If it errors here it's because the chunks
// being decoded are not strings.
const body: Array<Buffer> = []
for await (const chunk of req.body) {
body.push(chunk)
}

// Decode the body as a string.
postponed = Buffer.concat(body).toString('utf8')

addRequestMeta(req, 'postponed', postponed)

return false
}

private handleNextDataRequest: RouteHandler = async (req, res, parsedUrl) => {
const middleware = this.getMiddleware()
const params = matchNextDataPathname(parsedUrl.pathname)
Expand Down Expand Up @@ -939,17 +897,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
)
}

let finished: boolean = false
if (this.minimalMode && this.enabledDirectories.app) {
finished = await this.handleRSCRequest(req, res, parsedUrl)
if (finished) return

if (this.nextConfig.experimental.ppr) {
finished = await this.handleNextPostponedRequest(req, res, parsedUrl)
if (finished) return
}
}

req.headers['x-forwarded-host'] ??= `${this.hostname}:${this.port}`
req.headers['x-forwarded-port'] ??= this.port?.toString()
const { originalRequest } = req as NodeNextRequest
Expand All @@ -959,8 +906,16 @@ export default abstract class Server<ServerOptions extends Options = Options> {
: 'http'
req.headers['x-forwarded-for'] ??= originalRequest.socket?.remoteAddress

// This should be done before any normalization of the pathname happens as
// it captures the initial URL.
this.attachRequestMeta(req, parsedUrl)

let finished: boolean = false
if (this.minimalMode && this.enabledDirectories.app) {
finished = await this.handleRSCRequest(req, res, parsedUrl)
if (finished) return
}

const domainLocale = this.i18nProvider?.detectDomainLocale(
getHostname(parsedUrl, req.headers)
)
Expand Down Expand Up @@ -1006,13 +961,32 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// For ISR the URL is normalized to the prerenderPath so if
// it's a data request the URL path will be the data URL,
// basePath is already stripped by this point
if (this.normalizers.data.match(parsedUrl.pathname)) {
if (
this.enabledDirectories.pages &&
this.normalizers.data.match(parsedUrl.pathname)
) {
parsedUrl.query.__nextDataReq = '1'
}
// In minimal mode, if PPR is enabled, then we should check to see if
// the matched path is a postponed path, and if it is, handle it.
else if (
this.minimalMode &&
this.renderOpts.experimental.ppr &&
this.normalizers.postponed.match(matchedPath) &&
req.method === 'POST'
) {
// Decode the postponed state from the request body, it will come as
// an array of buffers, so collect them and then concat them to form
// the string.
const body: Array<Buffer> = []
for await (const chunk of req.body) {
body.push(chunk)
}
const postponed = Buffer.concat(body).toString('utf8')

addRequestMeta(req, 'postponed', postponed)
}

// We should attempt route normalization for these routes, but don't
// use them to determine the kind of the request, this is just now
// used for routing.
matchedPath = this.normalize(matchedPath)
const normalizedUrlPath = this.normalize(parsedUrl.pathname)

Expand Down Expand Up @@ -2725,7 +2699,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// and the revalidate options.
const onCacheEntry = getRequestMeta(req, 'onCacheEntry')
if (onCacheEntry) {
const finished = await onCacheEntry(cacheEntry)
const finished = await onCacheEntry(cacheEntry, {
url: getRequestMeta(req, 'initURL'),
})
if (finished) {
// TODO: maybe we have to end the request?
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ describe('PostponedPathnameNormalizer', () => {
describe('match', () => {
it('should not match if it is disabled', () => {
const pathnames = [
'/_next/postponed/foo',
'/_next/postponed/bar',
'/_next/postponed/baz',
'/_next/postponed/resume/foo',
'/_next/postponed/resume/bar',
'/_next/postponed/resume/baz',
]
const normalizer = new PostponedPathnameNormalizer(false)
for (const pathname of pathnames) {
Expand All @@ -16,9 +16,9 @@ describe('PostponedPathnameNormalizer', () => {

it('should match if it is enabled', () => {
const pathnames = [
'/_next/postponed/foo',
'/_next/postponed/bar',
'/_next/postponed/baz',
'/_next/postponed/resume/foo',
'/_next/postponed/resume/bar',
'/_next/postponed/resume/baz',
]
const normalizer = new PostponedPathnameNormalizer(true)
for (const pathname of pathnames) {
Expand All @@ -38,9 +38,9 @@ describe('PostponedPathnameNormalizer', () => {
describe('normalize', () => {
it('should not normalize if it is disabled', () => {
const pathnames = [
'/_next/postponed/foo',
'/_next/postponed/bar',
'/_next/postponed/baz',
'/_next/postponed/resume/foo',
'/_next/postponed/resume/bar',
'/_next/postponed/resume/baz',
]
const normalizer = new PostponedPathnameNormalizer(false)
for (const pathname of pathnames) {
Expand All @@ -60,15 +60,17 @@ describe('PostponedPathnameNormalizer', () => {
const pathnames = ['/foo', '/bar', '/baz']
const normalizer = new PostponedPathnameNormalizer(true)
for (const pathname of pathnames) {
expect(normalizer.normalize(`/_next/postponed${pathname}`, true)).toBe(
pathname
)
expect(
normalizer.normalize(`/_next/postponed/resume${pathname}`, true)
).toBe(pathname)
}
})

it('should normalize `/index` to `/`', () => {
const normalizer = new PostponedPathnameNormalizer(true)
expect(normalizer.normalize('/_next/postponed/index', true)).toBe('/')
expect(normalizer.normalize('/_next/postponed/resume/index', true)).toBe(
'/'
)
})
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { PathnameNormalizer } from './pathname-normalizer'

const prefix = '/_next/postponed/resume'

export class PostponedPathnameNormalizer implements PathnameNormalizer {
constructor(private readonly ppr: boolean | undefined) {}

Expand All @@ -8,7 +10,7 @@ export class PostponedPathnameNormalizer implements PathnameNormalizer {
if (!this.ppr) return false

// If the pathname doesn't start with the prefix, we don't match.
if (!pathname.startsWith('/_next/postponed')) return false
if (!pathname.startsWith(prefix)) return false

return true
}
Expand All @@ -21,7 +23,7 @@ export class PostponedPathnameNormalizer implements PathnameNormalizer {
if (!matched && !this.match(pathname)) return pathname

// Remove the prefix.
pathname = pathname.substring('/_next/postponed'.length) || '/'
pathname = pathname.substring(prefix.length) || '/'

// If the pathname is equal to `/index`, we normalize it to `/`.
if (pathname === '/index') return '/'
Expand Down
5 changes: 4 additions & 1 deletion packages/next/src/server/request-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ export interface RequestMeta {
* If provided, this will be called when a response cache entry was generated
* or looked up in the cache.
*/
onCacheEntry?: (cacheEntry: any) => Promise<boolean | void> | boolean | void
onCacheEntry?: (
cacheEntry: any,
requestMeta: any
) => Promise<boolean | void> | boolean | void
}

/**
Expand Down
17 changes: 0 additions & 17 deletions packages/next/src/shared/lib/router/utils/app-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,3 @@ export function normalizeRscURL(url: string) {
'$1'
)
}

/**
* Strips the `/_next/postponed` prefix if it's in the pathname.
*
* @param url the url to normalize
*/
export function normalizePostponedURL(url: string) {
const parsed = new URL(url)
const { pathname } = parsed
if (pathname && pathname.startsWith('/_next/postponed')) {
parsed.pathname = pathname.substring('/_next/postponed'.length) || '/'

return parsed.toString()
}

return url
}

0 comments on commit ec49f8b

Please sign in to comment.