From 7b4d445620b7b77ed9169098c016ca2c4c7e40f2 Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Wed, 1 Jun 2016 14:19:44 -0400 Subject: [PATCH 1/2] config options refactor --- lib/config.js | 90 ++++++++++++++++++++++++++-------------------- test/app_config.js | 16 ++++++++- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/lib/config.js b/lib/config.js index 7eea2b2..69ed318 100644 --- a/lib/config.js +++ b/lib/config.js @@ -6,11 +6,11 @@ const StaticPlugin = require('./plugins/static_plugin') const FsPlugin = require('./plugins/fs_plugin') const micromatch = require('micromatch') const union = require('lodash.union') +const merge = require('lodash.merge') const postcssImport = require('postcss-import') const BrowserSyncPlugin = require('browser-sync-webpack-plugin') const {accessSync} = require('fs') const hygienist = require('hygienist-middleware') -const merge = require('lodash.merge') const SpikeUtils = require('spike-util') /** @@ -55,7 +55,7 @@ module.exports = class Config { stringifier: Joi.object(), syntax: Joi.object() }), - babel: Joi.object().default({}), + babel: Joi.object(), cleanUrls: Joi.bool().default(true), jade: Joi.object().default({}), dumpDirs: Joi.array().default(['views', 'assets']), @@ -99,7 +99,26 @@ module.exports = class Config { // string or array), then push ['node_modules', '.git', outputDir] to make // sure they're not watched res.server.watchOptions.ignored = Array.prototype.concat(res.server.watchOptions.ignored) - res.server.watchOptions.ignored = union(res.server.watchOptions.ignored, ['node_modules', '.git', res.outputDir]) + res.server.watchOptions.ignored = union(res.server.watchOptions.ignored, [ + 'node_modules', + '.git', + res.outputDir + ]) + + // core ignores + res.ignore.unshift( + 'node_modules/**', // node_modules folder + `${res.outputDir}/**`, // anything in the public folder + '.git/**', // any git content + 'package.json', // the primary package.json file + '**/.DS_Store', // any dumb DS Store file + 'app.js', // primary config + 'app.*.js' // any environment config + ) + + // Loader excludes use absolute paths for some reason, so we add the context + // to the beginning of the path so users can input them as relative to root. + res.ignore = res.ignore.map((i) => path.join(res.root, i)) // Here we set up the matchers that will watch for newly added files to the // project. @@ -143,7 +162,23 @@ module.exports = class Config { * @return {Class} returns self, but with the properties of a webpack config */ transformSpikeOptionsToWebpack (opts) { - this.entry = opts.entry + // `disallow` options would break spike if modified. + const disallow = ['output', 'resolveLoader', 'spike', 'plugins', 'context', 'jade', 'postcss'] + + // `noCopy` options are spike-specific and shouldn't be directly added to + // webpack's config + const noCopy = ['root', 'matchers', 'env', 'locals', 'server', 'cleanUrls', 'dumpDirs', 'ignore', 'vendor', 'outputDir'] + + // All options other than `disallow` or `noCopy` are added directly to + // webpack's config object + const filteredOpts = removeKeys(opts, disallow.concat(noCopy)) + Object.assign(this, filteredOpts) + + // `noCopy` options are added under the `spike` property + this.spike = { files: {} } + Object.assign(this.spike, filterKeys(opts, noCopy)) + + // Now we run some spike-specific config transforms this.context = opts.root this.output = { @@ -159,22 +194,7 @@ module.exports = class Config { ] } - // core ignores - opts.ignore.unshift( - 'node_modules/**', // node_modules folder - `${opts.outputDir}/**`, // anything in the public folder - '.git/**', // any git content - 'package.json', // the primary package.json file - '**/.DS_Store', // any dumb DS Store file - 'app.js', // primary config - 'app.*.js' // any environment config - ) - - // Loader excludes use absolute paths for some reason, so we add the context - // to the beginning of the path so users can input them as relative to root. - opts.ignore = opts.ignore.map((i) => path.join(this.context, i)) const reIgnores = opts.ignore.map(mmToRe) - const spikeLoaders = [ { exclude: reIgnores, @@ -195,7 +215,6 @@ module.exports = class Config { } ] - this.module = opts.module this.module.loaders = opts.module.loaders.concat(spikeLoaders) this.postcss = function (wp) { @@ -208,25 +227,8 @@ module.exports = class Config { pretty: true }, opts.jade) - this.modulesDirectories = opts.modulesDirectories - - // for spike-specific shared config and utilities - this.spike = { - files: {}, - env: opts.env, - locals: opts.locals, - server: opts.server, - dumpDirs: opts.dumpDirs, - ignore: opts.ignore, - vendor: opts.vendor, - matchers: opts.matchers - } - - this.babel = opts.babel - const util = new SpikeUtils(this) - // TODO: revisit this before a stable launch this.plugins = [new FsPlugin(util)] .concat(opts.plugins) .concat([ @@ -240,8 +242,6 @@ module.exports = class Config { } }) ]) - if (this.resolve) this.resolve = opts.resolve - return this } @@ -279,3 +279,15 @@ function loadFile (f) { function mmToRe (mm) { return micromatch.makeRe(mm) } + +function removeKeys (obj, keys) { + const res = {} + for (const k in obj) { if (keys.indexOf(k) < 0) res[k] = obj[k] } + return res +} + +function filterKeys (obj, keys) { + const res = {} + for (const k in obj) { if (keys.indexOf(k) > 0) res[k] = obj[k] } + return res +} diff --git a/test/app_config.js b/test/app_config.js index 4c1da10..ed4bea9 100644 --- a/test/app_config.js +++ b/test/app_config.js @@ -18,6 +18,20 @@ test('API config overrides app.js config', (t) => { }) }) -test('Throws error for invalid app.js syntax', (t) => { +test('throws error for invalid app.js syntax', (t) => { return t.throws(() => compileFixture(t, 'app_config_error'), /SyntaxError: Unexpected token :/) }) + +test('adds custom options to the webpack config object', (t) => { + return compileFixture(t, 'app_config', { customOption: 'wow!' }) + .then(({res}) => { + t.truthy(res.stats.compilation.options.customOption === 'wow!') + }) +}) + +test('does not allow certain options to be configured', (t) => { + return compileFixture(t, 'app_config', { context: 'override!' }) + .then(({res}) => { + t.truthy(res.stats.compilation.options.context !== 'override!') + }) +}) From f892a7bc70620e1edbebd68cc345698dcda95c6e Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Thu, 2 Jun 2016 12:14:05 -0400 Subject: [PATCH 2/2] small refactors --- lib/config.js | 6 ++++++ lib/index.js | 8 ++++---- test/_helpers.js | 4 ++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/config.js b/lib/config.js index 69ed318..9a61f8c 100644 --- a/lib/config.js +++ b/lib/config.js @@ -194,6 +194,12 @@ module.exports = class Config { ] } + // If the user has passed options, accept them, but the ones set above take + // priority if there's a conflict, as they are essential to spike. + if (opts.resolveLoader) { + this.resolveLoader = merge(opts.resolveLoader, this.resolveLoader) + } + const reIgnores = opts.ignore.map(mmToRe) const spikeLoaders = [ { diff --git a/lib/index.js b/lib/index.js index 2511ea9..ed7fd68 100644 --- a/lib/index.js +++ b/lib/index.js @@ -14,7 +14,7 @@ const Sprout = require('sprout') const Joi = require('joi') const {EventEmitter} = require('events') const Config = require('./config') -const Errors = require('./errors') +const {Error, Warning} = require('./errors') /** * @class Spike @@ -157,17 +157,17 @@ function npmInstall (opts) { */ function compileCallback (id, err, stats) { if (err) { - return this.emit('error', new Errors.Error({ id: id, message: err })) + return this.emit('error', new Error({ id: id, message: err })) } // Webpack "soft errors" are classified as warnings in spike. An error is // an error. If it doesn't break the build, it's a warning. const jsonStats = stats.toJson() if (jsonStats.errors.length) { - this.emit('warning', new Errors.Warning({ id: id, message: jsonStats.errors })) + this.emit('warning', new Warning({ id: id, message: jsonStats.errors })) } /* istanbul ignore next */ if (jsonStats.warnings.length) { - this.emit('warning', new Errors.Warning({ id: id, message: jsonStats.warnings })) + this.emit('warning', new Warning({ id: id, message: jsonStats.warnings })) } this.emit('compile', { id: id, stats: stats }) diff --git a/test/_helpers.js b/test/_helpers.js index deeba8f..0ff85ac 100644 --- a/test/_helpers.js +++ b/test/_helpers.js @@ -20,9 +20,9 @@ exports.compileFixture = function compileFixture (t, name, options = {}) { const project = new Spike(Object.assign(options, { root: testPath })) const publicPath = path.join(testPath, 'public') - return When.promise(function (resolve, reject) { + return When.promise((resolve, reject) => { project.on('error', reject) - project.on('compile', function (res) { resolve({res, publicPath}) }) + project.on('compile', (res) => { resolve({res, publicPath}) }) project.compile() })