From 7cbdebf84779b5f109239ece5cc00a95d1a1f198 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 13 Jul 2015 13:45:22 -0700 Subject: [PATCH] Fix minify option in cordova and old bundler tests Rename 'mode' and 'minify' to 'minifyMode' consistently (including in minifier plugin API). Make minifyMode default to 'development' (fixes some other old bundler tests). This doesn't make bundler-assets pass but it gets farther along. --- .../standard-minifiers/plugin/minify-css.js | 2 +- .../standard-minifiers/plugin/minify-js.js | 2 +- tools/bundler.js | 49 ++++++++++--------- tools/commands-cordova.js | 2 +- tools/commands.js | 11 ++--- tools/tests/old/test-bundler-options.js | 4 +- 6 files changed, 37 insertions(+), 33 deletions(-) diff --git a/packages/standard-minifiers/plugin/minify-css.js b/packages/standard-minifiers/plugin/minify-css.js index 4c3232876a..336a39a8d7 100644 --- a/packages/standard-minifiers/plugin/minify-css.js +++ b/packages/standard-minifiers/plugin/minify-css.js @@ -10,7 +10,7 @@ Plugin.registerMinifier({ function CssToolsMinifier () {}; CssToolsMinifier.prototype.processFilesForTarget = function (files, options) { - var mode = options.mode; + var mode = options.minifyMode; if (! files.length) return; diff --git a/packages/standard-minifiers/plugin/minify-js.js b/packages/standard-minifiers/plugin/minify-js.js index 1c89606746..653c1243a1 100644 --- a/packages/standard-minifiers/plugin/minify-js.js +++ b/packages/standard-minifiers/plugin/minify-js.js @@ -8,7 +8,7 @@ Plugin.registerMinifier({ function UglifyJSMinifier () {}; UglifyJSMinifier.prototype.processFilesForTarget = function (files, options) { - var mode = options.mode; + var mode = options.minifyMode; // don't minify anything for development if (mode === 'development') { diff --git a/tools/bundler.js b/tools/bundler.js index e2cac7b946..08f60ef305 100644 --- a/tools/bundler.js +++ b/tools/bundler.js @@ -485,7 +485,7 @@ _.extend(Target.prototype, { // options // - packages: packages to include (Isopack or 'foo'), per // _determineLoadOrder - // - minify: 'development'/'production' + // - minifyMode: 'development'/'production' // - addCacheBusters: if true, make all files cacheable by adding // unique query strings to their URLs. unlikely to be of much use // on server targets. @@ -517,12 +517,12 @@ _.extend(Target.prototype, { }); }); if (minifiersByExt.js) { - self.minifyJs(minifiersByExt.js, options.minify); + self.minifyJs(minifiersByExt.js, options.minifyMode); } // CSS is minified only for client targets. if (self instanceof ClientTarget) { if (minifiersByExt.css) { - self.minifyCss(minifiersByExt.css, options.minify); + self.minifyCss(minifiersByExt.css, options.minifyMode); } } @@ -824,7 +824,7 @@ _.extend(Target.prototype, { }), // Minify the JS in this target - minifyJs: Profile("Target#minifyJs", function (minifierDef, mode) { + minifyJs: Profile("Target#minifyJs", function (minifierDef, minifyMode) { var self = this; // Avoid circular deps from top-level import. @@ -841,7 +841,7 @@ _.extend(Target.prototype, { buildmessage.enterJob("minifying app code", function () { try { var markedMinifier = buildmessage.markBoundary(minifier); - markedMinifier(sources, { mode: mode }); + markedMinifier(sources, { minifyMode }); } catch (e) { buildmessage.exception(e); } @@ -850,7 +850,8 @@ _.extend(Target.prototype, { self.js = _.flatten(_.map(sources, function (source) { return _.map(source._minifiedFiles, function (file) { var newFile = new File({ - info: mode === 'production' ? 'minified js' : '?', + // XXX BBP what is this ? here? + info: minifyMode === 'production' ? 'minified js' : '?', data: new Buffer(file.data, 'utf8') }); if (file.sourceMap) { @@ -989,7 +990,7 @@ util.inherits(ClientTarget, Target); _.extend(ClientTarget.prototype, { // Minify the CSS in this target - minifyCss: Profile("ClientTarget#minifyCss", function (minifierDef, mode) { + minifyCss: Profile("ClientTarget#minifyCss", function (minifierDef, minifyMode) { var self = this; // Avoid circular deps from top-level import. @@ -1006,7 +1007,7 @@ _.extend(ClientTarget.prototype, { buildmessage.enterJob("minifying app stylesheet", function () { try { var markedMinifier = buildmessage.markBoundary(minifier); - markedMinifier(sources, { mode: mode }); + markedMinifier(sources, { minifyMode }); } catch (e) { buildmessage.exception(e); } @@ -1015,7 +1016,8 @@ _.extend(ClientTarget.prototype, { self.css = _.flatten(_.map(sources, function (source) { return _.map(source._minifiedFiles, function (file) { var newFile = new File({ - info: mode === 'production' ? 'minified css' : '?', + // XXX BBP what is this ? ? + info: minifyMode === 'production' ? 'minified css' : '?', data: new Buffer(file.data, 'utf8') }); if (file.sourceMap) { @@ -1041,7 +1043,7 @@ _.extend(ClientTarget.prototype, { // the target // - nodePath: an array of paths required to be set in the NODE_PATH // environment variable. - write: Profile("ClientTarget#write", function (builder, {mode}) { + write: Profile("ClientTarget#write", function (builder, {minifyMode}) { var self = this; builder.reserve("program.json"); @@ -1089,7 +1091,7 @@ _.extend(ClientTarget.prototype, { let mapData = null; // don't need to do this in devel mode - if (mode === 'production') { + if (minifyMode === 'production') { mapData = antiXSSIPrepend(JSON.stringify(file.sourceMap)); } else { mapData = new Buffer(JSON.stringify(file.sourceMap), 'utf8'); @@ -1726,15 +1728,15 @@ var writeTargetToPath = Profile( includeNodeModules, getRelativeTargetPath, previousBuilder, - mode + minifyMode }) { var builder = new Builder({ outputPath: files.pathJoin(outputPath, 'programs', name), previousBuilder }); - var targetBuild = - target.write(builder, {includeNodeModules, getRelativeTargetPath, mode}); + var targetBuild = target.write( + builder, {includeNodeModules, getRelativeTargetPath, minifyMode}); builder.complete(); @@ -1783,7 +1785,7 @@ var writeSiteArchive = Profile( releaseName, getRelativeTargetPath, previousBuilders, - mode + minifyMode }) { const builders = {}; @@ -1859,7 +1861,7 @@ Find out more about Meteor at meteor.com. releaseName, getRelativeTargetPath, previousBuilder, - mode + minifyMode }); builders[name] = targetBuilder; @@ -1934,7 +1936,7 @@ Find out more about Meteor at meteor.com. * ~/.meteor/packages/package/version/npm) * * - buildOptions: may include - * - minify: string, type of minification for the CSS and JS assets + * - minifyMode: string, type of minification for the CSS and JS assets * ('development'/'production', defaults to 'development') * - serverArch: the server architecture to target (string, default * archinfo.host()) @@ -1984,6 +1986,7 @@ exports.bundle = function ({ var serverArch = buildOptions.serverArch || archinfo.host(); var webArchs = buildOptions.webArchs || projectContext.platformList.getWebArchs(); + const minifyMode = buildOptions.minifyMode || 'development'; var releaseName = release.current.isCheckout() ? "none" : release.current.name; @@ -2019,7 +2022,7 @@ exports.bundle = function ({ client.make({ packages: [app], - minify: buildOptions.minify, + minifyMode: minifyMode, minifiers: options.minifiers || [], addCacheBusters: true }); @@ -2043,7 +2046,9 @@ exports.bundle = function ({ server.make({ packages: [app], - minify: 'development' + // Never minify on the server. + // XXX BBP This is weird. Why not leave this up to the minifier plugin? + minifyMode: 'development' }); return server; @@ -2073,8 +2078,8 @@ exports.bundle = function ({ } var minifiers = null; - if (! _.contains(['development', 'production'], buildOptions.minify)) { - throw new Error('Unrecognized minification mode: ' + buildOptions.minify); + if (! _.contains(['development', 'production'], minifyMode)) { + throw new Error('Unrecognized minification mode: ' + minifyMode); } minifiers = compiler.getMinifiers(packageSource, { isopackCache: projectContext.isopackCache, @@ -2122,7 +2127,7 @@ exports.bundle = function ({ builtBy, releaseName, getRelativeTargetPath, - mode: buildOptions.minify + minifyMode: minifyMode }; if (outputPath !== null) { diff --git a/tools/commands-cordova.js b/tools/commands-cordova.js index 70409f038f..5843541621 100644 --- a/tools/commands-cordova.js +++ b/tools/commands-cordova.js @@ -348,7 +348,7 @@ var getBundle = function (projectContext, bundlePath, options) { projectContext: projectContext, outputPath: bundlePath, buildOptions: { - minify: ! options.debug, + minifyMode: options.debug ? 'development' : 'production', // XXX can we ask it not to create the server arch? serverArch: archinfo.host(), webArchs: [WEB_ARCH_NAME], diff --git a/tools/commands.js b/tools/commands.js index 8f3f15ec7b..c20669b290 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -403,7 +403,7 @@ function doRunCommand (options) { debugPort: options['debug-port'], settingsFile: options.settings, buildOptions: { - minify: options.production ? 'production' : 'development', + minifyMode: options.production ? 'production' : 'development', includeDebug: ! options.production }, rootUrl: process.env.ROOT_URL, @@ -899,7 +899,7 @@ var buildCommand = function (options) { projectContext: projectContext, outputPath: bundlePath, buildOptions: { - minify: options.debug ? 'development' : 'production', + minifyMode: options.debug ? 'development' : 'production', // XXX is this a good idea, or should linux be the default since // that's where most people are deploying // default? i guess the problem with using DEPLOY_ARCH as default @@ -1007,7 +1007,6 @@ main.registerCommand({ }, () => packageSource.initFromPackageDir(packageDir)); }); - main.captureAndExit("=> Errors while setting up package:", () => // Read metadata and initialize catalog. projectContext.initializeCatalog() @@ -1040,7 +1039,7 @@ main.registerCommand({ projectContext: projectContext, outputPath: null, buildOptions: { - minify: 'development' + minifyMode: 'development' } }); @@ -1249,7 +1248,7 @@ main.registerCommand({ projectContext.packageMapDelta.displayOnConsole(); var buildOptions = { - minify: options.debug ? 'development' : 'production', + minifyMode: options.debug ? 'development' : 'production', includeDebug: options.debug, serverArch: buildArch }; @@ -1651,7 +1650,7 @@ var getTestPackageNames = function (projectContext, packageNames) { var runTestAppForPackages = function (projectContext, options) { var buildOptions = { - minify: options.production ? 'production' : 'development', + minifyMode: options.production ? 'production' : 'development', includeDebug: ! options.production }; diff --git a/tools/tests/old/test-bundler-options.js b/tools/tests/old/test-bundler-options.js index 0dc434eab7..ff9df7fe47 100644 --- a/tools/tests/old/test-bundler-options.js +++ b/tools/tests/old/test-bundler-options.js @@ -61,7 +61,7 @@ var runTest = function () { var result = bundler.bundle({ projectContext: projectContext, outputPath: tmpOutputDir, - buildOptions: { minify: true } + buildOptions: { minifyMode: 'production' } }); assert.strictEqual(result.errors, false, result.errors && result.errors[0]); @@ -93,7 +93,7 @@ var runTest = function () { var result = bundler.bundle({ projectContext: projectContext, outputPath: tmpOutputDir, - buildOptions: { minify: false } + buildOptions: { minifyMode: 'development' } }); assert.strictEqual(result.errors, false);