Skip to content

Commit

Permalink
Hot reload when conflicted pages detected and when conflicts are reso…
Browse files Browse the repository at this point in the history 
…lved (#51516)

When conflicted pages occur between app and pages server used to throw error and you need to solve the conflicts and restart the server.

This PR capture the error during files aggregation, and send to client to display the error for files confliction. This will send all the conflicts to client and it will still error until you solve all of them. But since the server is not stuck by errors, client can recover once the error is solved.

Closes NEXT-1283
  • Loading branch information
@huozhi
huozhi authored Jun 22, 2023
1 parent db4b985 commit 89f36a9
Show file tree
Hide file tree
Showing 27 changed files with 300 additions and 211 deletions.
81 changes: 48 additions & 33 deletions packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,39 @@ function processMessage(
router: ReturnType<typeof useRouter>,
dispatcher: Dispatcher
) {
const obj = JSON.parse(e.data)
let obj
try {
obj = JSON.parse(e.data)
} catch {}

if (!obj || !('action' in obj)) {
return
}

function handleErrors(errors: any[]) {
// "Massage" webpack messages.
const formatted = formatWebpackMessages({
errors: errors,
warnings: [],
})

// Only show the first error.
dispatcher.onBuildError(formatted.errors[0])

// Also log them to the console.
for (let i = 0; i < formatted.errors.length; i++) {
console.error(stripAnsi(formatted.errors[i]))
}

// Do not attempt to reload now.
// We will reload on next success instead.
if (process.env.__NEXT_TEST_MODE) {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB(formatted.errors[0])
self.__NEXT_HMR_CB = null
}
}
}

switch (obj.action) {
case 'building': {
Expand Down Expand Up @@ -239,28 +271,7 @@ function processMessage(
})
)

// "Massage" webpack messages.
let formatted = formatWebpackMessages({
errors: errors,
warnings: [],
})

// Only show the first error.
dispatcher.onBuildError(formatted.errors[0])

// Also log them to the console.
for (let i = 0; i < formatted.errors.length; i++) {
console.error(stripAnsi(formatted.errors[i]))
}

// Do not attempt to reload now.
// We will reload on next success instead.
if (process.env.__NEXT_TEST_MODE) {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB(formatted.errors[0])
self.__NEXT_HMR_CB = null
}
}
handleErrors(errors)
return
}

