From 14ed867537cf200b34b3e6d6b7c810c3fb3983db Mon Sep 17 00:00:00 2001 From: Christoph Tavan Date: Tue, 29 Oct 2019 01:01:25 +0100 Subject: [PATCH] Disable worker_threads by default for firebase compatibility (#9199) * 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. https://github.com/zeit/next.js/issues/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 --- packages/next/build/index.ts | 3 +- packages/next/build/webpack-config.ts | 5 +-- .../terser-webpack-plugin/src/TaskRunner.js | 5 +-- .../terser-webpack-plugin/src/index.js | 3 ++ packages/next/export/index.ts | 2 +- packages/next/next-server/server/config.ts | 1 + .../integration/firebase-grpc/pages/page-1.js | 2 ++ .../integration/firebase-grpc/pages/page-2.js | 2 ++ .../firebase-grpc/test/index.test.js | 33 +++++++++++++++++++ 9 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 test/integration/firebase-grpc/pages/page-1.js create mode 100644 test/integration/firebase-grpc/pages/page-2.js create mode 100644 test/integration/firebase-grpc/test/index.test.js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 693fc4803d2da..1d6eefdc45f3f 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -337,7 +337,7 @@ export default async function build(dir: string, conf = null): Promise { const staticCheckWorkers = new Worker(staticCheckWorker, { numWorkers: config.experimental.cpus, - enableWorkerThreads: true, + enableWorkerThreads: config.experimental.workerThreads, }) const analysisBegin = process.hrtime() @@ -471,6 +471,7 @@ export default async function build(dir: string, conf = null): Promise { sprPages, silent: true, buildExport: true, + threads: config.experimental.cpus, pages: combinedPages, outdir: path.join(distDir, 'export'), } diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index ec50bea7f612c..74b044cdaed42 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -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: { diff --git a/packages/next/build/webpack/plugins/terser-webpack-plugin/src/TaskRunner.js b/packages/next/build/webpack/plugins/terser-webpack-plugin/src/TaskRunner.js index b306966292ede..d8c0b48df39fd 100644 --- a/packages/next/build/webpack/plugins/terser-webpack-plugin/src/TaskRunner.js +++ b/packages/next/build/webpack/plugins/terser-webpack-plugin/src/TaskRunner.js @@ -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) { @@ -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) diff --git a/packages/next/build/webpack/plugins/terser-webpack-plugin/src/index.js b/packages/next/build/webpack/plugins/terser-webpack-plugin/src/index.js index 1acafe76f9891..4302cb344e4d1 100644 --- a/packages/next/build/webpack/plugins/terser-webpack-plugin/src/index.js +++ b/packages/next/build/webpack/plugins/terser-webpack-plugin/src/index.js @@ -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, @@ -134,6 +136,7 @@ export class TerserPlugin { distDir: this.distDir, cpus: this.cpus, cache: this.options.cache, + workerThreads: this.workerThreads, }) const processedAssets = new WeakSet() diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index dba1116a3f3d1..2a0ae8b13958c 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -267,7 +267,7 @@ export default async function( { maxRetries: 0, numWorkers: threads, - enableWorkerThreads: true, + enableWorkerThreads: nextConfig.experimental.workerThreads, exposedMethods: ['default'], } ) as any diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts index 3fe9b13f1a7b3..97a8745c9ab9f 100644 --- a/packages/next/next-server/server/config.ts +++ b/packages/next/next-server/server/config.ts @@ -48,6 +48,7 @@ const defaultConfig: { [key: string]: any } = { publicDirectory: false, sprFlushToDisk: true, deferScripts: false, + workerThreads: false, }, future: { excludeDefaultMomentLocales: false, diff --git a/test/integration/firebase-grpc/pages/page-1.js b/test/integration/firebase-grpc/pages/page-1.js new file mode 100644 index 0000000000000..dd9c28611eea2 --- /dev/null +++ b/test/integration/firebase-grpc/pages/page-1.js @@ -0,0 +1,2 @@ +import 'firebase/firestore' +export default () =>
Firebase
diff --git a/test/integration/firebase-grpc/pages/page-2.js b/test/integration/firebase-grpc/pages/page-2.js new file mode 100644 index 0000000000000..dd9c28611eea2 --- /dev/null +++ b/test/integration/firebase-grpc/pages/page-2.js @@ -0,0 +1,2 @@ +import 'firebase/firestore' +export default () =>
Firebase
diff --git a/test/integration/firebase-grpc/test/index.test.js b/test/integration/firebase-grpc/test/index.test.js new file mode 100644 index 0000000000000..dc24e5fba36de --- /dev/null +++ b/test/integration/firebase-grpc/test/index.test.js @@ -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\./ + ) + }) +})