Skip to content

Commit

Permalink
Disable worker_threads by default for firebase compatibility (#9199)
Browse files Browse the repository at this point in the history
* Pass config.experimental.cpus to export during build

Currently, there is no way of specifying the number of worker threads of
`next export` when run as part of `next build`.

I suggest a sane default should be to just use the same amount of
workers that were used during the build process which currently seems to
be configured through `config.experimental.cpus`.

This setting is already respected in the two other places where
jest-workers are in use: The TerserPlugin and the staticCheckWorkers in
`next build`.

* Only enable worker threads if there is more than 1 worker

Multiple worker threads can cause problems when certain dependencies are
being used, see e.g. #7894

This patch allows disabling of worker threads by setting
`config.experimental.cpus = 1`.

The benefit of spawning 1 worker thread, if there is any at all, should
very limited anyways, so the workload can just as well be processed in
the main thread.

* Disable parallel build for firebase authentication example

* Add integration test to cover #7894

* Rename test suite and add worker_threads config

* Disable worker_threads by default

* Update index.test.js

* Use workerThreads config for TerserPlugin

* Update to use workerThreads config in
TerserPlugin for consistency

* Disable node 12 specific test
  • Loading branch information
ctavan authored and Timer committed Oct 29, 2019
1 parent ad2a3d6 commit 14ed867
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 6 deletions.
3 changes: 2 additions & 1 deletion packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export default async function build(dir: string, conf = null): Promise<void> {

const staticCheckWorkers = new Worker(staticCheckWorker, {
numWorkers: config.experimental.cpus,
enableWorkerThreads: true,
enableWorkerThreads: config.experimental.workerThreads,
})

const analysisBegin = process.hrtime()
Expand Down Expand Up @@ -471,6 +471,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
sprPages,
silent: true,
buildExport: true,
threads: config.experimental.cpus,
pages: combinedPages,
outdir: path.join(distDir, 'export'),
}
Expand Down
5 changes: 3 additions & 2 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ export default async function getBaseWebpackConfig(
const webpackMode = dev ? 'development' : 'production'

const terserPluginConfig = {
parallel: true,
sourceMap: false,
cache: true,
cpus: config.experimental.cpus,
distDir: distDir,
parallel: true,
sourceMap: false,
workerThreads: config.experimental.workerThreads,
}
const terserOptions = {
parse: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ const writeFileP = promisify(writeFile)
const readFileP = promisify(readFile)

export default class TaskRunner {
constructor({ distDir, cpus, cache }) {
constructor({ distDir, cpus, cache, workerThreads }) {
if (cache) {
mkdirp.sync((this.cacheDir = join(distDir, 'cache', 'next-minifier')))
}
// In some cases cpus() returns undefined
// https://github.com/nodejs/node/issues/19022
this.maxConcurrentWorkers = cpus
this.useWorkerThreads = workerThreads
}

run(tasks, callback) {
Expand All @@ -28,7 +29,7 @@ export default class TaskRunner {

if (this.maxConcurrentWorkers > 1) {
this.workers = new Worker(worker, {
enableWorkerThreads: true,
enableWorkerThreads: this.useWorkerThreads,
numWorkers: this.maxConcurrentWorkers,
})
this.boundWorkers = options => this.workers.default(options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ export class TerserPlugin {
cache = false,
cpus,
distDir,
workerThreads,
} = options

this.cpus = cpus
this.distDir = distDir
this.workerThreads = workerThreads
this.options = {
warningsFilter,
sourceMap,
Expand Down Expand Up @@ -134,6 +136,7 @@ export class TerserPlugin {
distDir: this.distDir,
cpus: this.cpus,
cache: this.options.cache,
workerThreads: this.workerThreads,
})

const processedAssets = new WeakSet()
Expand Down
2 changes: 1 addition & 1 deletion packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export default async function(
{
maxRetries: 0,
numWorkers: threads,
enableWorkerThreads: true,
enableWorkerThreads: nextConfig.experimental.workerThreads,
exposedMethods: ['default'],
}
) as any
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const defaultConfig: { [key: string]: any } = {
publicDirectory: false,
sprFlushToDisk: true,
deferScripts: false,
workerThreads: false,
},
future: {
excludeDefaultMomentLocales: false,
Expand Down
2 changes: 2 additions & 0 deletions test/integration/firebase-grpc/pages/page-1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import 'firebase/firestore'
export default () => <div>Firebase</div>
2 changes: 2 additions & 0 deletions test/integration/firebase-grpc/pages/page-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import 'firebase/firestore'
export default () => <div>Firebase</div>
33 changes: 33 additions & 0 deletions test/integration/firebase-grpc/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* eslint-env jest */
/* global jasmine */
import path from 'path'
import fs from 'fs-extra'
import { nextBuild } from 'next-test-utils'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1
const appDir = path.join(__dirname, '..')
const nextConfig = path.join(appDir, 'next.config.js')

describe('Building Firebase', () => {
// TODO: investigate re-enabling this test in node 12 environment
xit('Throws an error when building with firebase dependency with worker_threads', async () => {
await fs.writeFile(
nextConfig,
`module.exports = { experimental: { workerThreads: true } }`
)
const results = await nextBuild(appDir, [], { stdout: true, stderr: true })
expect(results.stdout + results.stderr).toMatch(/Build error occurred/)
expect(results.stdout + results.stderr).toMatch(
/grpc_node\.node\. Module did not self-register\./
)
})

it('Throws no error when building with firebase dependency without worker_threads', async () => {
await fs.remove(nextConfig)
const results = await nextBuild(appDir, [], { stdout: true, stderr: true })
expect(results.stdout + results.stderr).not.toMatch(/Build error occurred/)
expect(results.stdout + results.stderr).not.toMatch(
/grpc_node\.node\. Module did not self-register\./
)
})
})

0 comments on commit 14ed867

Please sign in to comment.