Expand Down Expand Up @@ -388,6 +399,16 @@ function processMessage(
router.fastRefresh()
return
}
case 'serverError': {
const { errorJSON } = obj
if (errorJSON) {
const { message, stack } = JSON.parse(errorJSON)
const error = new Error(message)
error.stack = stack
handleErrors([error])
}
return
}
case 'pong': {
const { invalid } = obj
if (invalid) {
Expand Down Expand Up @@ -468,18 +489,12 @@ export default function HotReload({
const router = useRouter()
useEffect(() => {
const handler = (event: MessageEvent<PongEvent>) => {
if (
!event.data.includes('action') &&
// TODO-APP: clean this up for consistency
!event.data.includes('pong')
) {
return
}

try {
processMessage(event, sendMessage, router, dispatcher)
} catch (ex) {
console.warn('Invalid HMR message: ' + event.data + '\n', ex)
} catch (err: any) {
console.warn(
'[HMR] Invalid message: ' + event.data + '\n' + (err?.stack ?? '')
)
}
}

Expand Down
8 changes: 6 additions & 2 deletions packages/next/src/client/dev/amp-dev.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ addMessageListener((event) => {
try {
const message = JSON.parse(event.data)

// action `serverError` is not for amp-dev
if (message.action === 'serverError') return
if (message.action === 'sync' || message.action === 'built') {
if (!message.hash) {
return
Expand All @@ -94,8 +96,10 @@ addMessageListener((event) => {
} else if (message.action === 'reloadPage') {
window.location.reload()
}
} catch (ex) {
console.warn('Invalid HMR message: ' + event.data + '\n' + ex)
} catch (err: any) {
console.warn(
'[HMR] Invalid message: ' + event.data + '\n' + (err?.stack ?? '')
)
}
})

Expand Down
26 changes: 19 additions & 7 deletions packages/next/src/client/dev/error-overlay/hot-dev-client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,15 @@ export default function connect() {
register()

addMessageListener((event) => {
if (!event.data.includes('action')) return

try {
processMessage(event)
} catch (ex) {
console.warn('Invalid HMR message: ' + event.data + '\n', ex)
const payload = JSON.parse(event.data)
if (!('action' in payload)) return

processMessage(payload)
} catch (err: any) {
console.warn(
'[HMR] Invalid message: ' + event.data + '\n' + (err?.stack ?? '')
)
}
})

Expand Down Expand Up @@ -226,8 +229,7 @@ function handleAvailableHash(hash: string) {
}

// Handle messages from the server.
function processMessage(e: any) {
const obj = JSON.parse(e.data)
function processMessage(obj: any) {
switch (obj.action) {
case 'building': {
startLatency = Date.now()
Expand Down Expand Up @@ -279,6 +281,16 @@ function processMessage(e: any) {
window.location.reload()
return
}
case 'serverError': {
const { errorJSON } = obj
if (errorJSON) {
const { message, stack } = JSON.parse(errorJSON)
const error = new Error(message)
error.stack = stack
handleErrors([error])
}
return
}
default: {
if (customHmrEventHandler) {
customHmrEventHandler(obj)
Expand Down
3 changes: 1 addition & 2 deletions packages/next/src/client/dev/error-overlay/websocket.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export function connectHMR(options: {
path: string
assetPrefix: string
timeout?: number
log?: boolean
}) {
if (!options.timeout) {
options.timeout = 5 * 1000
Expand All @@ -36,7 +35,7 @@ export function connectHMR(options: {
if (source) source.close()

function handleOnline() {
if (options.log) console.log('[HMR] connected')
window.console.log('[HMR] connected')
lastActivity = Date.now()
}

Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/client/dev/webpack-hot-middleware-client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export default () => {
}
return
}
if (obj.action === 'serverError') {
return
}
throw new Error('Unexpected action ' + obj.action)
})

Expand Down
23 changes: 17 additions & 6 deletions packages/next/src/client/next-dev.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,19 @@ initialize({ webpackHMR })

let buildIndicatorHandler: any = () => {}

function devPagesManifestListener(event: any) {
if (event.data.includes('devPagesManifest')) {
function devPagesHmrListener(event: any) {
let payload
try {
payload = JSON.parse(event.data)
} catch {}
if (payload.event === 'server-error' && payload.errorJSON) {
const { stack, message } = JSON.parse(payload.errorJSON)
const error = new Error(message)
error.stack = stack
throw error
} else if (payload.action === 'reloadPage') {
window.location.reload()
} else if (payload.action === 'devPagesManifestUpdate') {
fetch(
`${assetPrefix}/_next/static/development/_devPagesManifest.json`
)
Expand All @@ -66,10 +77,10 @@ initialize({ webpackHMR })
.catch((err) => {
console.log(`Failed to fetch devPagesManifest`, err)
})
} else if (event.data.includes('middlewareChanges')) {
} else if (payload.event === 'middlewareChanges') {
return window.location.reload()
} else if (event.data.includes('serverOnlyChanges')) {
const { pages } = JSON.parse(event.data)
} else if (payload.event === 'serverOnlyChanges') {
const { pages } = payload

// Make sure to reload when the dev-overlay is showing for an
// API route
Expand Down Expand Up @@ -106,7 +117,7 @@ initialize({ webpackHMR })
}
}
}
addMessageListener(devPagesManifestListener)
addMessageListener(devPagesHmrListener)

if (process.env.__NEXT_BUILD_INDICATOR) {
initializeBuildWatcher((handler: any) => {
Expand Down
14 changes: 13 additions & 1 deletion packages/next/src/server/dev/hot-reloader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ export default class HotReloader {
public edgeServerStats: webpack.Stats | null
private clientError: Error | null = null
private serverError: Error | null = null
private hmrServerError: Error | null = null
private serverPrevDocumentHash: string | null
private serverChunkNames?: Set<string>
private prevChunkNames?: Set<any>
Expand Down Expand Up @@ -327,10 +328,21 @@ export default class HotReloader {
return { finished }
}

public setHmrServerError(error: Error | null): void {
this.hmrServerError = error
}

public clearHmrServerError(): void {
if (this.hmrServerError) {
this.setHmrServerError(null)
this.send('reloadPage')
}
}

public onHMR(req: IncomingMessage, _socket: Duplex, head: Buffer) {
wsServer.handleUpgrade(req, req.socket, head, (client) => {
this.webpackHotMiddleware?.onHMR(client)
this.onDemandEntries?.onHMR(client)
this.onDemandEntries?.onHMR(client, () => this.hmrServerError)

client.addEventListener('message', ({ data }) => {
data = typeof data !== 'string' ? data.toString() : data
Expand Down
40 changes: 26 additions & 14 deletions packages/next/src/server/dev/next-dev-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ export default class DevServer extends Server {
const fileWatchTimes = new Map()
let enabledTypeScript = this.usingTypeScript
let previousClientRouterFilters: any
let previousConflictingPagePaths: Set<string> = new Set()

wp.on('aggregated', async () => {
let middlewareMatchers: MiddlewareMatcher[] | undefined
Expand All @@ -440,6 +441,7 @@ export default class DevServer extends Server {

let envChange = false
let tsconfigChange = false
let conflictingPageChange = 0

devPageFiles.clear()

Expand Down Expand Up @@ -598,20 +600,30 @@ export default class DevServer extends Server {
}

const numConflicting = conflictingAppPagePaths.size
if (numConflicting > 0) {
Log.error(
`Conflicting app and page file${
conflictingPageChange =
numConflicting - previousConflictingPagePaths.size

if (conflictingPageChange !== 0) {
if (numConflicting > 0) {
let errorMessage = `Conflicting app and page file${
numConflicting === 1 ? ' was' : 's were'
} found, please remove the conflicting files to continue:`
)
for (const p of conflictingAppPagePaths) {
const appPath = relative(this.dir, appPageFilePaths.get(p)!)
const pagesPath = relative(this.dir, pagesPageFilePaths.get(p)!)
Log.error(` "${pagesPath}" - "${appPath}"`)
} found, please remove the conflicting files to continue:\n`

for (const p of conflictingAppPagePaths) {
const appPath = relative(this.dir, appPageFilePaths.get(p)!)
const pagesPath = relative(this.dir, pagesPageFilePaths.get(p)!)
errorMessage += ` "${pagesPath}" - "${appPath}"\n`
}
this.hotReloader?.setHmrServerError(new Error(errorMessage))
} else if (numConflicting === 0) {
await this.matchers.reload()
this.hotReloader?.clearHmrServerError()
}
}
let clientRouterFilters: any

previousConflictingPagePaths = conflictingAppPagePaths

let clientRouterFilters: any
if (this.nextConfig.experimental.clientRouterFilter) {
clientRouterFilters = createClientRouterFilter(
Object.keys(appPaths),
Expand Down Expand Up @@ -802,7 +814,9 @@ export default class DevServer extends Server {
!this.sortedRoutes?.every((val, idx) => val === sortedRoutes[idx])
) {
// emit the change so clients fetch the update
this.hotReloader?.send(undefined, { devPagesManifest: true })
this.hotReloader?.send('devPagesManifestUpdate', {
devPagesManifest: true,
})
}
this.sortedRoutes = sortedRoutes

Expand Down Expand Up @@ -997,9 +1011,7 @@ export default class DevServer extends Server {
)
}
if (appFile && pagesFile) {
throw new Error(
`Conflicting app and page file found: "app${appFile}" and "pages${pagesFile}". Please remove one to continue.`
)
return false
}

return Boolean(appFile || pagesFile)
Expand Down
Loading

0 comments on commit 89f36a9

Please sign in to comment.