Skip to content

Fix handling of hot reloader middlewares #71104

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

Merged
merged 1 commit into from
Oct 10, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ export async function createOriginalStackFrame(
}

export function getOverlayMiddleware(project: Project) {
return async function (req: IncomingMessage, res: ServerResponse) {
return async function (
req: IncomingMessage,
res: ServerResponse,
next: () => void
): Promise<void> {
const { pathname, searchParams } = new URL(req.url!, 'http://n')

const frame = {
Expand All @@ -97,7 +101,8 @@ export function getOverlayMiddleware(project: Project) {

if (!originalStackFrame) {
res.statusCode = 404
return res.end('Unable to resolve sourcemap')
res.end('Unable to resolve sourcemap')
return
}

return json(res, originalStackFrame)
Expand All @@ -119,5 +124,7 @@ export function getOverlayMiddleware(project: Project) {

noContent(res)
}

return next()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ export function getOverlayMiddleware(options: {
return async function (
req: IncomingMessage,
res: ServerResponse,
next: Function
) {
next: () => void
): Promise<void> {
const { pathname, searchParams } = new URL(`http://n${req.url}`)

const frame = {
Expand Down
16 changes: 14 additions & 2 deletions packages/next/src/server/dev/hot-reloader-turbopack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,9 @@ export async function createHotReloaderTurbopack(
2
)
)
const overlayMiddleware = getOverlayMiddleware(project)

const middlewares = [getOverlayMiddleware(project)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in anticipation of adding a second middleware in #71042.


const versionInfoPromise = getVersionInfo(
isTestMode || opts.telemetry.isEnabled
)
Expand Down Expand Up @@ -610,7 +612,17 @@ export async function createHotReloaderTurbopack(
}
}

await overlayMiddleware(req, res)
for (const middleware of middlewares) {
let calledNext = false

await middleware(req, res, () => {
calledNext = true
})

if (!calledNext) {
return { finished: true }
}
}

// Request was not finished.
return { finished: undefined }
Expand Down
25 changes: 16 additions & 9 deletions packages/next/src/server/dev/hot-reloader-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
private dir: string
private buildId: string
private encryptionKey: string
private interceptors: any[]
private middlewares: ((
req: IncomingMessage,
res: ServerResponse,
next: () => void
) => Promise<void>)[]
private pagesDir?: string
private distDir: string
private webpackHotMiddleware?: WebpackHotMiddleware
Expand Down Expand Up @@ -297,7 +301,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
this.buildId = buildId
this.encryptionKey = encryptionKey
this.dir = dir
this.interceptors = []
this.middlewares = []
this.pagesDir = pagesDir
this.appDir = appDir
this.distDir = distDir
Expand Down Expand Up @@ -379,13 +383,16 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {

const { finished } = await handlePageBundleRequest(res, parsedUrl)

for (const fn of this.interceptors) {
await new Promise<void>((resolve, reject) => {
fn(req, res, (err: Error) => {
if (err) return reject(err)
resolve()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we were only resolving the promise when next() was called, which the middleware only does if it doesn't handle a request.

})
for (const middleware of this.middlewares) {
let calledNext = false

await middleware(req, res, () => {
calledNext = true
})

if (!calledNext) {
return { finished: true }
}
}

return { finished }
Expand Down Expand Up @@ -1491,7 +1498,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
}),
})

this.interceptors = [
this.middlewares = [
getOverlayMiddleware({
rootDirectory: this.dir,
stats: () => this.clientStats,
Expand Down
Loading