From 5262d0b56d94e522f9f65db76a92389d4da2551e Mon Sep 17 00:00:00 2001 From: Ahmad Amireh Date: Sat, 9 Apr 2016 16:25:18 +0300 Subject: [PATCH] better --watch plugin workflow, less state the "teardown" routine is now idempotent, got rid of much of the book-keeping state needed between initial/successive builds --- .travis.yml | 2 + README.md | 11 -- examples/dependency/lib/index.js | 3 +- examples/dependency/webpack.config.js | 1 - examples/single-loader/a.js | 1 + examples/single-loader/b.js | 2 + examples/single-loader/c.js | 0 examples/single-loader/d.js | 1 + examples/single-loader/identity-loader.js | 3 + examples/single-loader/webpack.config.js | 28 +++++ lib/HappyPlugin.js | 106 ++++++++++-------- lib/HappyTestUtils.js | 5 +- lib/OptionParser.js | 2 +- lib/__tests__/HappyPlugin.integration.test.js | 66 ----------- .../integration/webpack__bail.test.js | 43 +++++++ .../integration/webpack__watch.test.js | 94 ++++++++++++++++ lib/fnOnce.js | 11 +- 17 files changed, 247 insertions(+), 132 deletions(-) create mode 100644 examples/single-loader/a.js create mode 100644 examples/single-loader/b.js create mode 100644 examples/single-loader/c.js create mode 100644 examples/single-loader/d.js create mode 100644 examples/single-loader/identity-loader.js create mode 100644 examples/single-loader/webpack.config.js create mode 100644 lib/__tests__/integration/webpack__bail.test.js create mode 100644 lib/__tests__/integration/webpack__watch.test.js diff --git a/.travis.yml b/.travis.yml index aecc77c..44fb4bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,6 @@ language: node_js +env: + - HAPPY_TEST_TIMEOUT=60000 node_js: - "stable" - "4.1" diff --git a/README.md b/README.md index 0a28f2c..17207d0 100644 --- a/README.md +++ b/README.md @@ -151,17 +151,6 @@ as HappyPack will switch into a synchronous mode afterwards (i.e. in `watch` mode.) Also, if we're using the cache and the compiled versions are indeed cached, the threads will be idle. -### `installExitHandler: Boolean` - -Whether we should intercept the process's `SIGINT` and clean up when it is -received. This is needed because webpack's CLI does not expose any hook for -cleaning up when it is going down, so it's a good idea to hook into it. - -You can turn this off if you don't want this functionality or it gives you -trouble. - -Defaults to: `true` - ## How it works ![A diagram showing the flow between HappyPack's components](doc/HappyPack_Workflow.png) diff --git a/examples/dependency/lib/index.js b/examples/dependency/lib/index.js index dc12b80..abcaabf 100644 --- a/examples/dependency/lib/index.js +++ b/examples/dependency/lib/index.js @@ -1 +1,2 @@ -// index.js \ No newline at end of file +// index.js +require('./b'); \ No newline at end of file diff --git a/examples/dependency/webpack.config.js b/examples/dependency/webpack.config.js index 1dbf3c3..5f97db3 100644 --- a/examples/dependency/webpack.config.js +++ b/examples/dependency/webpack.config.js @@ -1,5 +1,4 @@ var path = require('path'); -var through = require('browserify-through'); module.exports = { entry: path.resolve(__dirname, 'lib/index.js'), diff --git a/examples/single-loader/a.js b/examples/single-loader/a.js new file mode 100644 index 0000000..22c1ab8 --- /dev/null +++ b/examples/single-loader/a.js @@ -0,0 +1 @@ +require('./b1'); \ No newline at end of file diff --git a/examples/single-loader/b.js b/examples/single-loader/b.js new file mode 100644 index 0000000..6c8d8af --- /dev/null +++ b/examples/single-loader/b.js @@ -0,0 +1,2 @@ +require('./c'); +require('./d'); \ No newline at end of file diff --git a/examples/single-loader/c.js b/examples/single-loader/c.js new file mode 100644 index 0000000..e69de29 diff --git a/examples/single-loader/d.js b/examples/single-loader/d.js new file mode 100644 index 0000000..0baafe1 --- /dev/null +++ b/examples/single-loader/d.js @@ -0,0 +1 @@ +console.log('success'); \ No newline at end of file diff --git a/examples/single-loader/identity-loader.js b/examples/single-loader/identity-loader.js new file mode 100644 index 0000000..d8e4c40 --- /dev/null +++ b/examples/single-loader/identity-loader.js @@ -0,0 +1,3 @@ +module.exports = function(code, map) { + this.callback(null, code, map); +}; \ No newline at end of file diff --git a/examples/single-loader/webpack.config.js b/examples/single-loader/webpack.config.js new file mode 100644 index 0000000..30d8b06 --- /dev/null +++ b/examples/single-loader/webpack.config.js @@ -0,0 +1,28 @@ +var path = require('path'); +var HappyPack = require('../../'); + +module.exports = { + entry: path.resolve(__dirname, 'a.js'), + + output: { + path: path.resolve(__dirname, 'dist'), + filename: '[name].js' + }, + + plugins: [ + new HappyPack({ + cache: process.env.HAPPY_CACHE === '1', + loaders: [{ path: path.resolve(__dirname, 'identity-loader.js') }], + threads: 2 + }) + ], + + module: { + loaders: [ + { + test: /\.js$/, + loader: path.resolve(__dirname, '../../loader') + } + ] + } +}; \ No newline at end of file diff --git a/lib/HappyPlugin.js b/lib/HappyPlugin.js index a245302..7039c72 100644 --- a/lib/HappyPlugin.js +++ b/lib/HappyPlugin.js @@ -1,6 +1,7 @@ var fs = require('fs'); var path = require('path'); var async = require('async'); +var assert = require('assert'); var HappyThreadPool = require('./HappyThreadPool'); var HappyRPCHandler = require('./HappyRPCHandler'); var HappyFSCache = require('./HappyFSCache'); @@ -10,6 +11,7 @@ var HappyFakeCompiler = require('./HappyFakeCompiler'); var WebpackUtils = require('./WebpackUtils'); var JSONSerializer = require('./JSONSerializer'); var OptionParser = require('./OptionParser'); +var fnOnce = require('./fnOnce'); var pkg = require('../package.json'); var uid = 0; @@ -22,8 +24,6 @@ function HappyPlugin(userConfig) { this.name = 'HappyPack'; this.state = { started: false, - initialBuildCompleted: false, - compiler: null, foregroundWorker: null, }; @@ -34,7 +34,6 @@ function HappyPlugin(userConfig) { cache: { type: 'boolean', default: true }, cacheContext: { default: {} }, cachePath: { type: 'string' }, - installExitHandler: { type: 'boolean', default: true }, loaders: { validate: function(value) { @@ -52,9 +51,7 @@ function HappyPlugin(userConfig) { }, isRequired: true, } - }, this.id); - - HappyUtils.mkdirSync(this.config.tempDir); + }, "HappyPack[" + this.id + "]"); if (isNaN(this.config.threads)) { throw new Error("ArgumentError: Happy[" + this.config.id + "] threads option is invalid."); @@ -67,6 +64,8 @@ function HappyPlugin(userConfig) { path.resolve(this.config.tempDir, 'cache--' + this.id + '.json') ); + HappyUtils.mkdirSync(this.config.tempDir); + return this; } @@ -76,60 +75,47 @@ HappyPlugin.resetUID = function() { HappyPlugin.prototype.apply = function(compiler) { var that = this; - var inWatchMode = Boolean(compiler.options.watch); - this.state.compiler = compiler; + var engageWatchMode = fnOnce(function() { + // Once the initial build has completed, we create a foreground worker and + // perform all compilations in this thread instead: + compiler.plugin('done', function() { + that.state.foregroundWorker = createForegroundWorker(compiler, that.config.loaders); + }); - compiler.plugin('run', start); - compiler.plugin('watch-run', start); + // TODO: anything special to do here? + compiler.plugin('failed', function(err) { + console.warn('fatal watch error!!!', err); + }); + }); - compiler.plugin('compilation', function(compilation) { - if (compiler.options.bail) { - compilation.plugin('failed-module', teardown); + compiler.plugin('watch-run', function(_, done) { + if (engageWatchMode() === fnOnce.ALREADY_CALLED) { + done(); + } + else { + that.start(compiler, done); } }); - compiler.plugin('done', function(/*stats*/) { - teardown(); + compiler.plugin('run', that.start.bind(that)); - that.state.initialBuildCompleted = true; - that.state.foregroundWorker = createForegroundWorker(compiler, that.config.loaders); - }); + // cleanup hooks: + compiler.plugin('done', that.stop.bind(that)); - if (inWatchMode && this.config.installExitHandler !== false) { - process.on('exit', teardown); - process.on('SIGINT', function() { - process.exit(0); + if (compiler.options.bail) { + compiler.plugin('compilation', function(compilation) { + compilation.plugin('failed-module', that.stop.bind(that)); }); } - - function start(_, done) { - that.start(compiler, done); - } - - function teardown() { - if (that.state.started) { - if (that.config.cache) { - that.cache.save(); - } - - if (that.threadPool) { - that.threadPool.stop(); - } - - that.state.started = false; - } - } }; HappyPlugin.prototype.start = function(compiler, done) { var that = this; - if (that.state.started || that.state.initialBuildCompleted) { - return done(); - } + assert(!that.state.started, "HappyPlugin has already been started!"); - console.log('Happy[%s]: Firing up! Version: %s. Using cache? %s. Threads: %d', + console.log('Happy[%s]: Version: %s. Using cache? %s. Threads: %d', that.id, pkg.version, that.config.cache ? 'yes' : 'no', that.config.threads); async.series([ @@ -183,7 +169,6 @@ HappyPlugin.prototype.start = function(compiler, done) { callback(); }, - // TODO: accept a config file path function serializeOptions(callback) { // serialize the options so that the workers can pick them up try { @@ -218,8 +203,21 @@ HappyPlugin.prototype.start = function(compiler, done) { ], done); }; +HappyPlugin.prototype.stop = function() { + assert(this.state.started, "HappyPlugin can not be torn down until started!"); + + if (this.config.cache) { + this.cache.save(); + } + + if (this.threadPool) { + this.threadPool.stop(); + this.threadPool = null; + } +}; + HappyPlugin.prototype.compile = function(loaderContext, done) { - if (this.state.initialBuildCompleted) { + if (this.state.foregroundWorker) { return this.compileInForeground(loaderContext, done); } else { @@ -277,16 +275,25 @@ HappyPlugin.prototype.generateRequest = function(resource) { // export this so that users get to override if needed HappyPlugin.SERIALIZABLE_OPTIONS = [ + 'amd', 'bail', 'cache', 'context', + 'entry', + 'externals', 'debug', 'devtool', - 'resolve', - 'resolveLoader', + 'devServer', + 'loader', 'module', 'node', 'output', + 'profile', + 'recordsPath', + 'recordsInputPath', + 'recordsOutputPath', + 'resolve', + 'resolveLoader', 'target', 'watch', ]; @@ -304,7 +311,7 @@ HappyPlugin.extractCompilerOptions = function(options) { }; function createForegroundWorker(compiler, loaders) { - var fakeCompiler = new HappyFakeCompiler('foreground', function applyCompilerRPC(message) { + var fakeCompiler = new HappyFakeCompiler('foreground', function executeCompilerRPC(message) { // TODO: DRY alert, see HappyThread.js HappyRPCHandler.execute(message.data.type, message.data.payload, function serveRPCResult(error, result) { fakeCompiler._handleResponse({ @@ -319,4 +326,5 @@ function createForegroundWorker(compiler, loaders) { return new HappyWorker({ compiler: fakeCompiler, loaders: loaders }); } + module.exports = HappyPlugin; diff --git a/lib/HappyTestUtils.js b/lib/HappyTestUtils.js index 6aea8b8..f0147ea 100644 --- a/lib/HappyTestUtils.js +++ b/lib/HappyTestUtils.js @@ -91,7 +91,10 @@ exports.clearCache = function() { }; exports.IntegrationSuite = function(mochaSuite) { - mochaSuite.timeout(60000); // for travis + mochaSuite.timeout(process.env.HAPPY_TEST_TIMEOUT ? + parseInt(process.env.HAPPY_TEST_TIMEOUT, 10) : + 2000 + ); mochaSuite.beforeEach(function() { HappyPlugin.resetUID(); diff --git a/lib/OptionParser.js b/lib/OptionParser.js index 8430ae5..d2c4ace 100644 --- a/lib/OptionParser.js +++ b/lib/OptionParser.js @@ -58,6 +58,6 @@ module.exports = function parseAndValidateOptions(params, schema, displayName) { } function format(message) { - return "HappyPack[" + displayName + "]: " + message; + return displayName + ": " + message; } }; \ No newline at end of file diff --git a/lib/__tests__/HappyPlugin.integration.test.js b/lib/__tests__/HappyPlugin.integration.test.js index 7f7233f..766ab3b 100644 --- a/lib/__tests__/HappyPlugin.integration.test.js +++ b/lib/__tests__/HappyPlugin.integration.test.js @@ -178,72 +178,6 @@ describe('[Integration] HappyPlugin', function() { }); }); - it('works with watch mode', function(done) { - const outputDir = tempDir('integration-[guid]'); - - const loader = createLoader(function(s) { - this.addDependency(this.resource.replace('.js', '.tmp.js')); - - return s + '\n// HAPPY!'; - }); - - const file1 = TestUtils.createFile('a.js', "require('./b.js');"); - - const file2 = TestUtils.createFile('b.js', ''); - - const compiler = webpack({ - entry: { - main: file1.getPath() - }, - - output: { - filename: '[name].js', - path: outputDir - }, - - module: { - loaders: [{ - test: /.js$/, - loader: path.resolve(__dirname, '../HappyLoader.js') - }] - }, - - plugins: [ - new HappyPlugin({ - cache: false, - loaders: [loader.path] - }) - ] - }); - - let runCount = 0; - - var watcher = compiler.watch({}, function(err, rawStats) { - if (err) { return done(err); } - - const stats = rawStats.toJson(); - - if (stats.errors.length > 0) { - return done(stats.errors); - } - - assert.match(fs.readFileSync(path.join(outputDir, 'main.js'), 'utf-8'), '// HAPPY'); - - process.stdout.write(rawStats.toString({}) + "\n"); - - if (++runCount === 3) { - watcher.close(function() { - done(); - }); - } - else { - setTimeout(function() { - fs.appendFileSync(file2.getPath(), '\nconsole.log("foo");', 'utf-8'); - }, 500); - } - }); - }); - // TODO pitch loader support it.skip('works with multiple plugins', function(done) { const outputDir = tempDir('integration-[guid]'); diff --git a/lib/__tests__/integration/webpack__bail.test.js b/lib/__tests__/integration/webpack__bail.test.js new file mode 100644 index 0000000..d910a0b --- /dev/null +++ b/lib/__tests__/integration/webpack__bail.test.js @@ -0,0 +1,43 @@ +'use strict'; + +const HappyPlugin = require('../../HappyPlugin'); +const TestUtils = require('../../HappyTestUtils'); +const webpack = require('webpack'); +const { assert, } = require('../../HappyTestUtils'); + +describe('[Integration] webpack --bail', function() { + TestUtils.IntegrationSuite(this); + + it('works', function(done) { + const loader = TestUtils.createLoader(s => s); + + const compiler = webpack({ + bail: true, + + entry: TestUtils.createFile('a.js', 'require("./b1");').getPath(), + output: { + path: TestUtils.tempDir('integration-[guid]') + }, + + module: { + loaders: [{ + test: /.js$/, + loader: TestUtils.HAPPY_LOADER_PATH + }] + }, + + plugins: [ + new HappyPlugin({ + cache: false, + loaders: [ loader.path ], + }) + ] + }); + + compiler.run(function(err) { + assert.match(err.toString(), /ModuleNotFoundError/); + + done(); + }); + }); +}); \ No newline at end of file diff --git a/lib/__tests__/integration/webpack__watch.test.js b/lib/__tests__/integration/webpack__watch.test.js new file mode 100644 index 0000000..2b53fdb --- /dev/null +++ b/lib/__tests__/integration/webpack__watch.test.js @@ -0,0 +1,94 @@ +'use strict'; + +const webpack = require('webpack'); +const path = require('path'); +const fs = require('fs'); +const HappyPlugin = require('../../HappyPlugin'); +const TestUtils = require('../../HappyTestUtils'); +const { assert, } = require('../../HappyTestUtils'); + +describe('[Integration] webpack --watch', function() { + TestUtils.IntegrationSuite(this); + + it('works', function(done) { + const outputDir = TestUtils.tempDir('integration-[guid]'); + + const loader = TestUtils.createLoader(function(s) { + this.addDependency(this.resource.replace('.js', '.tmp.js')); + + return s + '\n// HAPPY!'; + }); + + const file1 = TestUtils.createFile('a.js', "require('./b.js');"); + const file2 = TestUtils.createFile('b.js', ''); + + const compiler = webpack({ + entry: { + main: file1.getPath() + }, + + watch: true, + + output: { + filename: '[name].js', + path: outputDir + }, + + module: { + loaders: [{ + test: /.js$/, + loader: TestUtils.HAPPY_LOADER_PATH + }] + }, + + plugins: [ + new HappyPlugin({ + cache: false, + loaders: [loader.path] + }) + ] + }); + + let runCount = 0; + let watcher; + + const closeAndEmitDone = (err) => { + try { + watcher.close(); + } + catch(e) { + console.warn('unable to stop watcher:', e); + } + finally { + done(err); + } + }; + + watcher = compiler.watch({}, function(err, rawStats) { + if (err) { + return closeAndEmitDone(err); + } + + const stats = rawStats.toJson(); + + if (stats.errors.length > 0) { + return done(stats.errors); + } + + assert.match(fs.readFileSync(path.join(outputDir, 'main.js'), 'utf-8'), '// HAPPY'); + + process.stdout.write(rawStats.toString({}) + "\n"); + + if (++runCount === 3) { + watcher.close(function() { + done(); + }); + } + else { + setTimeout(function() { + fs.appendFileSync(file2.getPath(), '\nconsole.log("foo");', 'utf-8'); + }, 250); + } + }); + }); +}); \ No newline at end of file diff --git a/lib/fnOnce.js b/lib/fnOnce.js index 636d2f2..4b4a7ff 100644 --- a/lib/fnOnce.js +++ b/lib/fnOnce.js @@ -1,4 +1,4 @@ -module.exports = function Once(fn) { +function Once(fn) { var called = false; return function() { @@ -6,5 +6,12 @@ module.exports = function Once(fn) { called = true; return fn.apply(null, arguments); } + else { + return Once.ALREADY_CALLED; + } } -}; \ No newline at end of file +} + +Once.ALREADY_CALLED = Object.freeze('Once.ALREADY_CALLED'); + +module.exports = Once; \ No newline at end of file