From ef8a3dbae118283d85ba0ed74071fca9b3aa7255 Mon Sep 17 00:00:00 2001 From: Ian Faigao Date: Wed, 19 Feb 2014 10:21:22 +1300 Subject: [PATCH 1/6] Added callbacks on error handling. Removed the log.error call on the compressor callback to continue processing. --- lib/builder.js | 9 ++++++--- lib/index.js | 22 +++++++++++++--------- lib/module.js | 20 ++++++++++++++------ 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/lib/builder.js b/lib/builder.js index 9e804a5..0925fb0 100644 --- a/lib/builder.js +++ b/lib/builder.js @@ -81,12 +81,12 @@ exports.start = function (json, options, buildCallback) { start = new Date(), hasSkin = false, buildStat, - end = function () { + end = function (err) { var end = new Date(); log.info('done racing, the gears are toast'); log.info('finished in ' + timer.calc(start, end) + ', pretty fast huh?'); if (buildCallback) { - buildCallback(); + buildCallback(err); } }, post = function (json, callback) { @@ -168,7 +168,10 @@ exports.start = function (json, options, buildCallback) { } }); - stack.done(function () { + stack.done(function (err) { + if (err) { + return end(err); + } var rollups = []; if (json.rollups) { log.info('build has rollup builds, shifting them now'); diff --git a/lib/index.js b/lib/index.js index bb64ae7..19fd5af 100644 --- a/lib/index.js +++ b/lib/index.js @@ -34,10 +34,14 @@ var runQueue = function() { var item = queue.pop(); if (item) { buildRunning = true; - exports.init(item.opts, function() { + exports.init(item.opts, function(err) { buildRunning = false; - item.callback(); - runQueue(); + if (err) { + item.callback(err); + } else { + item.callback(); + runQueue(); + } }); } } @@ -82,7 +86,7 @@ exports.init = function (opts, initCallback) { require('./help'); return; } - + if (options.quiet) { log.quiet(); } @@ -90,7 +94,7 @@ exports.init = function (opts, initCallback) { if (options.silent) { log.silent(); } - + if (options['global-config']) { log.info('racing to find the closest .shifter.json file'); find(CWD, '.shifter.json', function(err, file) { @@ -153,10 +157,10 @@ exports.init = function (opts, initCallback) { } else { builder = require('./builder'); builder.reset(); - builder.start(json, options, function() { + builder.start(json, options, function(err) { buildRunning = false; if (initCallback) { - initCallback(); + initCallback(err); } }); } @@ -187,10 +191,10 @@ exports.init = function (opts, initCallback) { builder.reset(); builder.start({ builds: mods - }, options, function() { + }, options, function(err) { buildRunning = false; if (initCallback) { - initCallback(); + initCallback(err); } }); } diff --git a/lib/module.js b/lib/module.js index fa31697..9893b4d 100644 --- a/lib/module.js +++ b/lib/module.js @@ -55,7 +55,6 @@ var Stack = require('./stack').Stack, callback: function (e) { log.err('compression failed'); log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white')); - log.error('dropped the clutch, build failed'); } }; } @@ -657,7 +656,7 @@ var setReplacers = function (options) { }; var build = function (mod, name, options, callback) { - var stack = new Stack(), _build; + var stack = new Stack(), _build, buildFailed = false; if (options.lint === false) { defaultLint = options.lint; @@ -688,6 +687,7 @@ var build = function (mod, name, options, callback) { exports.js(mod, name, stack.add(function (err) { if (err) { log.warn('skipping coverage file build due to previous build error'); + buildFailed = true; } else { if (options.coverage) { exports.coverage(mod, name, stack.add(noop)); @@ -742,7 +742,11 @@ var build = function (mod, name, options, callback) { stack.done(function () { if (!stack.complete) { stack.complete = true; - callback(); + if (buildFailed) { + callback('build failed, errors encountered in 1 or more files, see log for more info.'); + } else { + callback(); + } } }); }; @@ -811,11 +815,11 @@ exports.build = function (mod, name, options, callback) { mod.buildDir = options['build-dir']; - var end = function () { + var end = function (err) { if (mod.postexec) { exec(mod.postexec, name, callback); } else { - callback(); + callback(err); } }; if (mod.exec) { @@ -919,7 +923,11 @@ exports.rollup = function (mods, callback) { } } } - build(mod.build, name, options, function () { + build(mod.build, name, options, function (err) { + if (err) { + callback(err); + return; + } delete mod.build; log.info('sub build complete, up shifting to rollup'); _rollup(mod, name, options, function () { From 41e40cad0afe80d9e470962be7b1f6a40c074d9b Mon Sep 17 00:00:00 2001 From: Ian Faigao Date: Tue, 25 Feb 2014 17:21:56 +1300 Subject: [PATCH 2/6] Added callback handling on log.error() calls. Added unit tests and documentation --- conf/docs/index.mustache | 2 + lib/ant.js | 2 +- lib/builder.js | 6 +- lib/index.js | 14 +- lib/log.js | 8 +- lib/module.js | 22 +- lib/walk.js | 2 +- output/index.html | 2 + tests/13-builder-uglify-badmodule.js | 54 +++++ tests/assets/badmodule/src/yql/assets/foo.png | 1 + tests/assets/badmodule/src/yql/build.json | 16 ++ tests/assets/badmodule/src/yql/js/yql.js | 193 ++++++++++++++++++ tests/assets/badmodule/src/yql/meta/yql.json | 8 + tests/assets/badmodule/src/yql/test.json | 10 + 14 files changed, 321 insertions(+), 19 deletions(-) create mode 100644 tests/13-builder-uglify-badmodule.js create mode 100644 tests/assets/badmodule/src/yql/assets/foo.png create mode 100644 tests/assets/badmodule/src/yql/build.json create mode 100644 tests/assets/badmodule/src/yql/js/yql.js create mode 100644 tests/assets/badmodule/src/yql/meta/yql.json create mode 100644 tests/assets/badmodule/src/yql/test.json diff --git a/conf/docs/index.mustache b/conf/docs/index.mustache index 7bf9e34..76b556a 100644 --- a/conf/docs/index.mustache +++ b/conf/docs/index.mustache @@ -459,6 +459,8 @@ foo.bar = function () { If there is an issue with compressing your module with Uglify, you can revert to using YUI Compressor with the `--compressor` flag.

+

UglifyJS parse errors are now propagated through Shifter's callback routines so that the build does not terminate if Shifter is called programmatically.

+

Forcing lint to stderr

diff --git a/lib/ant.js b/lib/ant.js index 38e97dc..c652a34 100644 --- a/lib/ant.js +++ b/lib/ant.js @@ -137,7 +137,7 @@ exports.process = function (options, callback) { } }); } else { - log.error('no .properties files located, hit the brakes'); + log.error('no .properties files located, hit the brakes', callback); } }); diff --git a/lib/builder.js b/lib/builder.js index 0925fb0..df03bb2 100644 --- a/lib/builder.js +++ b/lib/builder.js @@ -67,7 +67,7 @@ var prebuild = function (jobs, options, callback) { child.on('exit', stack.add(function (code) { log.info('build for ' + job + ' complete, downshifting'); if (code) { - log.error('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!'); + log.error('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!', callback); } })); }); @@ -129,7 +129,7 @@ exports.start = function (json, options, buildCallback) { json2 = JSON.parse(fs.readFileSync(options.buildFile, 'utf8')); } catch (e) { console.log(e.stack); - log.error('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?'); + log.error('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?', buildCallback); } if (pack.valid(json2)) { @@ -139,7 +139,7 @@ exports.start = function (json, options, buildCallback) { exports.start(json2, options, buildCallback); }); } else { - log.error('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!'); + log.error('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!', buildCallback); } } else { exports.start(json, options, buildCallback); diff --git a/lib/index.js b/lib/index.js index 19fd5af..b57682e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -133,7 +133,7 @@ exports.init = function (opts, initCallback) { var json, walk, ant, mods, builder; if (yes) { if (options.ant) { - log.error('already has a ' + buildFileName + ' file, hitting the brakes'); + log.error('already has a ' + buildFileName + ' file, hitting the brakes', initCallback); } log.info('found ' + buildFileName + ' file, shifting'); if (path.extname(buildFileName) === '.json') { @@ -141,7 +141,7 @@ exports.init = function (opts, initCallback) { json = require(buildFile); } catch (e) { console.log(e.stack); - log.error('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?'); + log.error('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?', initCallback); } if (pack.valid(json)) { log.info('putting the hammer down, let\'s build this thing!'); @@ -161,12 +161,14 @@ exports.init = function (opts, initCallback) { buildRunning = false; if (initCallback) { initCallback(err); + } else { + process.exit(err ? 1 : 0); } }); } }); } else { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); + log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); } } else if (path.extname(buildFileName) === '.js') { // probably a row module @@ -183,7 +185,7 @@ exports.init = function (opts, initCallback) { vm.runInContext(fs.readFileSync(buildFile, 'utf8'), contextForRunInContext, buildFile); } catch (e) { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); + log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); } if (mods) { // raw yui module without build.json @@ -195,11 +197,13 @@ exports.init = function (opts, initCallback) { buildRunning = false; if (initCallback) { initCallback(err); + } else { + process.exit(err ? 1 : 0); } }); } } else { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); + log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); } } else { if (options.walk) { diff --git a/lib/log.js b/lib/log.js index af7e442..2dedacb 100644 --- a/lib/log.js +++ b/lib/log.js @@ -57,11 +57,15 @@ exports.warn = function (str) { } }; -exports.error = function (str) { +exports.error = function (str, callback) { if (!silent) { console.error(prefix, exports.color('[error]', 'red'), str); } - process.exit(1); + if (callback) { + callback(str); + } else { + process.exit(1); + } }; exports.err = function (str) { diff --git a/lib/module.js b/lib/module.js index 9893b4d..4caf3f0 100644 --- a/lib/module.js +++ b/lib/module.js @@ -41,7 +41,7 @@ var Stack = require('./stack').Stack, coverageType = 'yuitest', compressorFn, compressorConfig, - configCompressor = function (options) { + configCompressor = function (options, cb) { if (options.compressor) { compressorFn = 'compressor'; compressorConfig = { @@ -55,6 +55,13 @@ var Stack = require('./stack').Stack, callback: function (e) { log.err('compression failed'); log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white')); + if (cb) { + log.error('dropped the clutch, build failed', function () { + cb(e.message); + }); + } else { + log.error('dropped the clutch, build failed'); + } } }; } @@ -363,7 +370,7 @@ var buildJS = function (mod, name, callback) { log.warn(name + ': ' + err); } else { if (/ENOENT/.test(err)) { - log.error('Failed to open file: ' + err.path); + log.error('Failed to open file: ' + err.path, callback); } log.err(name + ': ' + err); } @@ -498,7 +505,7 @@ var buildSkin = function (mod, name, callback) { fs.readdir(path.join(shifter.cwd(), 'assets', subMod, 'skins'), stack.add(function (err, skins) { if (err) { log.console.log(err); - log.error('skin files are not right!'); + log.error('skin files are not right!', callback); } //Walk the skins and write them out @@ -540,7 +547,7 @@ var buildSkin = function (mod, name, callback) { if (err) { log.err(err); if (err.code === 'ENOENT') { - log.error('skin file is missing: ' + err.path); + log.error('skin file is missing: ' + err.path, callback); } } @@ -670,7 +677,7 @@ var build = function (mod, name, options, callback) { setJSLint(); } - configCompressor(options); + configCompressor(options, callback); setReplacers(options); cacheBuild = options.cache; @@ -716,6 +723,7 @@ var build = function (mod, name, options, callback) { exports.css(mod, name, stack.add(function (err) { if (err) { log.warn('skipping assets copy due to previous build error'); + buildFailed = true; } else { if (options.assets && mod.assets) { copyAssets(mod, name, stack.add(noop)); @@ -884,7 +892,7 @@ var _rollup = function (mod, name, options, callback) { .write(path.join(mod.buildDir, fileName, fileName + '-min.js')) .run(function (err) { if (err) { - log.error(name + ' rollup: ' + err); + log.error(name + ' rollup: ' + err, callback); } callback(); }); @@ -909,7 +917,7 @@ exports.rollup = function (mods, callback) { mod = item.mod; setReplacers(options); - configCompressor(options); + configCompressor(options, callback); if (mod.build) { log.info('found a sub build, down shifting'); diff --git a/lib/walk.js b/lib/walk.js index 81440d7..e1eb872 100644 --- a/lib/walk.js +++ b/lib/walk.js @@ -134,7 +134,7 @@ exports.run = function (options, callback) { modStack.done(function () { if (!mods.length) { - log.error('no modules found, hitting the brakes.'); + log.error('no modules found, hitting the brakes.', callback); } if (bar) { bar.total = mods.length - 1; diff --git a/output/index.html b/output/index.html index 4aea893..0d42153 100644 --- a/output/index.html +++ b/output/index.html @@ -474,6 +474,8 @@

using YUI Compressor

If there is an issue with compressing your module with Uglify, you can revert to using YUI Compressor with the --compressor flag.

+

UglifyJS parsing errors are now propagated through Shifter's callback routines so that the build does not terminate if Shifter is called programmatically.

+

Forcing lint to stderr

diff --git a/tests/13-builder-uglify-badmodule.js b/tests/13-builder-uglify-badmodule.js new file mode 100644 index 0000000..0c90072 --- /dev/null +++ b/tests/13-builder-uglify-badmodule.js @@ -0,0 +1,54 @@ +var vows = require('vows'), + assert = require('assert'), + path = require('path'), + fs = require('fs'), + shifter = require('../lib'), + base = path.join(__dirname, 'assets/badmodule/'), + buildBase = path.join(base, 'build'), + srcBase = path.join(base, 'src/yql'), + rimraf = require('rimraf'); + +var tests = { + 'clean build': { + topic: function() { + rimraf(path.join(buildBase, 'yql'), this.callback); + }, + 'should not have build dir and': { + topic: function() { + var self = this; + fs.stat(path.join(buildBase, 'yql'), function(err) { + self.callback(null, err); + }); + }, + 'should not have build/yql': function(foo, err) { + assert.isNotNull(err); + assert.equal(err.code, 'ENOENT'); + }, + 'should build yql and': { + topic: function() { + var self = this; + + shifter.add({ + silent: false, + cwd: srcBase, + compressor: false, + 'global-config': false, + 'cache': false, + lint: 'config', + 'replace-version': '1.2.3.4' + }, function(err) { + self.callback(null, err); + }); + }, + 'should fail with an error': function (foo, err) { + assert.isNotNull(err); + }, + 'should fail with an error message': function(foo, err) { + assert.isString(err.toString()); + } + } + } + } +}; + +vows.describe('building badmodule with UglifyJS').addBatch(tests).export(module); diff --git a/tests/assets/badmodule/src/yql/assets/foo.png b/tests/assets/badmodule/src/yql/assets/foo.png new file mode 100644 index 0000000..ccd24a9 --- /dev/null +++ b/tests/assets/badmodule/src/yql/assets/foo.png @@ -0,0 +1 @@ +// totally an image diff --git a/tests/assets/badmodule/src/yql/build.json b/tests/assets/badmodule/src/yql/build.json new file mode 100644 index 0000000..4e198b9 --- /dev/null +++ b/tests/assets/badmodule/src/yql/build.json @@ -0,0 +1,16 @@ +{ + "name": "yql", + "builds": { + "yql": { + "assets": false, + "exec": [ + "shifter --config test.json --no-global-config", + "echo 'Foobar'" + ], + "jsfiles": [ + "yql.js" + ], + "cssfiles": [] + } + } +} diff --git a/tests/assets/badmodule/src/yql/js/yql.js b/tests/assets/badmodule/src/yql/js/yql.js new file mode 100644 index 0000000..a2cd232 --- /dev/null +++ b/tests/assets/badmodule/src/yql/js/yql.js @@ -0,0 +1,193 @@ + +/*! +ENSURE THIS STAYS AT MIN TIME +Copyright 2012 Yahoo! Inc. All rights reserved. +Licensed under the BSD License. +http://yuilibrary.com/license/ +*/ + + /** + * This class adds a sugar class to allow access to YQL (http://developer.yahoo.com/yql/). + * @module yql + */ + /** + * Utility Class used under the hood my the YQL class + * @class YQLRequest + * @constructor + * @param {String} sql The SQL statement to execute + * @param {Function/Object} callback The callback to execute after the query (Falls through to JSONP). + * @param {Object} params An object literal of extra parameters to pass along (optional). + * @param {Object} opts An object literal of configuration options (optional): proto (http|https), base (url) + */ + var YQLRequest = function (sql, callback, params, opts) { + this._types = { + esc: { + token: '\uE000', + re: /\\[:\[\]\(\)#\.\'\>+~"]/gi + }, + + attr: { + token: '\uE001', + re: /(\[[^\]]*\])/g + }, + + pseudo: { + token: '\uE002', + re: /(\([^\)]*\))/g + } + }; + + if (!params) { + params = {}; + // Invalid character + # + } + params.q = sql; + //Allow format override.. JSON-P-X + if (!params.format) { + params.format = Y.YQLRequest.FORMAT; + } + if (!params.env) { + params.env = Y.YQLRequest.ENV; + } + + this._context = this; + + if (opts && opts.context) { + this._context = opts.context; + delete opts.context; + } + + if (params && params.context) { + this._context = params.context; + delete params.context; + } + + this._params = params; + this._opts = opts; + this._callback = callback; + + }; + + YQLRequest.prototype = { + /** + * @private + * @property _jsonp + * @description Reference to the JSONP instance used to make the queries + */ + _jsonp: null, + /** + * @private + * @property _opts + * @description Holder for the opts argument + */ + _opts: null, + /** + * @private + * @property _callback + * @description Holder for the callback argument + */ + _callback: null, + /** + * @private + * @property _params + * @description Holder for the params argument + */ + _params: null, + /** + * @private + * @property _context + * @description The context to execute the callback in + */ + _context: null, + /** + * @private + * @method _internal + * @description Internal Callback Handler + */ + _internal: function() { + this._callback.apply(this._context, arguments); + }, + /** + * @method send + * @description The method that executes the YQL Request. + * @chainable + * @return {YQLRequest} + */ + send: function() { + var qs = [], url = ((this._opts && this._opts.proto) ? this._opts.proto : Y.YQLRequest.PROTO), + o; + + Y.each(this._params, function(v, k) { + qs.push(k + '=' + encodeURIComponent(v)); + }); + + qs = qs.join('&'); + + url += ((this._opts && this._opts.base) ? this._opts.base : Y.YQLRequest.BASE_URL) + qs; + + o = (!Y.Lang.isFunction(this._callback)) ? this._callback : { on: { success: this._callback } }; + + o.on = o.on || {}; + this._callback = o.on.success; + + o.on.success = Y.bind(this._internal, this); + + if (o.allowCache !== false) { + o.allowCache = true; + } + Y.log('URL: ' + url, 'info', 'yql'); + + if (!this._jsonp) { + this._jsonp = Y.jsonp(url, o); + } else { + this._jsonp.url = url; + if (o.on && o.on.success) { + this._jsonp._config.on.success = o.on.success; + } + this._jsonp.send(); + } + return this; + } + }; + + /** + * @static + * @property FORMAT + * @description Default format to use: json + */ + YQLRequest.FORMAT = 'json'; + /** + * @static + * @property PROTO + * @description Default protocol to use: http + */ + YQLRequest.PROTO = 'http'; + /** + * @static + * @property BASE_URL + * @description The base URL to query: query.yahooapis.com/v1/public/yql? + */ + YQLRequest.BASE_URL = ':/'+'/query.yahooapis.com/v1/public/yql?'; + /** + * @static + * @property ENV + * @description The environment file to load: http://datatables.org/alltables.env + */ + YQLRequest.ENV = 'http:/'+'/datatables.org/alltables.env'; + + Y.YQLRequest = YQLRequest; + + /** + * This class adds a sugar class to allow access to YQL (http://developer.yahoo.com/yql/). + * @class YQL + * @constructor + * @param {String} sql The SQL statement to execute + * @param {Function} callback The callback to execute after the query (optional). + * @param {Object} params An object literal of extra parameters to pass along (optional). + * @param {Object} opts An object literal of configuration options (optional): proto (http|https), base (url) + */ + Y.YQL = function(sql, callback, params, opts) { + return new Y.YQLRequest(sql, callback, params, opts).send(); + }; + diff --git a/tests/assets/badmodule/src/yql/meta/yql.json b/tests/assets/badmodule/src/yql/meta/yql.json new file mode 100644 index 0000000..815e422 --- /dev/null +++ b/tests/assets/badmodule/src/yql/meta/yql.json @@ -0,0 +1,8 @@ +{ + "yql": { + "requires": [ + "jsonp", + "jsonp-url" + ] + } +} diff --git a/tests/assets/badmodule/src/yql/test.json b/tests/assets/badmodule/src/yql/test.json new file mode 100644 index 0000000..9aa6b08 --- /dev/null +++ b/tests/assets/badmodule/src/yql/test.json @@ -0,0 +1,10 @@ +{ + "name": "yql23", + "builds": { + "yql": { + "jsfiles": [ + "yql.js" + ] + } + } +} From 8f0decc13138c2239dea2155e9fd1085151ea60f Mon Sep 17 00:00:00 2001 From: Ian Faigao Date: Wed, 26 Feb 2014 16:08:40 +1300 Subject: [PATCH 3/6] Code improvements. Added 2 more tests for bad uglifyJS builds on command line and rollups. Improved the documentation. --- conf/docs/index.mustache | 8 +- lib/module.js | 16 +- output/index.html | 11 +- tests/14-builder-uglify-badmodule-cmd.js | 52 ++++ tests/15-builder-uglify-event-badrollup.js | 58 ++++ tests/assets/badrollup/src/event/build.json | 30 ++ .../src/event/js/event-facade-dom-ie.js | 258 ++++++++++++++++++ .../src/event/js/event-ready-base-ie.js | 40 +++ .../badrollup/src/event/meta/event.json | 110 ++++++++ .../badrollup/src/event/meta/ie-base-test.js | 4 + 10 files changed, 571 insertions(+), 16 deletions(-) create mode 100644 tests/14-builder-uglify-badmodule-cmd.js create mode 100644 tests/15-builder-uglify-event-badrollup.js create mode 100644 tests/assets/badrollup/src/event/build.json create mode 100644 tests/assets/badrollup/src/event/js/event-facade-dom-ie.js create mode 100644 tests/assets/badrollup/src/event/js/event-ready-base-ie.js create mode 100644 tests/assets/badrollup/src/event/meta/event.json create mode 100644 tests/assets/badrollup/src/event/meta/ie-base-test.js diff --git a/conf/docs/index.mustache b/conf/docs/index.mustache index 76b556a..732e2c5 100644 --- a/conf/docs/index.mustache +++ b/conf/docs/index.mustache @@ -459,8 +459,6 @@ foo.bar = function () { If there is an issue with compressing your module with Uglify, you can revert to using YUI Compressor with the `--compressor` flag.

-

UglifyJS parse errors are now propagated through Shifter's callback routines so that the build does not terminate if Shifter is called programmatically.

-

Forcing lint to stderr

@@ -487,3 +485,9 @@ I've added a `--lint-stderr` config option which forces all lint output to `stde

Adding `--recursive` to the `--walk` command will tell `shifter` to walk the directories recursively looking for `build.json` files.

+ +

Error handling when shifter is called programmatically

+ +

+ When `shifter` is called programmatically, build errors such as UglifyJS parsing errors, are propagated through callback routines. This allows another external build process that uses `shifter` to handle the errors accordingly. +

diff --git a/lib/module.js b/lib/module.js index 4caf3f0..746f439 100644 --- a/lib/module.js +++ b/lib/module.js @@ -55,13 +55,9 @@ var Stack = require('./stack').Stack, callback: function (e) { log.err('compression failed'); log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white')); - if (cb) { - log.error('dropped the clutch, build failed', function () { - cb(e.message); - }); - } else { - log.error('dropped the clutch, build failed'); - } + log.error('dropped the clutch, build failed', function () { + cb(e.message); + }); } }; } @@ -931,11 +927,7 @@ exports.rollup = function (mods, callback) { } } } - build(mod.build, name, options, function (err) { - if (err) { - callback(err); - return; - } + build(mod.build, name, options, function () { delete mod.build; log.info('sub build complete, up shifting to rollup'); _rollup(mod, name, options, function () { diff --git a/output/index.html b/output/index.html index 0d42153..157f41d 100644 --- a/output/index.html +++ b/output/index.html @@ -474,8 +474,6 @@

using YUI Compressor

If there is an issue with compressing your module with Uglify, you can revert to using YUI Compressor with the --compressor flag.

-

UglifyJS parsing errors are now propagated through Shifter's callback routines so that the build does not terminate if Shifter is called programmatically.

-

Forcing lint to stderr

@@ -501,6 +499,12 @@

Recursive Building

Adding --recursive to the --walk command will tell shifter to walk the directories recursively looking for build.json files.

+ +

Error handling when shifter is called programmatically

+ +

+ When shifter is called programmatically, build errors such as UglifyJS parsing errors, are propagated through callback routines. This allows another external build process that uses shifter to handle the errors accordingly. +

@@ -608,6 +612,9 @@

Table of Contents

  • Recursive Building
    • +
  • +Error handling when shifter is called programmatically +
    • diff --git a/tests/14-builder-uglify-badmodule-cmd.js b/tests/14-builder-uglify-badmodule-cmd.js new file mode 100644 index 0000000..ea93b32 --- /dev/null +++ b/tests/14-builder-uglify-badmodule-cmd.js @@ -0,0 +1,52 @@ +var vows = require('vows'), + assert = require('assert'), + path = require('path'), + fs = require('fs'), + exec = require('child_process').exec, + base = path.join(__dirname, 'assets/badmodule/'), + buildBase = path.join(base, 'build'), + srcBase = path.join(base, 'src/yql'), + rimraf = require('rimraf'); + +var tests = { + 'clean build': { + topic: function() { + rimraf(path.join(buildBase, 'yql'), this.callback); + }, + 'should not have build dir and': { + topic: function() { + var self = this; + fs.stat(path.join(buildBase, 'yql'), function(err) { + self.callback(null, err); + }); + }, + 'should not have build/yql': function(foo, err) { + assert.isNotNull(err); + assert.equal(err.code, 'ENOENT'); + }, + 'should build yql and': { + topic: function() { + var self = this, + child; + + process.chdir(path.resolve(base, srcBase)); + + child = exec('../../../../../bin/shifter --config test.json --no-global-config', function (error, stdout, stderr) { + self.callback(null, { + error: error, + stderr: stderr + }); + }); + }, + 'should fail with an error code 1': function (topic) { + assert.equal(topic.error.code, 1); + }, + 'should fail with an error message': function(topic) { + assert.isNotNull(topic.stderr); + } + } + } + } +}; + +vows.describe('building badmodule with UglifyJS via command line').addBatch(tests).export(module); diff --git a/tests/15-builder-uglify-event-badrollup.js b/tests/15-builder-uglify-event-badrollup.js new file mode 100644 index 0000000..4faa68c --- /dev/null +++ b/tests/15-builder-uglify-event-badrollup.js @@ -0,0 +1,58 @@ +var vows = require('vows'), + assert = require('assert'), + path = require('path'), + fs = require('fs'), + shifter = require('../lib'), + base = path.join(__dirname, 'assets/badrollup/'), + buildBase = path.join(base, 'build'), + srcBase = path.join(base, 'src/event'), + rimraf = require('rimraf'); + + +var tests = { + 'clean build': { + topic: function() { + rimraf(path.join(buildBase, 'event-base-ie'), this.callback); + }, + 'should not have build dir and': { + topic: function() { + var self = this; + fs.stat(path.join(buildBase, 'event-base-ie'), function(err) { + self.callback(null, err); + }); + }, + 'should not have build/event-base-ie': function(foo, err) { + assert.isNotNull(err); + assert.equal(err.code, 'ENOENT'); + }, + 'should build Event Base IE and': { + topic: function() { + var self = this; + + shifter.init({ + silent: true, + cwd: srcBase, + 'global-config': false, + 'cache': false + }, function (err) { + self.callback(null, err); + }); + }, + 'should fail with an error': function (foo, err) { + assert.isNotNull(err); + }, + 'should create build dir and': { + topic: function() { + fs.stat(path.join(buildBase, 'event-base-ie'), this.callback); + }, + 'should create build/event-base-ie': function(err, stat) { + assert.isNull(err); + assert.isTrue(stat.isDirectory()); + } + } + } + } + } +}; + +vows.describe('building badrollup event with UglifyJS').addBatch(tests).export(module); diff --git a/tests/assets/badrollup/src/event/build.json b/tests/assets/badrollup/src/event/build.json new file mode 100644 index 0000000..a44ce0b --- /dev/null +++ b/tests/assets/badrollup/src/event/build.json @@ -0,0 +1,30 @@ +{ + "name": "event", + "builds": { + "event-base-ie": { + "prependfiles": [ + "js/event-ready-base-ie.js" + ], + "jsfiles": [ + "event-facade-dom-ie.js" + ] + } + }, + "rollups": { + "event-base": { + "files": [ + "event-base-ie" + ], + "build": { + "event-base-ie": { + "prependfiles": [ + "js/event-ready-base-ie.js" + ], + "jsfiles": [ + "event-facade-dom-ie.js" + ] + } + } + } + } +} diff --git a/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js b/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js new file mode 100644 index 0000000..a671646 --- /dev/null +++ b/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js @@ -0,0 +1,258 @@ +/* + * Custom event engine, DOM event listener abstraction layer, synthetic DOM + * events. + * @module event + * @submodule event-base + */ + +function IEEventFacade() { + // IEEventFacade.superclass.constructor.apply(this, arguments); + Y.DOM2EventFacade.apply(this, arguments); +} + +/* + * (intentially left out of API docs) + * Alternate Facade implementation that is based on Object.defineProperty, which + * is partially supported in IE8. Properties that involve setup work are + * deferred to temporary getters using the static _define method. + */ +function IELazyFacade(e) { + var proxy = Y.config.doc.createEventObject(e), + proto = IELazyFacade.prototype; + + // TODO: necessary? + proxy.hasOwnProperty = function () { return true; }; + + proxy.init = proto.init; + proxy.halt = proto.halt; + proxy.preventDefault = proto.preventDefault; + proxy.stopPropagation = proto.stopPropagation; + proxy.stopImmediatePropagation = proto.stopImmediatePropagation; + + Y.DOM2EventFacade.apply(proxy, arguments); + + return proxy; +} + + +var imp = Y.config.doc && Y.config.doc.implementation, + useLazyFacade = Y.config.lazyEventFacade, + + buttonMap = { + 0: 1, // left click + 4: 2, // middle click + 2: 3 // right click + }, + relatedTargetMap = { + mouseout: 'toElement', + mouseover: 'fromElement' + }, + + resolve = Y.DOM2EventFacade.resolve, + + proto = { + init: function() { + + IEEventFacade.superclass.init.apply(this, arguments); + + var e = this._event, + x, y, d, b, de, t; + + this.target = resolve(e.srcElement); + + if (('clientX' in e) && (!x) && (0 !== x)) { + x = e.clientX; + y = e.clientY; + + d = Y.config.doc; + b = d.body; + de = d.documentElement; + + x += (de.scrollLeft || (b && b.scrollLeft) || 0); + y += (de.scrollTop || (b && b.scrollTop) || 0); + + this.pageX = x; + this.pageY = y; + } + + if (e.type == "mouseout") { + t = e.toElement; + } else if (e.type == "mouseover") { + t = e.fromElement; + } + + // fallback to t.relatedTarget to support simulated events. + // IE doesn't support setting toElement or fromElement on generic + // events, so Y.Event.simulate sets relatedTarget instead. + this.relatedTarget = resolve(t || e.relatedTarget); + + // which should contain the unicode key code if this is a key event. + // For click events, which is normalized for which mouse button was + // clicked. + this.which = // chained assignment + this.button = e.keyCode || buttonMap[e.button] || e.button; + }, + + stopPropagation: function() { + this._event.cancelBubble = true; + this._wrapper.stopped = 1; + this.stopped = 1; + }, + + stopImmediatePropagation: function() { + this.stopPropagation(); + this._wrapper.stopped = 2; + this.stopped = 2; + }, + + preventDefault: function(returnValue) { + this._event.returnValue = returnValue || false; + this._wrapper.prevented = 1; + this.prevented = 1; + } + }; + +Y.extend(IEEventFacade, Y.DOM2EventFacade, proto); + +Y.extend(IELazyFacade, Y.DOM2EventFacade, proto); +IELazyFacade.prototype.init = function () { + var e = this._event, + overrides = this._wrapper.overrides, + define = IELazyFacade._define, + lazyProperties = IELazyFacade._lazyProperties, + prop; + + this.altKey = e.altKey; + this.ctrlKey = e.ctrlKey; + this.metaKey = e.metaKey; + this.shiftKey = e.shiftKey; + this.type = (overrides && overrides.type) || e.type; + this.clientX = e.clientX; + this.clientY = e.clientY; + this.keyCode = // chained assignment + this.charCode = e.keyCode; + this.which = // chained assignment + this.button = e.keyCode || buttonMap[e.button] || e.button; + + for (prop in lazyProperties) { + if (lazyProperties.hasOwnProperty(prop)) { + define(this, prop, lazyProperties[prop]); + } + } + + if (this._touch) { + this._touch(e, this._currentTarget, this._wrapper); + } +}; + +IELazyFacade._lazyProperties = { + target: function () { + return resolve(this._event.srcElement); + }, + relatedTarget: function () { + var e = this._event, + targetProp = relatedTargetMap[e.type] || 'relatedTarget'; + + // fallback to t.relatedTarget to support simulated events. + // IE doesn't support setting toElement or fromElement on generic + // events, so Y.Event.simulate sets relatedTarget instead. + return resolve(e[targetProp] || e.relatedTarget); + }, + currentTarget: function () { + return resolve(this._currentTarget); + }, + + wheelDelta: function () { + var e = this._event; + + if (e.type === "mousewheel" || e.type === "DOMMouseScroll") { + return (e.detail) ? + (e.detail * -1) : + // wheelDelta between -80 and 80 result in -1 or 1 + Math.round(e.wheelDelta / 80) || ((e.wheelDelta < 0) ? -1 : 1); + } + }, + + pageX: function () { + var e = this._event, + val = e.pageX, + doc, bodyScroll, docScroll; + + if (val === undefined) { + doc = Y.config.doc; + bodyScroll = doc.body && doc.body.scrollLeft; + docScroll = doc.documentElement.scrollLeft; + + val = e.clientX + (docScroll || bodyScroll || 0); + } + + return val; + }, + pageY: function () { + var e = this._event, + val = e.pageY, + doc, bodyScroll, docScroll; + + if (val === undefined) { + doc = Y.config.doc; + bodyScroll = doc.body && doc.body.scrollTop; + docScroll = doc.documentElement.scrollTop; + + val = e.clientY + (docScroll || bodyScroll || 0); + } + + return val; + } +}; + + +/** + * Wrapper function for Object.defineProperty that creates a property whose + * value will be calulated only when asked for. After calculating the value, + * the getter wll be removed, so it will behave as a normal property beyond that + * point. A setter is also assigned so assigning to the property will clear + * the getter, so foo.prop = 'a'; foo.prop; won't trigger the getter, + * overwriting value 'a'. + * + * Used only by the DOMEventFacades used by IE8 when the YUI configuration + * lazyEventFacade is set to true. + * + * @method _define + * @param o {DOMObject} A DOM object to add the property to + * @param prop {String} The name of the new property + * @param valueFn {Function} The function that will return the initial, default + * value for the property. + * @static + * @private + */ +IELazyFacade._define = function (o, prop, valueFn) { + function val(v) { + var ret = (arguments.length) ? v : valueFn.call(this); + + delete o[prop]; + Object.defineProperty(o, prop, { + value: ret, + configurable: true, + writable: true + }); + return ret; + } + Object.defineProperty(o, prop, { + get: val, + set: val, + configurable: true + }); +}; + +if (imp && (!imp.hasFeature('Events', '2.0'))) { + if (useLazyFacade) { + // Make sure we can use the lazy facade logic + try { + Object.defineProperty(Y.config.doc.createEventObject(), 'z', {}); + } catch (e) { + useLazyFacade = false; + } + } + + Y.DOMEventFacade = (useLazyFacade) ? IELazyFacade : IEEventFacade; +} diff --git a/tests/assets/badrollup/src/event/js/event-ready-base-ie.js b/tests/assets/badrollup/src/event/js/event-ready-base-ie.js new file mode 100644 index 0000000..6d51c65 --- /dev/null +++ b/tests/assets/badrollup/src/event/js/event-ready-base-ie.js @@ -0,0 +1,40 @@ +(function() { + +var stateChangeListener, + GLOBAL_ENV = YUI.Env, + config = YUI.config, + doc = config.doc, + docElement = doc && doc.documentElement, + EVENT_NAME = 'onreadystatechange', + pollInterval = config.pollInterval || 40; + +if (docElement.doScroll && !GLOBAL_ENV._ieready) { + GLOBAL_ENV._ieready = function() { + GLOBAL_ENV._ready(); + }; + +/*! DOMReady: based on work by: Dean Edwards/John Resig/Matthias Miller/Diego Perini */ +// Internet Explorer: use the doScroll() method on the root element. +// This isolates what appears to be a safe moment to manipulate the +// DOM prior to when the document's readyState suggests it is safe to do so. + if (self !== self.top) { + stateChangeListener = function() { + if (doc.readyState == 'complete') { + GLOBAL_ENV.remove(doc, EVENT_NAME, stateChangeListener); + GLOBAL_ENV.ieready();# + } + }; + GLOBAL_ENV.add(doc, EVENT_NAME, stateChangeListener); + } else { + GLOBAL_ENV._dri = setInterval(function() { + try { + docElement.doScroll('left'); + clearInterval(GLOBAL_ENV._dri); + GLOBAL_ENV._dri = null; + GLOBAL_ENV._ieready(); + } catch (domNotReady) { } + }, pollInterval); + } +} + +})(); diff --git a/tests/assets/badrollup/src/event/meta/event.json b/tests/assets/badrollup/src/event/meta/event.json new file mode 100644 index 0000000..846d4d7 --- /dev/null +++ b/tests/assets/badrollup/src/event/meta/event.json @@ -0,0 +1,110 @@ +{ + "event": { + "use": [ + "event-base", + "event-delegate", + "event-synthetic", + "event-mousewheel", + "event-mouseenter", + "event-key", + "event-focus", + "event-resize", + "event-hover", + "event-outside", + "event-touch", + "event-move", + "event-flick", + "event-valuechange", + "event-tap" + ], + "after": ["node-base"], + "submodules": { + "event-base": { + "after": ["node-base"], + "requires": [ + "event-custom-base" + ] + }, + "event-synthetic": { + "requires": [ + "node-base", + "event-custom-complex" + ] + }, + "event-delegate": { + "requires": [ + "node-base" + ] + }, + "event-focus": { + "requires": [ + "event-synthetic" + ] + }, + "event-key": { + "requires": [ + "event-synthetic" + ] + }, + "event-mouseenter": { + "requires": [ + "event-synthetic" + ] + }, + "event-mousewheel": { + "requires": [ + "node-base" + ] + }, + "event-resize": { + "requires": [ + "node-base", + "event-synthetic" + ] + }, + "event-hover": { + "requires": [ + "event-mouseenter" + ] + }, + "event-outside": { + "requires": [ + "event-synthetic" + ] + }, + "event-tap": { + "requires": [ + "node-base", + "event-base", + "event-touch", + "event-synthetic" + ] + }, + "event-contextmenu": { + "requires": [ + "event-synthetic", + "dom-screen" + ] + } + }, + "plugins": { + "event-touch": { + "requires" : [ + "node-base" + ] + }, + "event-base-ie": { + "requires": [ + "node-base" + ], + "after": [ + "event-base" + ], + "condition": { + "trigger": "node-base", + "test": "ie-base-test.js" + } + } + } + } +} diff --git a/tests/assets/badrollup/src/event/meta/ie-base-test.js b/tests/assets/badrollup/src/event/meta/ie-base-test.js new file mode 100644 index 0000000..a04d429 --- /dev/null +++ b/tests/assets/badrollup/src/event/meta/ie-base-test.js @@ -0,0 +1,4 @@ +function(Y) { + var imp = Y.config.doc && Y.config.doc.implementation; + return (imp && (!imp.hasFeature('Events', '2.0'))); +} From d24b2c168093945085225159cbe479047efb4813 Mon Sep 17 00:00:00 2001 From: James Bunt Date: Wed, 12 Mar 2014 21:51:41 +1300 Subject: [PATCH 4/6] Ensure all fatal errors callback and make all callbacks flow back to the initial call to shifter.init() or shifter.add(). In init/add either call back if there's a supplied callback or process.exit(1) (in the case of an error) if there is no callback. --- .gitignore | 2 + lib/ant.js | 2 +- lib/builder.js | 25 ++++++--- lib/index.js | 47 ++++++++++------- lib/log.js | 12 +---- lib/module.js | 38 ++++++++------ lib/pack.js | 26 ++++++---- lib/walk.js | 6 +-- tests/16-builder-lintfail-calendar.js | 75 +++++++++++++++++++++++++++ tests/6-builder-uglify-calendar.js | 20 ++----- tests/general.js | 37 +++---------- 11 files changed, 179 insertions(+), 111 deletions(-) create mode 100644 tests/16-builder-lintfail-calendar.js diff --git a/.gitignore b/.gitignore index f8b72ee..679f355 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ CVS/ *~ .com.apple.timemachine.supported tests/assets/yql/build/*/* +tests/assets/badmodule/build/*/* +tests/assets/badrollup/build/*/* tests/assets-global tests/assets/freestyle/build coverage diff --git a/lib/ant.js b/lib/ant.js index c652a34..1dfdbc5 100644 --- a/lib/ant.js +++ b/lib/ant.js @@ -137,7 +137,7 @@ exports.process = function (options, callback) { } }); } else { - log.error('no .properties files located, hit the brakes', callback); + callback('no .properties files located, hit the brakes'); } }); diff --git a/lib/builder.js b/lib/builder.js index df03bb2..6902580 100644 --- a/lib/builder.js +++ b/lib/builder.js @@ -67,7 +67,7 @@ var prebuild = function (jobs, options, callback) { child.on('exit', stack.add(function (code) { log.info('build for ' + job + ' complete, downshifting'); if (code) { - log.error('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!', callback); + callback('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!'); } })); }); @@ -92,7 +92,11 @@ exports.start = function (json, options, buildCallback) { post = function (json, callback) { if (json.postbuilds && options.exec) { log.info('found a postbuild, shifting it'); - prebuild(json.postbuilds, options, function () { + prebuild(json.postbuilds, options, function (err) { + if (err) { + return buildCallback(err); + } + delete json.postbuilds; post(json, callback); }); @@ -108,7 +112,11 @@ exports.start = function (json, options, buildCallback) { if (json.prebuilds && options.exec) { log.info('found a prebuild, shifting it'); - prebuild(json.prebuilds, options, function () { + prebuild(json.prebuilds, options, function (err) { + if (err) { + return buildCallback(err); + } + delete json.prebuilds; exports.start(json, options, buildCallback); }); @@ -129,17 +137,22 @@ exports.start = function (json, options, buildCallback) { json2 = JSON.parse(fs.readFileSync(options.buildFile, 'utf8')); } catch (e) { console.log(e.stack); - log.error('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?', buildCallback); + buildCallback('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?'); } if (pack.valid(json2)) { - pack.munge(json2, options, function (json2, options) { + pack.munge(json2, options, function (err, json2, options) { delete json2.exec; delete json2.prebuilds; + + if (err) { + return buildCallback(err); + } + exports.start(json2, options, buildCallback); }); } else { - log.error('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!', buildCallback); + buildCallback('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!'); } } else { exports.start(json, options, buildCallback); diff --git a/lib/index.js b/lib/index.js index b57682e..b8d5973 100644 --- a/lib/index.js +++ b/lib/index.js @@ -47,10 +47,17 @@ var runQueue = function() { } }; +function logAndExit(err) { + if (err) { + log.err(err); + process.exit(1); + } +} + exports.add = function(opts, callback) { queue.push({ opts: opts, - callback: callback + callback: callback || logAndExit }); runQueue(); }; @@ -63,6 +70,10 @@ exports.init = function (opts, initCallback) { log.reset(options); + if (!initCallback) { + initCallback = logAndExit; + } + if (options.cwd) { CWD = options.cwd; } @@ -133,7 +144,7 @@ exports.init = function (opts, initCallback) { var json, walk, ant, mods, builder; if (yes) { if (options.ant) { - log.error('already has a ' + buildFileName + ' file, hitting the brakes', initCallback); + return initCallback('already has a ' + buildFileName + ' file, hitting the brakes'); } log.info('found ' + buildFileName + ' file, shifting'); if (path.extname(buildFileName) === '.json') { @@ -141,11 +152,15 @@ exports.init = function (opts, initCallback) { json = require(buildFile); } catch (e) { console.log(e.stack); - log.error('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?', initCallback); + return initCallback('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?'); } if (pack.valid(json)) { log.info('putting the hammer down, let\'s build this thing!'); - pack.munge(json, options, function (json, options) { + pack.munge(json, options, function (err, json, options) { + if (err) { + return initCallback(err); + } + if (options.list) { mods = Object.keys(json.builds).sort(); log.info('This module includes these builds:'); @@ -159,16 +174,12 @@ exports.init = function (opts, initCallback) { builder.reset(); builder.start(json, options, function(err) { buildRunning = false; - if (initCallback) { - initCallback(err); - } else { - process.exit(err ? 1 : 0); - } + initCallback(err); }); } }); } else { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); + return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); } } else if (path.extname(buildFileName) === '.js') { // probably a row module @@ -185,7 +196,7 @@ exports.init = function (opts, initCallback) { vm.runInContext(fs.readFileSync(buildFile, 'utf8'), contextForRunInContext, buildFile); } catch (e) { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); + return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); } if (mods) { // raw yui module without build.json @@ -195,15 +206,11 @@ exports.init = function (opts, initCallback) { builds: mods }, options, function(err) { buildRunning = false; - if (initCallback) { - initCallback(err); - } else { - process.exit(err ? 1 : 0); - } + initCallback(err); }); } } else { - log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!', initCallback); + return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!'); } } else { if (options.walk) { @@ -212,7 +219,11 @@ exports.init = function (opts, initCallback) { } else { log.warn('no ' + buildFileName + ' file, downshifting to convert ant files'); ant = require('./ant'); - ant.process(options, function () { + ant.process(options, function (err) { + if (err) { + return initCallback(err); + } + if (!options.ant) { exports.init(options, initCallback); } diff --git a/lib/log.js b/lib/log.js index 2dedacb..7406523 100644 --- a/lib/log.js +++ b/lib/log.js @@ -57,23 +57,13 @@ exports.warn = function (str) { } }; -exports.error = function (str, callback) { - if (!silent) { - console.error(prefix, exports.color('[error]', 'red'), str); - } - if (callback) { - callback(str); - } else { - process.exit(1); - } -}; - exports.err = function (str) { if (!silent) { console.error(prefix, exports.color('[err]', 'red'), str); } }; +exports.error = exports.err; exports.console = { log: function() { diff --git a/lib/module.js b/lib/module.js index 746f439..c5b21cb 100644 --- a/lib/module.js +++ b/lib/module.js @@ -41,7 +41,7 @@ var Stack = require('./stack').Stack, coverageType = 'yuitest', compressorFn, compressorConfig, - configCompressor = function (options, cb) { + configCompressor = function (options) { if (options.compressor) { compressorFn = 'compressor'; compressorConfig = { @@ -55,9 +55,8 @@ var Stack = require('./stack').Stack, callback: function (e) { log.err('compression failed'); log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white')); - log.error('dropped the clutch, build failed', function () { - cb(e.message); - }); + log.err('dropped the clutch, build failed'); + // Not calling back here, the jsminify task will callback with an error, failing the queue } }; } @@ -93,7 +92,8 @@ var Stack = require('./stack').Stack, } }); if (lintFail) { - log.error('lint failed, aborting build'); + // Return an error to callback with, which should fail the build + return 'lint failed, aborting build'; } } else { log.info('css lint passed for ' + file); @@ -143,7 +143,8 @@ var Stack = require('./stack').Stack, } }); if (lintFail) { - log.error('lint failed, aborting build'); + // Return an error to callback with, which should fail the build + return 'lint failed, aborting build'; } } } @@ -366,12 +367,16 @@ var buildJS = function (mod, name, callback) { log.warn(name + ': ' + err); } else { if (/ENOENT/.test(err)) { - log.error('Failed to open file: ' + err.path, callback); + callback('Failed to open file: ' + err.path); + } else { + callback(name + ': ' + err); } - log.err(name + ': ' + err); + return; } } - callback(err, result); + + // Either no error or just the 'file not changed' warning, consider that a success + callback(null, result); }); }; @@ -501,7 +506,8 @@ var buildSkin = function (mod, name, callback) { fs.readdir(path.join(shifter.cwd(), 'assets', subMod, 'skins'), stack.add(function (err, skins) { if (err) { log.console.log(err); - log.error('skin files are not right!', callback); + log.err('skin files are not right!'); + return; } //Walk the skins and write them out @@ -543,7 +549,8 @@ var buildSkin = function (mod, name, callback) { if (err) { log.err(err); if (err.code === 'ENOENT') { - log.error('skin file is missing: ' + err.path, callback); + log.err('skin file is missing: ' + err.path); + return; } } @@ -673,7 +680,7 @@ var build = function (mod, name, options, callback) { setJSLint(); } - configCompressor(options, callback); + configCompressor(options); setReplacers(options); cacheBuild = options.cache; @@ -888,9 +895,10 @@ var _rollup = function (mod, name, options, callback) { .write(path.join(mod.buildDir, fileName, fileName + '-min.js')) .run(function (err) { if (err) { - log.error(name + ' rollup: ' + err, callback); + callback(name + ' rollup: ' + err); + } else { + callback(); } - callback(); }); }; @@ -913,7 +921,7 @@ exports.rollup = function (mods, callback) { mod = item.mod; setReplacers(options); - configCompressor(options, callback); + configCompressor(options); if (mod.build) { log.info('found a sub build, down shifting'); diff --git a/lib/pack.js b/lib/pack.js index 8351ab4..794aa5c 100644 --- a/lib/pack.js +++ b/lib/pack.js @@ -55,17 +55,12 @@ exports.munge = function (json, options, callback) { exists(meta, function (yes) { if (yes) { - var files = fs.readdirSync(meta), mod; - files.forEach(function (file) { + var files = fs.readdirSync(meta), mod, i, processFile; + + processFile = function(file) { if (path.extname(file) === '.json') { log.info('munging in loader meta data into build.json'); - try { - mod = flatten(require(path.join(meta, file))); - } catch (e) { - console.log(e.stack); - log.error('hitting the brakes! failed to parse ' + file + ', syntax error?'); - return; - } + mod = flatten(require(path.join(meta, file))); Object.keys(json.builds).forEach(function (name) { if (mod[name]) { json.builds[name].config = mix(mod[name], json.builds[name].config); @@ -79,7 +74,16 @@ exports.munge = function (json, options, callback) { }); } } - }); + }; + + for (i = 0; i < files.length; i += 1) { + try { + processFile(files[i]); + } catch (e) { + console.log(e.stack); + return callback('hitting the brakes! failed to parse ' + files[i] + ', syntax error?'); + } + } } else { log.warn('down shifting, can\'t find a meta directory'); } @@ -97,7 +101,7 @@ exports.munge = function (json, options, callback) { }); } - callback(json, options); + callback(null, json, options); }); }; diff --git a/lib/walk.js b/lib/walk.js index e1eb872..a416e82 100644 --- a/lib/walk.js +++ b/lib/walk.js @@ -37,7 +37,7 @@ exports.run = function (options, callback) { } if (options.progress) { - ProgressBar = require('progress'), + ProgressBar = require('progress'); bar = new ProgressBar(log.color(' shifting [', 'magenta') + log.color(':bar', 'cyan') + log.color(']', 'magenta') + log.color(' :percent :etas', 'yellow'), { @@ -134,7 +134,7 @@ exports.run = function (options, callback) { modStack.done(function () { if (!mods.length) { - log.error('no modules found, hitting the brakes.', callback); + return callback('no modules found, hitting the brakes.'); } if (bar) { bar.total = mods.length - 1; @@ -182,7 +182,7 @@ exports.run = function (options, callback) { errors.forEach(function (mod) { console.log(' ', log.color(mod, 'red')); }); - process.exit(1); + callback('Walk failed, ' + errors.length + ' builds exited with a 1'); } else if (typeof callback === 'function') { callback(); } diff --git a/tests/16-builder-lintfail-calendar.js b/tests/16-builder-lintfail-calendar.js new file mode 100644 index 0000000..769a1c6 --- /dev/null +++ b/tests/16-builder-lintfail-calendar.js @@ -0,0 +1,75 @@ +var vows = require('vows'), + assert = require('assert'), + path = require('path'), + fs = require('fs'), + shifter = require('../lib'), + base = path.join(__dirname, 'assets/yql/'), + buildBase = path.join(base, 'build'), + srcBase = path.join(base, 'src/calendar'), + rimraf = require('rimraf'); + +var tests = { + 'clean build': { + topic: function() { + var self = this; + rimraf(path.join(buildBase, 'calendar-base'), function() { + rimraf(path.join(buildBase, 'calendarnavigator'), function() { + rimraf(path.join(buildBase, 'calendar'), self.callback); + }); + }); + }, + 'should not have build dir and': { + topic: function() { + var self = this; + fs.stat(path.join(buildBase, 'calendar'), function(err) { + self.callback(null, err); + }); + }, + 'should not have build/calendar': function(foo, err) { + assert.isNotNull(err); + assert.equal(err.code, 'ENOENT'); + }, + 'should attempt to build Calendar and': { + topic: function() { + var self = this, + _exit = process.exit; + + process.exit = function(code) { + process.exit = _exit; + self.callback(null, { + code: code + }); + }; + + shifter.add({ + silent: true, + cwd: srcBase, + 'global-config': false, + 'lint-stderr': true, + csslint: false, + fail: true, + 'cache': false, + cssproc: 'http://foobar.com/baz/' + }); // No callback provided to test that process.exit(1) happens on error if no callback + }, + 'should have failed with lint errors': function(topic) { + assert.equal(topic.code, 1); + }, + 'should not create build dir': { + topic: function() { + var self = this; + fs.stat(path.join(buildBase, 'calendar'), function(err) { + self.callback(null, err); + }); + }, + 'should create build/calendar': function(foo, err) { + assert.isNotNull(err); + assert.equal(err.code, 'ENOENT'); + } + } + } + } + } +}; + +vows.describe('building calendar with lint errors and `fail: true`').addBatch(tests).export(module); diff --git a/tests/6-builder-uglify-calendar.js b/tests/6-builder-uglify-calendar.js index 868e441..bf03d43 100644 --- a/tests/6-builder-uglify-calendar.js +++ b/tests/6-builder-uglify-calendar.js @@ -36,13 +36,7 @@ function createTests(buildSkin) { 'should build Calendar and': (function() { var context = { topic: function() { - var self = this, - _exit = process.exit, - code; - - process.exit = function(c) { - code = c; - }; + var self = this; shifter.add({ silent: true, @@ -50,20 +44,14 @@ function createTests(buildSkin) { 'global-config': false, 'lint-stderr': true, csslint: false, - fail: true, + fail: false, 'cache': false, cssproc: 'http://foobar.com/baz/', assets: buildSkin - }, function() { - process.exit = _exit; - self.callback(null, { - code: code - }); + }, function(err) { + self.callback(err); }); }, - 'should have failed with lint errors': function(topic) { - assert.equal(topic.code, 1); - }, 'should create build dir and': { topic: function() { fs.stat(path.join(buildBase, 'calendar'), this.callback); diff --git a/tests/general.js b/tests/general.js index 89876d5..ef78630 100644 --- a/tests/general.js +++ b/tests/general.js @@ -71,8 +71,8 @@ var tests = { spec: true, foo: true } - }, {}, function(json, opts) { - self.callback(null, { + }, {}, function(err, json, opts) { + self.callback(err, { json: json, options: opts }); @@ -88,20 +88,12 @@ var tests = { 'munging the pack, with a BAD shifter config': { topic: function() { var self = this, - _exit = process.exit, cwd = path.join(__dirname, 'assets/badmeta'); shifter.cwd = function() { return cwd; }; - process.exit = function(code) { - process.exit = _exit; - self.callback(null, { - code: code - }); - }; - pack.munge({ "name": "yql", "builds": { @@ -123,11 +115,13 @@ var tests = { spec: true, foo: true } - }, {}, function(json, opts) { + }, {}, function(err, json, opts) { + self.callback(null, err); }); }, - "should exit with code 1": function(topic) { - assert.equal(topic.code, 1); + "should callback with an error": function(topic) { + assert.ok(topic); + assert.equal(topic, 'hitting the brakes! failed to parse bad.json, syntax error?'); } } }, @@ -292,23 +286,6 @@ var tests = { topic.quiet(); topic.console.log('test console.log'); topic.console.error('test console.error'); - }, - 'testing log.error': function(topic) { - var exit = process.exit, - status; - process.exit = function(code) { - status = code; - }; - - topic.error('foobar'); - assert.equal(status, 1); - status = null; - topic.reset(); - topic.silent(); - topic.error('foobar'); - assert.equal(status, 1); - - process.exit = exit; } }, 'general tasks': { From 6f2e135e5cbab1be55695bedee807da094785b59 Mon Sep 17 00:00:00 2001 From: James Bunt Date: Thu, 20 Mar 2014 10:50:04 +1300 Subject: [PATCH 5/6] Improvements as per pull request feedback. --- lib/index.js | 7 +++---- lib/module.js | 29 +++++++++-------------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/lib/index.js b/lib/index.js index b8d5973..26aeb91 100644 --- a/lib/index.js +++ b/lib/index.js @@ -37,11 +37,10 @@ var runQueue = function() { exports.init(item.opts, function(err) { buildRunning = false; if (err) { - item.callback(err); - } else { - item.callback(); - runQueue(); + return item.callback(err); } + item.callback(); + runQueue(); }); } } diff --git a/lib/module.js b/lib/module.js index c5b21cb..9123095 100644 --- a/lib/module.js +++ b/lib/module.js @@ -365,18 +365,14 @@ var buildJS = function (mod, name, callback) { if (err) { if (/file has not changed/.test(err)) { log.warn(name + ': ' + err); + } else if (/ENOENT/.test(err)) { + log.err('Failed to open file: ' + err.path); } else { - if (/ENOENT/.test(err)) { - callback('Failed to open file: ' + err.path); - } else { - callback(name + ': ' + err); - } - return; + log.err(name + ': ' + err); } } - // Either no error or just the 'file not changed' warning, consider that a success - callback(null, result); + callback(err, result); }); }; @@ -666,7 +662,7 @@ var setReplacers = function (options) { }; var build = function (mod, name, options, callback) { - var stack = new Stack(), _build, buildFailed = false; + var stack = new Stack(), _build; if (options.lint === false) { defaultLint = options.lint; @@ -697,7 +693,6 @@ var build = function (mod, name, options, callback) { exports.js(mod, name, stack.add(function (err) { if (err) { log.warn('skipping coverage file build due to previous build error'); - buildFailed = true; } else { if (options.coverage) { exports.coverage(mod, name, stack.add(noop)); @@ -726,7 +721,6 @@ var build = function (mod, name, options, callback) { exports.css(mod, name, stack.add(function (err) { if (err) { log.warn('skipping assets copy due to previous build error'); - buildFailed = true; } else { if (options.assets && mod.assets) { copyAssets(mod, name, stack.add(noop)); @@ -750,14 +744,10 @@ var build = function (mod, name, options, callback) { - stack.done(function () { + stack.done(function (errs) { if (!stack.complete) { stack.complete = true; - if (buildFailed) { - callback('build failed, errors encountered in 1 or more files, see log for more info.'); - } else { - callback(); - } + callback(errs); } }); }; @@ -895,10 +885,9 @@ var _rollup = function (mod, name, options, callback) { .write(path.join(mod.buildDir, fileName, fileName + '-min.js')) .run(function (err) { if (err) { - callback(name + ' rollup: ' + err); - } else { - callback(); + return callback(name + ' rollup: ' + err); } + callback(); }); }; From bfc36cbc6f071386a548f8cbe3a77a1701ead211 Mon Sep 17 00:00:00 2001 From: James Bunt Date: Thu, 20 Mar 2014 10:51:26 +1300 Subject: [PATCH 6/6] Simpler examples for testing fatal syntax errors. --- tests/13-builder-uglify-badmodule.js | 10 +- tests/14-builder-uglify-badmodule-cmd.js | 12 +- tests/15-builder-uglify-event-badrollup.js | 17 +- .../src/{yql/test.json => foo/build.json} | 6 +- tests/assets/badmodule/src/foo/js/foo.js | 2 + tests/assets/badmodule/src/foo/meta/foo.json | 8 + tests/assets/badmodule/src/yql/assets/foo.png | 1 - tests/assets/badmodule/src/yql/build.json | 16 -- tests/assets/badmodule/src/yql/js/yql.js | 193 ------------- tests/assets/badmodule/src/yql/meta/yql.json | 8 - tests/assets/badrollup/src/event/build.json | 30 -- .../src/event/js/event-facade-dom-ie.js | 258 ------------------ .../src/event/js/event-ready-base-ie.js | 40 --- .../badrollup/src/event/meta/event.json | 110 -------- .../badrollup/src/event/meta/ie-base-test.js | 4 - .../assets/badrollup/src/something/build.json | 24 ++ .../assets/badrollup/src/something/js/foo.js | 2 + .../src/something/meta/something.json | 15 + 18 files changed, 73 insertions(+), 683 deletions(-) rename tests/assets/badmodule/src/{yql/test.json => foo/build.json} (54%) create mode 100644 tests/assets/badmodule/src/foo/js/foo.js create mode 100644 tests/assets/badmodule/src/foo/meta/foo.json delete mode 100644 tests/assets/badmodule/src/yql/assets/foo.png delete mode 100644 tests/assets/badmodule/src/yql/build.json delete mode 100644 tests/assets/badmodule/src/yql/js/yql.js delete mode 100644 tests/assets/badmodule/src/yql/meta/yql.json delete mode 100644 tests/assets/badrollup/src/event/build.json delete mode 100644 tests/assets/badrollup/src/event/js/event-facade-dom-ie.js delete mode 100644 tests/assets/badrollup/src/event/js/event-ready-base-ie.js delete mode 100644 tests/assets/badrollup/src/event/meta/event.json delete mode 100644 tests/assets/badrollup/src/event/meta/ie-base-test.js create mode 100644 tests/assets/badrollup/src/something/build.json create mode 100644 tests/assets/badrollup/src/something/js/foo.js create mode 100644 tests/assets/badrollup/src/something/meta/something.json diff --git a/tests/13-builder-uglify-badmodule.js b/tests/13-builder-uglify-badmodule.js index 0c90072..da3ec02 100644 --- a/tests/13-builder-uglify-badmodule.js +++ b/tests/13-builder-uglify-badmodule.js @@ -5,26 +5,26 @@ var vows = require('vows'), shifter = require('../lib'), base = path.join(__dirname, 'assets/badmodule/'), buildBase = path.join(base, 'build'), - srcBase = path.join(base, 'src/yql'), + srcBase = path.join(base, 'src/foo'), rimraf = require('rimraf'); var tests = { 'clean build': { topic: function() { - rimraf(path.join(buildBase, 'yql'), this.callback); + rimraf(path.join(buildBase, 'foo'), this.callback); }, 'should not have build dir and': { topic: function() { var self = this; - fs.stat(path.join(buildBase, 'yql'), function(err) { + fs.stat(path.join(buildBase, 'foo'), function(err) { self.callback(null, err); }); }, - 'should not have build/yql': function(foo, err) { + 'should not have build/foo': function(foo, err) { assert.isNotNull(err); assert.equal(err.code, 'ENOENT'); }, - 'should build yql and': { + 'should build foo and': { topic: function() { var self = this; diff --git a/tests/14-builder-uglify-badmodule-cmd.js b/tests/14-builder-uglify-badmodule-cmd.js index ea93b32..713a078 100644 --- a/tests/14-builder-uglify-badmodule-cmd.js +++ b/tests/14-builder-uglify-badmodule-cmd.js @@ -5,33 +5,33 @@ var vows = require('vows'), exec = require('child_process').exec, base = path.join(__dirname, 'assets/badmodule/'), buildBase = path.join(base, 'build'), - srcBase = path.join(base, 'src/yql'), + srcBase = path.join(base, 'src/foo'), rimraf = require('rimraf'); var tests = { 'clean build': { topic: function() { - rimraf(path.join(buildBase, 'yql'), this.callback); + rimraf(path.join(buildBase, 'foo'), this.callback); }, 'should not have build dir and': { topic: function() { var self = this; - fs.stat(path.join(buildBase, 'yql'), function(err) { + fs.stat(path.join(buildBase, 'foo'), function(err) { self.callback(null, err); }); }, - 'should not have build/yql': function(foo, err) { + 'should not have build/foo': function(foo, err) { assert.isNotNull(err); assert.equal(err.code, 'ENOENT'); }, - 'should build yql and': { + 'should build foo and': { topic: function() { var self = this, child; process.chdir(path.resolve(base, srcBase)); - child = exec('../../../../../bin/shifter --config test.json --no-global-config', function (error, stdout, stderr) { + child = exec('../../../../../bin/shifter --no-global-config', function (error, stdout, stderr) { self.callback(null, { error: error, stderr: stderr diff --git a/tests/15-builder-uglify-event-badrollup.js b/tests/15-builder-uglify-event-badrollup.js index 4faa68c..43c6512 100644 --- a/tests/15-builder-uglify-event-badrollup.js +++ b/tests/15-builder-uglify-event-badrollup.js @@ -5,27 +5,26 @@ var vows = require('vows'), shifter = require('../lib'), base = path.join(__dirname, 'assets/badrollup/'), buildBase = path.join(base, 'build'), - srcBase = path.join(base, 'src/event'), + srcBase = path.join(base, 'src/something'), rimraf = require('rimraf'); - var tests = { 'clean build': { topic: function() { - rimraf(path.join(buildBase, 'event-base-ie'), this.callback); + rimraf(path.join(buildBase, 'foo'), this.callback); }, 'should not have build dir and': { topic: function() { var self = this; - fs.stat(path.join(buildBase, 'event-base-ie'), function(err) { + fs.stat(path.join(buildBase, 'foo'), function(err) { self.callback(null, err); }); }, - 'should not have build/event-base-ie': function(foo, err) { + 'should not have build/foo': function(foo, err) { assert.isNotNull(err); assert.equal(err.code, 'ENOENT'); }, - 'should build Event Base IE and': { + 'should build foo and': { topic: function() { var self = this; @@ -43,9 +42,9 @@ var tests = { }, 'should create build dir and': { topic: function() { - fs.stat(path.join(buildBase, 'event-base-ie'), this.callback); + fs.stat(path.join(buildBase, 'foo'), this.callback); }, - 'should create build/event-base-ie': function(err, stat) { + 'should create build/foo': function(err, stat) { assert.isNull(err); assert.isTrue(stat.isDirectory()); } @@ -55,4 +54,4 @@ var tests = { } }; -vows.describe('building badrollup event with UglifyJS').addBatch(tests).export(module); +vows.describe('building badrollup with UglifyJS').addBatch(tests).export(module); diff --git a/tests/assets/badmodule/src/yql/test.json b/tests/assets/badmodule/src/foo/build.json similarity index 54% rename from tests/assets/badmodule/src/yql/test.json rename to tests/assets/badmodule/src/foo/build.json index 9aa6b08..e696a03 100644 --- a/tests/assets/badmodule/src/yql/test.json +++ b/tests/assets/badmodule/src/foo/build.json @@ -1,9 +1,9 @@ { - "name": "yql23", + "name": "foo", "builds": { - "yql": { + "foo": { "jsfiles": [ - "yql.js" + "foo.js" ] } } diff --git a/tests/assets/badmodule/src/foo/js/foo.js b/tests/assets/badmodule/src/foo/js/foo.js new file mode 100644 index 0000000..76d4dcc --- /dev/null +++ b/tests/assets/badmodule/src/foo/js/foo.js @@ -0,0 +1,2 @@ +var syntaxError = #'foo'; +alert(syntaxError); diff --git a/tests/assets/badmodule/src/foo/meta/foo.json b/tests/assets/badmodule/src/foo/meta/foo.json new file mode 100644 index 0000000..eec2737 --- /dev/null +++ b/tests/assets/badmodule/src/foo/meta/foo.json @@ -0,0 +1,8 @@ +{ + "foo": { + "requires": [ + "bar", + "baz" + ] + } +} diff --git a/tests/assets/badmodule/src/yql/assets/foo.png b/tests/assets/badmodule/src/yql/assets/foo.png deleted file mode 100644 index ccd24a9..0000000 --- a/tests/assets/badmodule/src/yql/assets/foo.png +++ /dev/null @@ -1 +0,0 @@ -// totally an image diff --git a/tests/assets/badmodule/src/yql/build.json b/tests/assets/badmodule/src/yql/build.json deleted file mode 100644 index 4e198b9..0000000 --- a/tests/assets/badmodule/src/yql/build.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "name": "yql", - "builds": { - "yql": { - "assets": false, - "exec": [ - "shifter --config test.json --no-global-config", - "echo 'Foobar'" - ], - "jsfiles": [ - "yql.js" - ], - "cssfiles": [] - } - } -} diff --git a/tests/assets/badmodule/src/yql/js/yql.js b/tests/assets/badmodule/src/yql/js/yql.js deleted file mode 100644 index a2cd232..0000000 --- a/tests/assets/badmodule/src/yql/js/yql.js +++ /dev/null @@ -1,193 +0,0 @@ - -/*! -ENSURE THIS STAYS AT MIN TIME -Copyright 2012 Yahoo! Inc. All rights reserved. -Licensed under the BSD License. -http://yuilibrary.com/license/ -*/ - - /** - * This class adds a sugar class to allow access to YQL (http://developer.yahoo.com/yql/). - * @module yql - */ - /** - * Utility Class used under the hood my the YQL class - * @class YQLRequest - * @constructor - * @param {String} sql The SQL statement to execute - * @param {Function/Object} callback The callback to execute after the query (Falls through to JSONP). - * @param {Object} params An object literal of extra parameters to pass along (optional). - * @param {Object} opts An object literal of configuration options (optional): proto (http|https), base (url) - */ - var YQLRequest = function (sql, callback, params, opts) { - this._types = { - esc: { - token: '\uE000', - re: /\\[:\[\]\(\)#\.\'\>+~"]/gi - }, - - attr: { - token: '\uE001', - re: /(\[[^\]]*\])/g - }, - - pseudo: { - token: '\uE002', - re: /(\([^\)]*\))/g - } - }; - - if (!params) { - params = {}; - // Invalid character - # - } - params.q = sql; - //Allow format override.. JSON-P-X - if (!params.format) { - params.format = Y.YQLRequest.FORMAT; - } - if (!params.env) { - params.env = Y.YQLRequest.ENV; - } - - this._context = this; - - if (opts && opts.context) { - this._context = opts.context; - delete opts.context; - } - - if (params && params.context) { - this._context = params.context; - delete params.context; - } - - this._params = params; - this._opts = opts; - this._callback = callback; - - }; - - YQLRequest.prototype = { - /** - * @private - * @property _jsonp - * @description Reference to the JSONP instance used to make the queries - */ - _jsonp: null, - /** - * @private - * @property _opts - * @description Holder for the opts argument - */ - _opts: null, - /** - * @private - * @property _callback - * @description Holder for the callback argument - */ - _callback: null, - /** - * @private - * @property _params - * @description Holder for the params argument - */ - _params: null, - /** - * @private - * @property _context - * @description The context to execute the callback in - */ - _context: null, - /** - * @private - * @method _internal - * @description Internal Callback Handler - */ - _internal: function() { - this._callback.apply(this._context, arguments); - }, - /** - * @method send - * @description The method that executes the YQL Request. - * @chainable - * @return {YQLRequest} - */ - send: function() { - var qs = [], url = ((this._opts && this._opts.proto) ? this._opts.proto : Y.YQLRequest.PROTO), - o; - - Y.each(this._params, function(v, k) { - qs.push(k + '=' + encodeURIComponent(v)); - }); - - qs = qs.join('&'); - - url += ((this._opts && this._opts.base) ? this._opts.base : Y.YQLRequest.BASE_URL) + qs; - - o = (!Y.Lang.isFunction(this._callback)) ? this._callback : { on: { success: this._callback } }; - - o.on = o.on || {}; - this._callback = o.on.success; - - o.on.success = Y.bind(this._internal, this); - - if (o.allowCache !== false) { - o.allowCache = true; - } - Y.log('URL: ' + url, 'info', 'yql'); - - if (!this._jsonp) { - this._jsonp = Y.jsonp(url, o); - } else { - this._jsonp.url = url; - if (o.on && o.on.success) { - this._jsonp._config.on.success = o.on.success; - } - this._jsonp.send(); - } - return this; - } - }; - - /** - * @static - * @property FORMAT - * @description Default format to use: json - */ - YQLRequest.FORMAT = 'json'; - /** - * @static - * @property PROTO - * @description Default protocol to use: http - */ - YQLRequest.PROTO = 'http'; - /** - * @static - * @property BASE_URL - * @description The base URL to query: query.yahooapis.com/v1/public/yql? - */ - YQLRequest.BASE_URL = ':/'+'/query.yahooapis.com/v1/public/yql?'; - /** - * @static - * @property ENV - * @description The environment file to load: http://datatables.org/alltables.env - */ - YQLRequest.ENV = 'http:/'+'/datatables.org/alltables.env'; - - Y.YQLRequest = YQLRequest; - - /** - * This class adds a sugar class to allow access to YQL (http://developer.yahoo.com/yql/). - * @class YQL - * @constructor - * @param {String} sql The SQL statement to execute - * @param {Function} callback The callback to execute after the query (optional). - * @param {Object} params An object literal of extra parameters to pass along (optional). - * @param {Object} opts An object literal of configuration options (optional): proto (http|https), base (url) - */ - Y.YQL = function(sql, callback, params, opts) { - return new Y.YQLRequest(sql, callback, params, opts).send(); - }; - diff --git a/tests/assets/badmodule/src/yql/meta/yql.json b/tests/assets/badmodule/src/yql/meta/yql.json deleted file mode 100644 index 815e422..0000000 --- a/tests/assets/badmodule/src/yql/meta/yql.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "yql": { - "requires": [ - "jsonp", - "jsonp-url" - ] - } -} diff --git a/tests/assets/badrollup/src/event/build.json b/tests/assets/badrollup/src/event/build.json deleted file mode 100644 index a44ce0b..0000000 --- a/tests/assets/badrollup/src/event/build.json +++ /dev/null @@ -1,30 +0,0 @@ -{ - "name": "event", - "builds": { - "event-base-ie": { - "prependfiles": [ - "js/event-ready-base-ie.js" - ], - "jsfiles": [ - "event-facade-dom-ie.js" - ] - } - }, - "rollups": { - "event-base": { - "files": [ - "event-base-ie" - ], - "build": { - "event-base-ie": { - "prependfiles": [ - "js/event-ready-base-ie.js" - ], - "jsfiles": [ - "event-facade-dom-ie.js" - ] - } - } - } - } -} diff --git a/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js b/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js deleted file mode 100644 index a671646..0000000 --- a/tests/assets/badrollup/src/event/js/event-facade-dom-ie.js +++ /dev/null @@ -1,258 +0,0 @@ -/* - * Custom event engine, DOM event listener abstraction layer, synthetic DOM - * events. - * @module event - * @submodule event-base - */ - -function IEEventFacade() { - // IEEventFacade.superclass.constructor.apply(this, arguments); - Y.DOM2EventFacade.apply(this, arguments); -} - -/* - * (intentially left out of API docs) - * Alternate Facade implementation that is based on Object.defineProperty, which - * is partially supported in IE8. Properties that involve setup work are - * deferred to temporary getters using the static _define method. - */ -function IELazyFacade(e) { - var proxy = Y.config.doc.createEventObject(e), - proto = IELazyFacade.prototype; - - // TODO: necessary? - proxy.hasOwnProperty = function () { return true; }; - - proxy.init = proto.init; - proxy.halt = proto.halt; - proxy.preventDefault = proto.preventDefault; - proxy.stopPropagation = proto.stopPropagation; - proxy.stopImmediatePropagation = proto.stopImmediatePropagation; - - Y.DOM2EventFacade.apply(proxy, arguments); - - return proxy; -} - - -var imp = Y.config.doc && Y.config.doc.implementation, - useLazyFacade = Y.config.lazyEventFacade, - - buttonMap = { - 0: 1, // left click - 4: 2, // middle click - 2: 3 // right click - }, - relatedTargetMap = { - mouseout: 'toElement', - mouseover: 'fromElement' - }, - - resolve = Y.DOM2EventFacade.resolve, - - proto = { - init: function() { - - IEEventFacade.superclass.init.apply(this, arguments); - - var e = this._event, - x, y, d, b, de, t; - - this.target = resolve(e.srcElement); - - if (('clientX' in e) && (!x) && (0 !== x)) { - x = e.clientX; - y = e.clientY; - - d = Y.config.doc; - b = d.body; - de = d.documentElement; - - x += (de.scrollLeft || (b && b.scrollLeft) || 0); - y += (de.scrollTop || (b && b.scrollTop) || 0); - - this.pageX = x; - this.pageY = y; - } - - if (e.type == "mouseout") { - t = e.toElement; - } else if (e.type == "mouseover") { - t = e.fromElement; - } - - // fallback to t.relatedTarget to support simulated events. - // IE doesn't support setting toElement or fromElement on generic - // events, so Y.Event.simulate sets relatedTarget instead. - this.relatedTarget = resolve(t || e.relatedTarget); - - // which should contain the unicode key code if this is a key event. - // For click events, which is normalized for which mouse button was - // clicked. - this.which = // chained assignment - this.button = e.keyCode || buttonMap[e.button] || e.button; - }, - - stopPropagation: function() { - this._event.cancelBubble = true; - this._wrapper.stopped = 1; - this.stopped = 1; - }, - - stopImmediatePropagation: function() { - this.stopPropagation(); - this._wrapper.stopped = 2; - this.stopped = 2; - }, - - preventDefault: function(returnValue) { - this._event.returnValue = returnValue || false; - this._wrapper.prevented = 1; - this.prevented = 1; - } - }; - -Y.extend(IEEventFacade, Y.DOM2EventFacade, proto); - -Y.extend(IELazyFacade, Y.DOM2EventFacade, proto); -IELazyFacade.prototype.init = function () { - var e = this._event, - overrides = this._wrapper.overrides, - define = IELazyFacade._define, - lazyProperties = IELazyFacade._lazyProperties, - prop; - - this.altKey = e.altKey; - this.ctrlKey = e.ctrlKey; - this.metaKey = e.metaKey; - this.shiftKey = e.shiftKey; - this.type = (overrides && overrides.type) || e.type; - this.clientX = e.clientX; - this.clientY = e.clientY; - this.keyCode = // chained assignment - this.charCode = e.keyCode; - this.which = // chained assignment - this.button = e.keyCode || buttonMap[e.button] || e.button; - - for (prop in lazyProperties) { - if (lazyProperties.hasOwnProperty(prop)) { - define(this, prop, lazyProperties[prop]); - } - } - - if (this._touch) { - this._touch(e, this._currentTarget, this._wrapper); - } -}; - -IELazyFacade._lazyProperties = { - target: function () { - return resolve(this._event.srcElement); - }, - relatedTarget: function () { - var e = this._event, - targetProp = relatedTargetMap[e.type] || 'relatedTarget'; - - // fallback to t.relatedTarget to support simulated events. - // IE doesn't support setting toElement or fromElement on generic - // events, so Y.Event.simulate sets relatedTarget instead. - return resolve(e[targetProp] || e.relatedTarget); - }, - currentTarget: function () { - return resolve(this._currentTarget); - }, - - wheelDelta: function () { - var e = this._event; - - if (e.type === "mousewheel" || e.type === "DOMMouseScroll") { - return (e.detail) ? - (e.detail * -1) : - // wheelDelta between -80 and 80 result in -1 or 1 - Math.round(e.wheelDelta / 80) || ((e.wheelDelta < 0) ? -1 : 1); - } - }, - - pageX: function () { - var e = this._event, - val = e.pageX, - doc, bodyScroll, docScroll; - - if (val === undefined) { - doc = Y.config.doc; - bodyScroll = doc.body && doc.body.scrollLeft; - docScroll = doc.documentElement.scrollLeft; - - val = e.clientX + (docScroll || bodyScroll || 0); - } - - return val; - }, - pageY: function () { - var e = this._event, - val = e.pageY, - doc, bodyScroll, docScroll; - - if (val === undefined) { - doc = Y.config.doc; - bodyScroll = doc.body && doc.body.scrollTop; - docScroll = doc.documentElement.scrollTop; - - val = e.clientY + (docScroll || bodyScroll || 0); - } - - return val; - } -}; - - -/** - * Wrapper function for Object.defineProperty that creates a property whose - * value will be calulated only when asked for. After calculating the value, - * the getter wll be removed, so it will behave as a normal property beyond that - * point. A setter is also assigned so assigning to the property will clear - * the getter, so foo.prop = 'a'; foo.prop; won't trigger the getter, - * overwriting value 'a'. - * - * Used only by the DOMEventFacades used by IE8 when the YUI configuration - * lazyEventFacade is set to true. - * - * @method _define - * @param o {DOMObject} A DOM object to add the property to - * @param prop {String} The name of the new property - * @param valueFn {Function} The function that will return the initial, default - * value for the property. - * @static - * @private - */ -IELazyFacade._define = function (o, prop, valueFn) { - function val(v) { - var ret = (arguments.length) ? v : valueFn.call(this); - - delete o[prop]; - Object.defineProperty(o, prop, { - value: ret, - configurable: true, - writable: true - }); - return ret; - } - Object.defineProperty(o, prop, { - get: val, - set: val, - configurable: true - }); -}; - -if (imp && (!imp.hasFeature('Events', '2.0'))) { - if (useLazyFacade) { - // Make sure we can use the lazy facade logic - try { - Object.defineProperty(Y.config.doc.createEventObject(), 'z', {}); - } catch (e) { - useLazyFacade = false; - } - } - - Y.DOMEventFacade = (useLazyFacade) ? IELazyFacade : IEEventFacade; -} diff --git a/tests/assets/badrollup/src/event/js/event-ready-base-ie.js b/tests/assets/badrollup/src/event/js/event-ready-base-ie.js deleted file mode 100644 index 6d51c65..0000000 --- a/tests/assets/badrollup/src/event/js/event-ready-base-ie.js +++ /dev/null @@ -1,40 +0,0 @@ -(function() { - -var stateChangeListener, - GLOBAL_ENV = YUI.Env, - config = YUI.config, - doc = config.doc, - docElement = doc && doc.documentElement, - EVENT_NAME = 'onreadystatechange', - pollInterval = config.pollInterval || 40; - -if (docElement.doScroll && !GLOBAL_ENV._ieready) { - GLOBAL_ENV._ieready = function() { - GLOBAL_ENV._ready(); - }; - -/*! DOMReady: based on work by: Dean Edwards/John Resig/Matthias Miller/Diego Perini */ -// Internet Explorer: use the doScroll() method on the root element. -// This isolates what appears to be a safe moment to manipulate the -// DOM prior to when the document's readyState suggests it is safe to do so. - if (self !== self.top) { - stateChangeListener = function() { - if (doc.readyState == 'complete') { - GLOBAL_ENV.remove(doc, EVENT_NAME, stateChangeListener); - GLOBAL_ENV.ieready();# - } - }; - GLOBAL_ENV.add(doc, EVENT_NAME, stateChangeListener); - } else { - GLOBAL_ENV._dri = setInterval(function() { - try { - docElement.doScroll('left'); - clearInterval(GLOBAL_ENV._dri); - GLOBAL_ENV._dri = null; - GLOBAL_ENV._ieready(); - } catch (domNotReady) { } - }, pollInterval); - } -} - -})(); diff --git a/tests/assets/badrollup/src/event/meta/event.json b/tests/assets/badrollup/src/event/meta/event.json deleted file mode 100644 index 846d4d7..0000000 --- a/tests/assets/badrollup/src/event/meta/event.json +++ /dev/null @@ -1,110 +0,0 @@ -{ - "event": { - "use": [ - "event-base", - "event-delegate", - "event-synthetic", - "event-mousewheel", - "event-mouseenter", - "event-key", - "event-focus", - "event-resize", - "event-hover", - "event-outside", - "event-touch", - "event-move", - "event-flick", - "event-valuechange", - "event-tap" - ], - "after": ["node-base"], - "submodules": { - "event-base": { - "after": ["node-base"], - "requires": [ - "event-custom-base" - ] - }, - "event-synthetic": { - "requires": [ - "node-base", - "event-custom-complex" - ] - }, - "event-delegate": { - "requires": [ - "node-base" - ] - }, - "event-focus": { - "requires": [ - "event-synthetic" - ] - }, - "event-key": { - "requires": [ - "event-synthetic" - ] - }, - "event-mouseenter": { - "requires": [ - "event-synthetic" - ] - }, - "event-mousewheel": { - "requires": [ - "node-base" - ] - }, - "event-resize": { - "requires": [ - "node-base", - "event-synthetic" - ] - }, - "event-hover": { - "requires": [ - "event-mouseenter" - ] - }, - "event-outside": { - "requires": [ - "event-synthetic" - ] - }, - "event-tap": { - "requires": [ - "node-base", - "event-base", - "event-touch", - "event-synthetic" - ] - }, - "event-contextmenu": { - "requires": [ - "event-synthetic", - "dom-screen" - ] - } - }, - "plugins": { - "event-touch": { - "requires" : [ - "node-base" - ] - }, - "event-base-ie": { - "requires": [ - "node-base" - ], - "after": [ - "event-base" - ], - "condition": { - "trigger": "node-base", - "test": "ie-base-test.js" - } - } - } - } -} diff --git a/tests/assets/badrollup/src/event/meta/ie-base-test.js b/tests/assets/badrollup/src/event/meta/ie-base-test.js deleted file mode 100644 index a04d429..0000000 --- a/tests/assets/badrollup/src/event/meta/ie-base-test.js +++ /dev/null @@ -1,4 +0,0 @@ -function(Y) { - var imp = Y.config.doc && Y.config.doc.implementation; - return (imp && (!imp.hasFeature('Events', '2.0'))); -} diff --git a/tests/assets/badrollup/src/something/build.json b/tests/assets/badrollup/src/something/build.json new file mode 100644 index 0000000..9c09fca --- /dev/null +++ b/tests/assets/badrollup/src/something/build.json @@ -0,0 +1,24 @@ +{ + "name": "something", + "builds": { + "foo": { + "jsfiles": [ + "foo.js" + ] + } + }, + "rollups": { + "something-foo": { + "files": [ + "foo" + ], + "build": { + "foo": { + "jsfiles": [ + "foo.js" + ] + } + } + } + } +} diff --git a/tests/assets/badrollup/src/something/js/foo.js b/tests/assets/badrollup/src/something/js/foo.js new file mode 100644 index 0000000..76d4dcc --- /dev/null +++ b/tests/assets/badrollup/src/something/js/foo.js @@ -0,0 +1,2 @@ +var syntaxError = #'foo'; +alert(syntaxError); diff --git a/tests/assets/badrollup/src/something/meta/something.json b/tests/assets/badrollup/src/something/meta/something.json new file mode 100644 index 0000000..aad23ea --- /dev/null +++ b/tests/assets/badrollup/src/something/meta/something.json @@ -0,0 +1,15 @@ +{ + "something": { + "use": [ + "something-foo" + ], + "submodules": { + "something-foo": { + "requires": [ + "bar", + "baz" + ] + } + } + } +}