Skip to content

Commit

Permalink
[BREAKING] moduleBundler: Always default to optimize: true (#685)
Browse files Browse the repository at this point in the history 
bundleOptions are often omitted. So document them as optional and always
default 'optimize' to true, even if bundleOptions are specified.
  • Loading branch information
@matz3
matz3 authored Jan 20, 2022
1 parent f27513a commit de5837c
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 22 deletions.
18 changes: 11 additions & 7 deletions lib/processors/bundlers/moduleBundler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions lib/tasks/bundlers/generateBundle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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
Expand Down
182 changes: 170 additions & 12 deletions test/lib/processors/bundlers/moduleBundler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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");

Expand Down Expand Up @@ -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");

Expand All @@ -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");

Expand All @@ -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 = [];
Expand Down Expand Up @@ -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");

Expand All @@ -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");

Expand All @@ -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;

Expand All @@ -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",
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)]);
});

0 comments on commit de5837c

Please sign in to comment.