[BREAKING] moduleBundler: Always default to optimize: true

lib/processors/bundlers/moduleBundler.js

@@ -89,7 +89,7 @@ const log = require("@ui5/logger").getLogger("builder:processors:bundlers:module
  *
  * @public
  * @typedef {object} ModuleBundleOptions
- * @property {boolean} [optimize=false] If set to 'true' the module bundle gets minified
+ * @property {boolean} [optimize=true] Whether the module bundle gets minified
  * @property {boolean} [decorateBootstrapModule=false] If set to 'false', the module won't be decorated
  *   with an optimization marker
  * @property {boolean} [addTryCatchRestartWrapper=false] Whether to wrap bootable module bundles with
@@ -109,15 +109,19 @@ const log = require("@ui5/logger").getLogger("builder:processors:bundlers:module
  * @param {module:@ui5/fs.Resource[]} parameters.resources Resources
  * @param {object} parameters.options Options
  * @param {ModuleBundleDefinition} parameters.options.bundleDefinition Module bundle definition
- * @param {ModuleBundleOptions} parameters.options.bundleOptions Module bundle options
+ * @param {ModuleBundleOptions} [parameters.options.bundleOptions] Module bundle options
  * @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving with module bundle resources
  */
 module.exports = function({resources, options: {bundleDefinition, bundleOptions}}) {
-//	console.log("preloadBundler bundleDefinition:");
-//	console.log(JSON.stringify(options.bundleDefinition, null, 4));
-
-	// TODO 3.0: Fix defaulting behavior, align with JSDoc
-	bundleOptions = bundleOptions || {optimize: true};
+	// Apply defaults without modifying the passed object
+	bundleOptions = Object.assign({}, {
+		optimize: true,
+		decorateBootstrapModule: false,
+		addTryCatchRestartWrapper: false,
+		usePredefineCalls: false,
+		numberOfParts: 1,
+		ignoreMissingModules: false
+	}, bundleOptions);
 
 	const pool = new LocatorResourcePool({
 		ignoreMissingModules: bundleOptions.ignoreMissingModules

lib/tasks/bundlers/generateBundle.js

@@ -14,7 +14,7 @@ const ReaderCollectionPrioritized = require("@ui5/fs").ReaderCollectionPrioritiz
  * @param {object} parameters.options Options
  * @param {string} parameters.options.projectName Project name
  * @param {ModuleBundleDefinition} parameters.options.bundleDefinition Module bundle definition
- * @param {ModuleBundleOptions} parameters.options.bundleOptions Module bundle options
+ * @param {ModuleBundleOptions} [parameters.options.bundleOptions] Module bundle options
  * @returns {Promise} Promise resolving with <code>undefined</code> once data has been written
  */
 module.exports = function({
@@ -26,14 +26,16 @@ module.exports = function({
 	});
 
 	if (taskUtil) {
+		const optimize = !bundleOptions || bundleOptions.optimize !== false;
+
 		// Omit -dbg files for optimize bundles and vice versa
-		const filterTag = bundleOptions.optimize ?
+		const filterTag = optimize ?
 			taskUtil.STANDARD_TAGS.IsDebugVariant : taskUtil.STANDARD_TAGS.HasDebugVariant;
 		combo = combo.filter(function(resource) {
 			return !taskUtil.getTag(resource, filterTag);
 		});
 
-		if (!bundleOptions.optimize) {
+		if (!optimize) {
 			// For "unoptimized" bundles, the non-debug files have already been filtered out
 			// Now rename the debug variants to the same name so that they appear like the original
 			// resource to the bundler

test/lib/processors/bundlers/moduleBundler.js

@@ -77,7 +77,7 @@ test.serial("Builder returns single bundle", async (t) => {
 	t.true(LocatorResourcePool.calledWithNew());
 	t.deepEqual(LocatorResourcePool.getCall(0).args, [
 		{
-			ignoreMissingModules: undefined // not defined in bundleOptions
+			ignoreMissingModules: false // default
 		}
 	], "LocatorResourcePool should be called with expected args");
 
@@ -94,8 +94,18 @@ test.serial("Builder returns single bundle", async (t) => {
 	t.is(builder.createBundle.getCall(0).args.length, 2);
 	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
 		"builder.createBundle should be called with bundleDefinition");
-	t.is(builder.createBundle.getCall(0).args[1], bundleOptions,
-		"builder.createBundle should be called with bundleOptions");
+	t.deepEqual(builder.createBundle.getCall(0).args[1], {
+		// default bundleOptions
+		optimize: true,
+		decorateBootstrapModule: false,
+		addTryCatchRestartWrapper: false,
+		usePredefineCalls: false,
+		numberOfParts: 1,
+		ignoreMissingModules: false,
+
+		some: "option"
+	},
+	"builder.createBundle should be called with bundleOptions");
 	t.true(builder.createBundle.calledAfter(pool.prepare),
 		"builder.createBundle should be called before pool.prepare");
 
@@ -173,7 +183,7 @@ test.serial("Builder returns multiple bundles", async (t) => {
 	t.true(LocatorResourcePool.calledWithNew());
 	t.deepEqual(LocatorResourcePool.getCall(0).args, [
 		{
-			ignoreMissingModules: undefined // not defined in bundleOptions
+			ignoreMissingModules: false // default
 		}
 	], "LocatorResourcePool should be called with expected args");
 
@@ -190,8 +200,18 @@ test.serial("Builder returns multiple bundles", async (t) => {
 	t.is(builder.createBundle.getCall(0).args.length, 2);
 	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
 		"builder.createBundle should be called with bundleDefinition");
-	t.is(builder.createBundle.getCall(0).args[1], bundleOptions,
-		"builder.createBundle should be called with bundleOptions");
+	t.deepEqual(builder.createBundle.getCall(0).args[1], {
+		// default bundleOptions
+		optimize: true,
+		decorateBootstrapModule: false,
+		addTryCatchRestartWrapper: false,
+		usePredefineCalls: false,
+		numberOfParts: 1,
+		ignoreMissingModules: false,
+
+		some: "option"
+	},
+	"builder.createBundle should be called with bundleOptions");
 	t.true(builder.createBundle.calledAfter(pool.prepare),
 		"builder.createBundle should be called before pool.prepare");
 
@@ -213,7 +233,7 @@ test.serial("Builder returns multiple bundles", async (t) => {
 	t.is(log.verbose.callCount, 0, "log.verbose is not called when verbose level is not enabled");
 });
 
-test.serial("bundleOptions default", async (t) => {
+test.serial("bundleOptions default (no options passed)", async (t) => {
 	const {processor, Resource, LocatorResourcePool, pool, BundleBuilder, builder, log} = t.context;
 
 	const resources = [];
@@ -250,7 +270,7 @@ test.serial("bundleOptions default", async (t) => {
 	t.true(LocatorResourcePool.calledWithNew());
 	t.deepEqual(LocatorResourcePool.getCall(0).args, [
 		{
-			ignoreMissingModules: undefined // not defined in bundleOptions
+			ignoreMissingModules: false // default
 		}
 	], "LocatorResourcePool should be called with expected args");
 
@@ -267,8 +287,16 @@ test.serial("bundleOptions default", async (t) => {
 	t.is(builder.createBundle.getCall(0).args.length, 2);
 	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
 		"builder.createBundle should be called with bundleDefinition");
-	t.deepEqual(builder.createBundle.getCall(0).args[1], {optimize: true}, // default bundleOptions
-		"builder.createBundle should be called with bundleOptions");
+	t.deepEqual(builder.createBundle.getCall(0).args[1], {
+		// default bundleOptions
+		optimize: true,
+		decorateBootstrapModule: false,
+		addTryCatchRestartWrapper: false,
+		usePredefineCalls: false,
+		numberOfParts: 1,
+		ignoreMissingModules: false
+	},
+	"builder.createBundle should be called with bundleOptions");
 	t.true(builder.createBundle.calledAfter(pool.prepare),
 		"builder.createBundle should be called before pool.prepare");
 
@@ -284,6 +312,113 @@ test.serial("bundleOptions default", async (t) => {
 	t.is(log.verbose.callCount, 0, "log.verbose is not called when verbose level is not enabled");
 });
 
+test.serial("bundleOptions default (empty options passed)", async (t) => {
+	const {processor, LocatorResourcePool, builder, log} = t.context;
+
+	const resources = [];
+	const bundleDefinition = {
+		"some": "definition"
+	};
+	const bundleOptions = {};
+
+	const createdBundle = {
+		name: "BundleName.js",
+		content: "Bundle Content",
+		bundleInfo: {
+			"Bundle": "Info"
+		}
+	};
+
+	builder.createBundle.resolves(createdBundle);
+
+	await processor({
+		resources,
+		options: {
+			bundleDefinition,
+			bundleOptions
+		}
+	});
+
+	t.is(LocatorResourcePool.callCount, 1, "LocatorResourcePool should be created once");
+	t.true(LocatorResourcePool.calledWithNew());
+	t.deepEqual(LocatorResourcePool.getCall(0).args, [
+		{
+			ignoreMissingModules: false // default
+		}
+	], "LocatorResourcePool should be called with expected args");
+
+	t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
+	t.is(builder.createBundle.getCall(0).args.length, 2);
+	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
+		"builder.createBundle should be called with bundleDefinition");
+	t.deepEqual(builder.createBundle.getCall(0).args[1], {
+		// default bundleOptions
+		optimize: true,
+		decorateBootstrapModule: false,
+		addTryCatchRestartWrapper: false,
+		usePredefineCalls: false,
+		numberOfParts: 1,
+		ignoreMissingModules: false
+	},
+	"builder.createBundle should be called with bundleOptions");
+
+	t.deepEqual(bundleOptions, {}, "Passed bundleOptions object should not be modified");
+
+	t.is(log.verbose.callCount, 0, "log.verbose is not called when verbose level is not enabled");
+});
+
+test.serial("bundleOptions (all options passed)", async (t) => {
+	const {processor, LocatorResourcePool, builder, log} = t.context;
+
+	const resources = [];
+	const bundleDefinition = {
+		"some": "definition"
+	};
+	const bundleOptions = {
+		optimize: false,
+		decorateBootstrapModule: true,
+		addTryCatchRestartWrapper: true,
+		usePredefineCalls: true,
+		numberOfParts: 7,
+		ignoreMissingModules: true
+	};
+
+	const createdBundle = {
+		name: "BundleName.js",
+		content: "Bundle Content",
+		bundleInfo: {
+			"Bundle": "Info"
+		}
+	};
+
+	builder.createBundle.resolves(createdBundle);
+
+	await processor({
+		resources,
+		options: {
+			bundleDefinition,
+			bundleOptions
+		}
+	});
+
+	t.is(LocatorResourcePool.callCount, 1, "LocatorResourcePool should be created once");
+	t.true(LocatorResourcePool.calledWithNew());
+	t.deepEqual(LocatorResourcePool.getCall(0).args, [
+		{
+			ignoreMissingModules: true
+		}
+	], "LocatorResourcePool should be called with expected args");
+
+	t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
+	t.is(builder.createBundle.getCall(0).args.length, 2);
+	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
+		"builder.createBundle should be called with bundleDefinition");
+	t.deepEqual(builder.createBundle.getCall(0).args[1], bundleOptions,
+		"builder.createBundle should be called with bundleOptions");
+
+	t.is(log.verbose.callCount, 0, "log.verbose is not called when verbose level is not enabled");
+});
+
 test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", async (t) => {
 	const {processor, Resource, LocatorResourcePool, pool, BundleBuilder, builder, log} = t.context;
 
@@ -295,6 +430,17 @@ test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", a
 		ignoreMissingModules: "foo"
 	};
 
+	const effectiveBundleOptions = {
+		// Defaults
+		"optimize": true,
+		"decorateBootstrapModule": false,
+		"addTryCatchRestartWrapper": false,
+		"usePredefineCalls": false,
+		"numberOfParts": 1,
+
+		"ignoreMissingModules": "foo"
+	};
+
 	const createdBundle = {
 		name: "BundleName.js",
 		content: "Bundle Content",
@@ -342,7 +488,7 @@ test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", a
 	t.is(builder.createBundle.getCall(0).args.length, 2);
 	t.is(builder.createBundle.getCall(0).args[0], bundleDefinition,
 		"builder.createBundle should be called with bundleDefinition");
-	t.is(builder.createBundle.getCall(0).args[1], bundleOptions,
+	t.deepEqual(builder.createBundle.getCall(0).args[1], effectiveBundleOptions,
 		"builder.createBundle should be called with bundleOptions");
 	t.true(builder.createBundle.calledAfter(pool.prepare),
 		"builder.createBundle should be called before pool.prepare");
@@ -370,6 +516,18 @@ test.serial("Verbose Logging", async (t) => {
 		"some": "option"
 	};
 
+	const effectiveBundleOptions = {
+		// Defaults
+		"optimize": true,
+		"decorateBootstrapModule": false,
+		"addTryCatchRestartWrapper": false,
+		"usePredefineCalls": false,
+		"numberOfParts": 1,
+		"ignoreMissingModules": false,
+
+		"some": "option",
+	};
+
 	const createdBundle = {
 		name: "Bundle Name",
 		content: "Bundle Content",
@@ -402,5 +560,5 @@ test.serial("Verbose Logging", async (t) => {
 
 	t.deepEqual(log.verbose.getCall(0).args, ["Generating bundle:"]);
 	t.deepEqual(log.verbose.getCall(1).args, ["bundleDefinition: " + JSON.stringify(bundleDefinition, null, 2)]);
-	t.deepEqual(log.verbose.getCall(2).args, ["bundleOptions: " + JSON.stringify(bundleOptions, null, 2)]);
+	t.deepEqual(log.verbose.getCall(2).args, ["bundleOptions: " + JSON.stringify(effectiveBundleOptions, null, 2)]);
 });