From e2aff71bf90a0bb7787b5f568a32c17ebc51fb0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 13 Nov 2014 15:31:35 +0100 Subject: [PATCH 1/3] Use `chai` instead of `must` As of 1.10, chai supports nonary assertions, there are no more reasons for using a different variant. --- package.json | 5 ++--- test/browser.test.js | 4 ++-- test/compiler.test.js | 12 ++++++------ test/executor.test.js | 2 +- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 7eba26f..e5c8750 100644 --- a/package.json +++ b/package.json @@ -32,15 +32,14 @@ "underscore": "^1.6.0" }, "devDependencies": { - "browserify": "^6.1.0", - "fs-extra": "^0.12.0", "browserify": "^4.1.8", + "chai": "^1.10.0", "coffee-script": "^1.8.0", "coffeeify": "^0.7.0", + "fs-extra": "^0.12.0", "jshint": "^2.5.6", "loopback": "^2.5.0", "mocha": "^1.19.0", - "must": "^0.12.0", "supertest": "^0.14.0" } } diff --git a/test/browser.test.js b/test/browser.test.js index 985c26f..01b3132 100644 --- a/test/browser.test.js +++ b/test/browser.test.js @@ -1,7 +1,7 @@ var boot = require('../'); var fs = require('fs'); var path = require('path'); -var expect = require('must'); +var expect = require('chai').expect; var browserify = require('browserify'); var sandbox = require('./helpers/sandbox'); var vm = require('vm'); @@ -91,7 +91,7 @@ function browserifyTestApp(appDir, strategy, next) { var bundlePath = sandbox.resolve('browser-app-bundle.js'); var out = fs.createWriteStream(bundlePath); b.bundle().pipe(out); - + out.on('error', function(err) { return next(err); }); out.on('close', function() { next(null, bundlePath); diff --git a/test/compiler.test.js b/test/compiler.test.js index ba362bd..2bf2e0a 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -1,7 +1,7 @@ var boot = require('../'); var fs = require('fs-extra'); var path = require('path'); -var expect = require('must'); +var expect = require('chai').expect; var sandbox = require('./helpers/sandbox'); var appdir = require('./helpers/appdir'); @@ -379,7 +379,7 @@ describe('compiler', function() { var instructions = boot.compile(appdir.PATH); expect(instructions.files.boot).to.eql([initJs]); }); - + it('supports `bootDirs` option', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('custom-boot/init.js', @@ -390,7 +390,7 @@ describe('compiler', function() { }); expect(instructions.files.boot).to.eql([initJs]); }); - + it('supports `bootScripts` option', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('custom-boot/init.js', @@ -475,19 +475,19 @@ describe('compiler', function() { sourceFile: path.resolve(appdir.PATH, 'models', 'car.coffee') }); }); - + it('supports `modelSources` option', function() { appdir.createConfigFilesSync({}, {}, { Car: { dataSource: 'db' } }); appdir.writeConfigFileSync('custom-models/car.json', { name: 'Car' }); appdir.writeFileSync('custom-models/car.js', ''); - + var instructions = boot.compile({ appRootDir: appdir.PATH, modelSources: ['./custom-models'] }); - + expect(instructions.models).to.have.length(1); expect(instructions.models[0]).to.eql({ name: 'Car', diff --git a/test/executor.test.js b/test/executor.test.js index c4f248c..d1d6daf 100644 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -2,7 +2,7 @@ var boot = require('../'); var path = require('path'); var loopback = require('loopback'); var assert = require('assert'); -var expect = require('must'); +var expect = require('chai').expect; var fs = require('fs-extra'); var sandbox = require('./helpers/sandbox'); var appdir = require('./helpers/appdir'); From 83723379a232a803355436cf97b12ac330b1d792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 13 Nov 2014 15:33:23 +0100 Subject: [PATCH 2/3] Clean up .jshintrc jshint does not support `trailing` as of v1.5.0. Remove `maxlen` too, jscs provides better implementation. Fix definition of global mocha variables - mark them as immutable. --- .jshintrc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/.jshintrc b/.jshintrc index 3f23a40..9260a27 100644 --- a/.jshintrc +++ b/.jshintrc @@ -7,18 +7,16 @@ "undef": true, "unused": true, "quotmark": "single", -"maxlen": 80, -"trailing": true, "newcap": true, "nonew": true, "sub": true, "unused": "vars", "globals": { - "describe": true, - "it": true, - "before": true, - "beforeEach": true, - "after": true, - "afterEach": true + "describe": false, + "it": false, + "before": false, + "beforeEach": false, + "after": false, + "afterEach": false } } From 5b5071864ba1686cde245a707f1d35848bd6ac86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 13 Nov 2014 15:54:59 +0100 Subject: [PATCH 3/3] Add jscs style check, fix violations found --- .jscsrc | 24 ++++++++++++++++++++++++ .jshintrc | 1 - index.js | 2 +- lib/compiler.js | 12 +++++++----- lib/config-loader.js | 12 ++++++------ lib/executor.js | 22 +++++++++++----------- package.json | 3 ++- test/browser.test.js | 2 +- test/compiler.test.js | 9 ++++++--- test/executor.test.js | 9 ++++----- test/fixtures/simple-app/boot/barSync.js | 1 - test/fixtures/simple-app/boot/foo.js | 2 +- 12 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 .jscsrc diff --git a/.jscsrc b/.jscsrc new file mode 100644 index 0000000..30b1a58 --- /dev/null +++ b/.jscsrc @@ -0,0 +1,24 @@ +{ + "preset": "google", + "requireCurlyBraces": [ + "do", + "try", + "catch" + ], + "disallowSpacesInsideObjectBrackets": null, + "requireSpaceAfterLineComment": true, + "maximumLineLength": { + "value": 80, + "allowRegex": true + }, + "validateJSDoc": { + "checkParamNames": false, + "checkRedundantParams": false, + "requireParamTypes": true + }, + "excludeFiles": [ + "node_modules/**", + "coverage/**", + "test/sandbox/**" + ] +} diff --git a/.jshintrc b/.jshintrc index 9260a27..6f9fcc5 100644 --- a/.jshintrc +++ b/.jshintrc @@ -1,7 +1,6 @@ { "node": true, "browser": true, -"camelcase" : true, "eqnull" : true, "indent": 2, "undef": true, diff --git a/index.js b/index.js index 4894277..7e17282 100644 --- a/index.js +++ b/index.js @@ -136,7 +136,7 @@ exports.compileToBrowserify = function(options, bundler) { addInstructionsToBrowserify(compile(options), bundler); }; -//-- undocumented low-level API --// +/*-- undocumented low-level API --*/ exports.ConfigLoader = ConfigLoader; exports.compile = compile; diff --git a/lib/compiler.js b/lib/compiler.js index 11f3a3b..2cef7d0 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -22,7 +22,7 @@ var Module = require('module'); module.exports = function compile(options) { options = options || {}; - if(typeof options === 'string') { + if (typeof options === 'string') { options = { appRootDir: options }; } @@ -74,7 +74,7 @@ module.exports = function compile(options) { }; function assertIsValidConfig(name, config) { - if(config) { + if (config) { assert(typeof config === 'object', name + ' config must be a valid JSON object'); } @@ -143,7 +143,7 @@ function findScripts(dir) { } else { try { path.join(require.resolve(filepath)); - } catch(err) { + } catch (err) { debug('Skipping directory %s - %s', filepath, err.code || err); } } @@ -155,7 +155,7 @@ function findScripts(dir) { function tryReadDir() { try { return fs.readdirSync.apply(fs, arguments); - } catch(e) { + } catch (e) { return []; } } @@ -309,7 +309,9 @@ function loadModelDefinition(rootDir, jsonFile, allFiles) { var basename = path.basename(jsonFile, path.extname(jsonFile)); // find a matching file with a supported extension like `.js` or `.coffee` - var base, ext, validFileType; + var base; + var ext; + var validFileType; var sourceFile = allFiles .filter(function(f) { ext = path.extname(f); diff --git a/lib/config-loader.js b/lib/config-loader.js index 17834eb..b8a6dfe 100644 --- a/lib/config-loader.js +++ b/lib/config-loader.js @@ -103,7 +103,7 @@ function loadConfigFiles(files) { */ function mergeConfigurations(configObjects, mergeFn) { var result = configObjects.shift() || {}; - while(configObjects.length) { + while (configObjects.length) { var next = configObjects.shift(); mergeFn(result, next, next._filename); } @@ -163,7 +163,7 @@ function mergeArrays(target, config, keyPrefix) { } // Use for(;;) to iterate over undefined items, for(in) would skip them. - for (var ix=0; ix < target.length; ix++) { + for (var ix = 0; ix < target.length; ix++) { var fullKey = keyPrefix + '[' + ix + ']'; var err = mergeSingleItemOrProperty(target, config, ix, fullKey); if (err) return err; @@ -189,15 +189,15 @@ function hasCompatibleType(origValue, newValue) { /** * Try to read a config file with .json extension - * @param cwd Dirname of the file - * @param fileName Name of the file without extension + * @param {string} cwd Dirname of the file + * @param {string} fileName Name of the file without extension * @returns {Object|undefined} Content of the file, undefined if not found. */ function tryReadJsonConfig(cwd, fileName) { try { return require(path.join(cwd, fileName + '.json')); - } catch(e) { - if(e.code !== 'MODULE_NOT_FOUND') { + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') { throw e; } } diff --git a/lib/executor.js b/lib/executor.js index 16ca218..768b13d 100644 --- a/lib/executor.js +++ b/lib/executor.js @@ -45,10 +45,10 @@ function patchAppLoopback(app) { // patch the app object to make loopback-boot work with older versions too try { app.loopback = require('loopback'); - } catch(err) { + } catch (err) { if (err.code === 'MODULE_NOT_FOUND') { console.error( - 'When using loopback-boot with loopback <1.9, '+ + 'When using loopback-boot with loopback <1.9, ' + 'the loopback module must be available for `require(\'loopback\')`.'); } throw err; @@ -69,7 +69,7 @@ function assertLoopBackVersion(app) { } function setHost(app, instructions) { - //jshint camelcase:false + // jscs:disable requireCamelCaseOrUpperCaseIdentifiers var host = process.env.npm_config_host || process.env.OPENSHIFT_SLS_IP || @@ -79,14 +79,14 @@ function setHost(app, instructions) { process.env.npm_package_config_host || app.get('host'); - if(host !== undefined) { + if (host !== undefined) { assert(typeof host === 'string', 'app.host must be a string'); app.set('host', host); } } function setPort(app, instructions) { - //jshint camelcase:false + // jscs:disable requireCamelCaseOrUpperCaseIdentifiers var port = _.find([ process.env.npm_config_port, process.env.OPENSHIFT_SLS_PORT, @@ -98,7 +98,7 @@ function setPort(app, instructions) { 3000 ], _.isFinite); - if(port !== undefined) { + if (port !== undefined) { var portType = typeof port; assert(portType === 'string' || portType === 'number', 'app.port must be a string or number'); @@ -122,9 +122,9 @@ function setApiRoot(app, instructions) { function applyAppConfig(app, instructions) { var appConfig = instructions.config; - for(var configKey in appConfig) { + for (var configKey in appConfig) { var cur = app.get(configKey); - if(cur === undefined || cur === null) { + if (cur === undefined || cur === null) { app.set(configKey, appConfig[configKey]); } } @@ -198,7 +198,7 @@ function isBuiltinLoopBackModel(app, data) { } function forEachKeyedObject(obj, fn) { - if(typeof obj !== 'object') return; + if (typeof obj !== 'object') return; Object.keys(obj).forEach(function(key) { fn(key, obj[key]); @@ -240,8 +240,8 @@ function runScripts(app, list, callback) { function tryRequire(modulePath) { try { return require.apply(this, arguments); - } catch(e) { - if(e.code === 'MODULE_NOT_FOUND') { + } catch (e) { + if (e.code === 'MODULE_NOT_FOUND') { debug('Warning: cannot require %s - module not found.', modulePath); return undefined; } diff --git a/package.json b/package.json index e5c8750..cd69585 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "main": "index.js", "browser": "browser.js", "scripts": { - "pretest": "jshint .", + "pretest": "jscs . && jshint .", "test": "mocha" }, "license": { @@ -37,6 +37,7 @@ "coffee-script": "^1.8.0", "coffeeify": "^0.7.0", "fs-extra": "^0.12.0", + "jscs": "^1.7.3", "jshint": "^2.5.6", "loopback": "^2.5.0", "mocha": "^1.19.0", diff --git a/test/browser.test.js b/test/browser.test.js index 01b3132..3acd166 100644 --- a/test/browser.test.js +++ b/test/browser.test.js @@ -76,7 +76,7 @@ describe('browser support', function() { }); function browserifyTestApp(appDir, strategy, next) { - //set default args + // set default args if (((typeof strategy) === 'function') && !next) { next = strategy; strategy = undefined; diff --git a/test/compiler.test.js b/test/compiler.test.js index 2bf2e0a..5df2ed5 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -12,7 +12,10 @@ describe('compiler', function() { beforeEach(appdir.init); describe('from options', function() { - var options, instructions, appConfig; + var options; + var instructions; + var appConfig; + beforeEach(function() { options = { config: { @@ -256,7 +259,7 @@ describe('compiler', function() { appdir.writeConfigFileSync('config.local.json', { toplevel: [ { - nested: [ 'value' ] + nested: ['value'] } ] }); @@ -337,7 +340,7 @@ describe('compiler', function() { appConfigRootDir: path.resolve(appdir.PATH, 'custom') }); - expect(instructions.config).to.have.property('port'); + expect(instructions.config).to.have.property('port'); }); it('supports `dsRootDir` option', function() { diff --git a/test/executor.test.js b/test/executor.test.js index d1d6daf..95ed9f4 100644 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -171,7 +171,7 @@ describe('executor', function() { }; builtinModel.definition.redefined = true; - boot.execute(app, someInstructions({ models: [ builtinModel ] })); + boot.execute(app, someInstructions({ models: [builtinModel] })); expect(app.models.User.settings.redefined, 'redefined').to.not.equal(true); }); @@ -266,7 +266,7 @@ describe('executor', function() { }); it('should prioritize sources', function() { - /*jshint camelcase:false */ + // jscs:disable requireCamelCaseOrUpperCaseIdentifiers process.env.npm_config_host = randomHost(); process.env.OPENSHIFT_SLS_IP = randomHost(); process.env.OPENSHIFT_NODEJS_IP = randomHost(); @@ -323,12 +323,11 @@ describe('executor', function() { 'module.exports = function(app) { app.fnCalled = true; };'); delete app.fnCalled; - boot.execute(app, someInstructions({ files: { boot: [ file ] } })); + boot.execute(app, someInstructions({ files: { boot: [file] } })); expect(app.fnCalled, 'exported fn was called').to.be.true(); }); }); - function assertValidDataSource(dataSource) { // has methods assert.isFunc(dataSource, 'createModel'); @@ -340,7 +339,7 @@ function assertValidDataSource(dataSource) { assert.isFunc(dataSource, 'operations'); } -assert.isFunc = function (obj, name) { +assert.isFunc = function(obj, name) { assert(obj, 'cannot assert function ' + name + ' on object that does not exist'); assert(typeof obj[name] === 'function', name + ' is not a function'); diff --git a/test/fixtures/simple-app/boot/barSync.js b/test/fixtures/simple-app/boot/barSync.js index 64c0b0b..4a38f80 100644 --- a/test/fixtures/simple-app/boot/barSync.js +++ b/test/fixtures/simple-app/boot/barSync.js @@ -2,4 +2,3 @@ process.bootFlags.push('barSyncLoaded'); module.exports = function(app) { process.bootFlags.push('barSyncExecuted'); }; - diff --git a/test/fixtures/simple-app/boot/foo.js b/test/fixtures/simple-app/boot/foo.js index 6641f03..e2fd7a5 100644 --- a/test/fixtures/simple-app/boot/foo.js +++ b/test/fixtures/simple-app/boot/foo.js @@ -1 +1 @@ -process.bootFlags.push('fooLoaded'); \ No newline at end of file +process.bootFlags.push('fooLoaded');