Skip to content

Commit

Permalink
Wait for getFilesForRoute promise to fulfill before timeout in dev mode
Browse files Browse the repository at this point in the history
The fix in PR vercel#25749 only works some of the time. The reason why it
doesn't work all of the time is because the `devBuildResolve` function
is called when the assets have been built, but this can happen before
the `getFilesForRoute` promise chain has completed, and thus the
`cancelled` variable could not yet have been updated.

To fix this we wait for the `getFilesForRoute` promise chain to be
fulfilled and then resolve the `devBuildPromise` promise afterwards.
This allows the `getFilesForRoute` promise chain to be fulfilled before
and the `cancelled` variable can be updated accordingly.

If an error is thrown somewhere in the `getFilesForRoute` promise chain,
i.e. due to a failed fetch request, then that will also resolve the
`devBuildPromise` and the error will bubble up to ultimately become a
`routeChangeError` which will reload the page.

With this fix the need to listen for HMR events has become obsolete as
regardless of when the HMR build/sync events complete we still want to
ensure that the `getFilesForRoute` promise chain has been fulfilled
before resolving the `devBuildPromise`.
  • Loading branch information
tobiasjarvelov committed Jul 22, 2021
1 parent c218347 commit 7162aff
Showing 1 changed file with 22 additions and 45 deletions.
67 changes: 22 additions & 45 deletions packages/next/client/route-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,35 +152,6 @@ function appendScript(
// We wait for pages to be built in dev before we start the route transition
// timeout to prevent an un-necessary hard navigation in development.
let devBuildPromise: Promise<void> | undefined
let devBuildResolve: (() => void) | undefined

if (process.env.NODE_ENV === 'development') {
const { addMessageListener } = require('./dev/error-overlay/eventsource')

addMessageListener((event: any) => {
// This is the heartbeat event
if (event.data === '\uD83D\uDC93') {
return
}

const obj =
typeof event === 'string' ? { action: event } : JSON.parse(event.data)

switch (obj.action) {
case 'built':
case 'sync':
if (devBuildResolve) {
devBuildResolve()
devBuildResolve = undefined
}

break

default:
break
}
})
}

// Resolve a promise that times out after given amount of milliseconds.
function resolvePromiseWithTimeout<T>(
Expand Down Expand Up @@ -356,28 +327,34 @@ export function createRouteLoader(assetPrefix: string): RouteLoader {
},
loadRoute(route: string, prefetch?: boolean) {
return withFuture<RouteLoaderEntry>(route, routes, () => {
const routeFilesPromise = getFilesForRoute(assetPrefix, route)
.then(({ scripts, css }) => {
return Promise.all([
entrypoints.has(route)
? []
: Promise.all(scripts.map(maybeExecuteScript)),
Promise.all(css.map(fetchStyleSheet)),
] as const)
})
.then((res) => {
return this.whenEntrypoint(route).then((entrypoint) => ({
entrypoint,
styles: res[1],
}))
})

if (process.env.NODE_ENV === 'development') {
devBuildPromise = new Promise<void>((resolve) => {
devBuildResolve = resolve
if (routeFilesPromise) {
return routeFilesPromise.finally(() => {
resolve()
})
}
})
}

return resolvePromiseWithTimeout(
getFilesForRoute(assetPrefix, route)
.then(({ scripts, css }) => {
return Promise.all([
entrypoints.has(route)
? []
: Promise.all(scripts.map(maybeExecuteScript)),
Promise.all(css.map(fetchStyleSheet)),
] as const)
})
.then((res) => {
return this.whenEntrypoint(route).then((entrypoint) => ({
entrypoint,
styles: res[1],
}))
}),
routeFilesPromise,
MS_MAX_IDLE_DELAY,
markAssetError(new Error(`Route did not complete loading: ${route}`))
)
Expand Down

0 comments on commit 7162aff

Please sign in to comment.