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

Improvements to on-demand-entries #5176

Merged
merged 13 commits into from
Sep 16, 2018
3 changes: 1 addition & 2 deletions build/webpack/plugins/pages-manifest-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {PAGES_MANIFEST, ROUTE_NAME_REGEX} from '../../../lib/constants'
// It's also used by next export to provide defaultPathMap
export default class PagesManifestPlugin {
apply (compiler: any) {
compiler.hooks.emit.tapAsync('NextJsPagesManifest', (compilation, callback) => {
compiler.hooks.emit.tap('NextJsPagesManifest', (compilation) => {
const {entries} = compilation
const pages = {}

Expand All @@ -32,7 +32,6 @@ export default class PagesManifestPlugin {
}

compilation.assets[PAGES_MANIFEST] = new RawSource(JSON.stringify(pages))
callback()
})
}
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
"terser-webpack-plugin": "1.0.2",
"unfetch": "3.0.0",
"url": "0.11.0",
"webpack": "4.17.1",
"webpack": "4.19.0",
"webpack-dev-middleware": "3.2.0",
"webpack-hot-middleware": "2.22.3",
"webpack-sources": "1.2.0",
Expand Down
2 changes: 1 addition & 1 deletion server/hot-reloader.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ export default class HotReloader {
heartbeat: 2500
})

const onDemandEntries = onDemandEntryHandler(webpackDevMiddleware, multiCompiler.compilers, {
const onDemandEntries = onDemandEntryHandler(webpackDevMiddleware, multiCompiler, {
dir: this.dir,
buildId: this.buildId,
dev: true,
Expand Down
161 changes: 90 additions & 71 deletions server/on-demand-entry-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,18 @@ const BUILT = Symbol('built')
const glob = promisify(globModule)
const access = promisify(fs.access)

export default function onDemandEntryHandler (devMiddleware, compilers, {
// Based on https://github.com/webpack/webpack/blob/master/lib/DynamicEntryPlugin.js#L29-L37
function addEntry (compilation, context, name, entry) {
return new Promise((resolve, reject) => {
const dep = DynamicEntryPlugin.createDependency(entry, name)
compilation.addEntry(context, dep, name, (err) => {
if (err) return reject(err)
resolve()
})
})
}

export default function onDemandEntryHandler (devMiddleware, multiCompiler, {
buildId,
dir,
dev,
Expand All @@ -25,20 +36,18 @@ export default function onDemandEntryHandler (devMiddleware, compilers, {
maxInactiveAge = 1000 * 60,
pagesBufferLength = 2
}) {
const {compilers} = multiCompiler
const invalidator = new Invalidator(devMiddleware, multiCompiler)
let entries = {}
let lastAccessPages = ['']
let doneCallbacks = new EventEmitter()
const invalidator = new Invalidator(devMiddleware)
let reloading = false
let stopped = false
let reloadCallbacks = new EventEmitter()
// Keep the names of compilers which are building pages at a given moment.
const currentBuilders = new Set()

compilers.forEach(compiler => {
compiler.hooks.make.tapAsync('NextJsOnDemandEntries', function (compilation, done) {
for (const compiler of compilers) {
compiler.hooks.make.tapPromise('NextJsOnDemandEntries', (compilation) => {
invalidator.startBuilding()
currentBuilders.add(compiler.name)

const allEntries = Object.keys(entries).map(async (page) => {
const { name, entry } = entries[page]
Expand All @@ -57,65 +66,79 @@ export default function onDemandEntryHandler (devMiddleware, compilers, {
return addEntry(compilation, compiler.context, name, entry)
})

Promise.all(allEntries)
.then(() => done())
.catch(done)
return Promise.all(allEntries)
})
}

compiler.hooks.done.tap('NextJsOnDemandEntries', function (stats) {
// Wait until all the compilers mark the build as done.
currentBuilders.delete(compiler.name)
if (currentBuilders.size !== 0) return
multiCompiler.hooks.done.tap('NextJsOnDemandEntries', (multiStats) => {
const clientStats = multiStats.stats[0]
const { compilation } = clientStats
const hardFailedPages = compilation.errors
.filter(e => {
// Make sure to only pick errors which marked with missing modules
const hasNoModuleFoundError = /ENOENT/.test(e.message) || /Module not found/.test(e.message)
if (!hasNoModuleFoundError) return false

// The page itself is missing. So this is a failed page.
if (IS_BUNDLED_PAGE_REGEX.test(e.module.name)) return true

// No dependencies means this is a top level page.
// So this is a failed page.
return e.module.dependencies.length === 0
})
.map(e => e.module.chunks)
.reduce((a, b) => [...a, ...b], [])
.map(c => {
const pageName = ROUTE_NAME_REGEX.exec(c.name)[1]
return normalizePage(`/${pageName}`)
})

const { compilation } = stats
const hardFailedPages = compilation.errors
.filter(e => {
// Make sure to only pick errors which marked with missing modules
const hasNoModuleFoundError = /ENOENT/.test(e.message) || /Module not found/.test(e.message)
if (!hasNoModuleFoundError) return false
// compilation.entrypoints is a Map object, so iterating over it 0 is the key and 1 is the value
for (const [, entrypoint] of compilation.entrypoints.entries()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The main change in this PR is that we're no longer calling doneCallbacks on entries, because this leads to timing issues. Instead we use webpack's compilation.entrypoints as the source of truth of what's compiled (which is logical, as it holds exactly what's compiled in this compilation run).

const result = ROUTE_NAME_REGEX.exec(entrypoint.name)
if (!result) {
continue
}

// The page itself is missing. So this is a failed page.
if (IS_BUNDLED_PAGE_REGEX.test(e.module.name)) return true
const pagePath = result[1]

// No dependencies means this is a top level page.
// So this is a failed page.
return e.module.dependencies.length === 0
})
.map(e => e.module.chunks)
.reduce((a, b) => [...a, ...b], [])
.map(c => {
const pageName = ROUTE_NAME_REGEX.exec(c.name)[1]
return normalizePage(`/${pageName}`)
})
if (!pagePath) {
continue
}

// Call all the doneCallbacks
Object.keys(entries).forEach((page) => {
const entryInfo = entries[page]
if (entryInfo.status !== BUILDING) return
const page = normalizePage('/' + pagePath)

entryInfo.status = BUILT
entries[page].lastActiveTime = Date.now()
doneCallbacks.emit(page)
})
const entry = entries[page]
if (!entry) {
continue
}

invalidator.doneBuilding(compiler.name)

if (hardFailedPages.length > 0 && !reloading) {
console.log(`> Reloading webpack due to inconsistant state of pages(s): ${hardFailedPages.join(', ')}`)
reloading = true
reload()
.then(() => {
console.log('> Webpack reloaded.')
reloadCallbacks.emit('done')
stop()
})
.catch(err => {
console.error(`> Webpack reloading failed: ${err.message}`)
console.error(err.stack)
process.exit(1)
})
if (entry.status !== BUILDING) {
continue
}
})

entry.status = BUILT
entry.lastActiveTime = Date.now()
doneCallbacks.emit(page)
}

invalidator.doneBuilding()

if (hardFailedPages.length > 0 && !reloading) {
console.log(`> Reloading webpack due to inconsistant state of pages(s): ${hardFailedPages.join(', ')}`)
reloading = true
reload()
.then(() => {
console.log('> Webpack reloaded.')
reloadCallbacks.emit('done')
stop()
})
.catch(err => {
console.error(`> Webpack reloading failed: ${err.message}`)
console.error(err.stack)
process.exit(1)
})
}
})

const disposeHandler = setInterval(function () {
Expand Down Expand Up @@ -248,17 +271,6 @@ export default function onDemandEntryHandler (devMiddleware, compilers, {
}
}

// Based on https://github.com/webpack/webpack/blob/master/lib/DynamicEntryPlugin.js#L29-L37
function addEntry (compilation, context, name, entry) {
return new Promise((resolve, reject) => {
const dep = DynamicEntryPlugin.createDependency(entry, name)
compilation.addEntry(context, dep, name, (err) => {
if (err) return reject(err)
resolve()
})
})
}

function disposeInactiveEntries (devMiddleware, entries, lastAccessPages, maxInactiveAge) {
const disposingPages = []

Expand Down Expand Up @@ -291,10 +303,11 @@ function disposeInactiveEntries (devMiddleware, entries, lastAccessPages, maxIna
// /index and / is the same. So, we need to identify both pages as the same.
// This also applies to sub pages as well.
export function normalizePage (page) {
if (page === '/index' || page === '/') {
const unixPagePath = page.replace(/\\/g, '/')
if (unixPagePath === '/index' || unixPagePath === '/') {
return '/'
}
return page.replace(/\/index$/, '')
return unixPagePath.replace(/\/index$/, '')
}

function sendJson (res, payload) {
Expand All @@ -306,7 +319,8 @@ function sendJson (res, payload) {
// Make sure only one invalidation happens at a time
// Otherwise, webpack hash gets changed and it'll force the client to reload.
class Invalidator {
constructor (devMiddleware) {
constructor (devMiddleware, multiCompiler) {
this.multiCompiler = multiCompiler
this.devMiddleware = devMiddleware
// contains an array of types of compilers currently building
this.building = false
Expand All @@ -324,6 +338,11 @@ class Invalidator {
}

this.building = true
// Work around a bug in webpack, calling `invalidate` on Watching.js
// doesn't trigger the invalid call used to keep track of the `.done` hook on multiCompiler
for (const compiler of this.multiCompiler.compilers) {
compiler.hooks.invalid.call()
}
this.devMiddleware.invalidate()
}

Expand Down