From 63b5c2203b64272f6e885b6c7e9895355be46113 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 24 Feb 2022 13:24:01 +0100 Subject: [PATCH 01/13] [INTERNAL] minify task: Add more test cases --- test/lib/tasks/minify.js | 129 +++++++++++++++++++++++++++++++++------ 1 file changed, 112 insertions(+), 17 deletions(-) diff --git a/test/lib/tasks/minify.js b/test/lib/tasks/minify.js index 358a126c1..cea4f7db1 100644 --- a/test/lib/tasks/minify.js +++ b/test/lib/tasks/minify.js @@ -6,6 +6,22 @@ const ui5Fs = require("@ui5/fs"); const resourceFactory = ui5Fs.resourceFactory; const DuplexCollection = ui5Fs.DuplexCollection; +// Node.js itself tries to parse sourceMappingURLs in all JavaScript files. This is unwanted and might even lead to +// obscure errors when dynamically generating Data-URI soruceMappingURL values. +// Therefore use this constant to never write the actual string. +const SOURCE_MAPPING_URL = "//" + "# sourceMappingURL"; + +function createWorkspace() { + const reader = resourceFactory.createAdapter({ + virBasePath: "/" + }); + const writer = resourceFactory.createAdapter({ + virBasePath: "/" + }); + const workspace = new DuplexCollection({reader: reader, writer: writer}); + return {reader, writer, workspace}; +} + test.afterEach.always((t) => { sinon.restore(); }); @@ -19,13 +35,7 @@ test("integration: minify", async (t) => { OmitFromBuildResult: "3️⃣" } }; - const reader = resourceFactory.createAdapter({ - virBasePath: "/" - }); - const writer = resourceFactory.createAdapter({ - virBasePath: "/" - }); - const duplexCollection = new DuplexCollection({reader: reader, writer: writer}); + const {reader, writer, workspace} = createWorkspace(); const content = ` function test(paramA) { var variableA = paramA; @@ -39,7 +49,7 @@ test();`; await reader.write(testResource); await minify({ - workspace: duplexCollection, + workspace, taskUtil, options: { pattern: "/test.js" @@ -88,13 +98,7 @@ test("integration: minify omitSourceMapResources=false", async (t) => { OmitFromBuildResult: "3️⃣" } }; - const reader = resourceFactory.createAdapter({ - virBasePath: "/" - }); - const writer = resourceFactory.createAdapter({ - virBasePath: "/" - }); - const duplexCollection = new DuplexCollection({reader: reader, writer: writer}); + const {reader, writer, workspace} = createWorkspace(); const content = ` function test(paramA) { var variableA = paramA; @@ -108,7 +112,7 @@ test();`; await reader.write(testResource); await minify({ - workspace: duplexCollection, + workspace, taskUtil, options: { pattern: "/test.js", @@ -117,7 +121,7 @@ test();`; }); const expected = `function test(t){var o=t;console.log(o)}test(); -//# sourceMappingURL=test.js.map`; +${SOURCE_MAPPING_URL}=test.js.map`; const res = await writer.byPath("/test.js"); if (!res) { t.fail("Could not find /test.js in target locator"); @@ -148,3 +152,94 @@ test();`; "Third taskUtil.setTag call with expected arguments"); }); +test("integration: minify (without taskUtil)", async (t) => { + const {reader, writer, workspace} = createWorkspace(); + const content = ` +function test(paramA) { + var variableA = paramA; + console.log(variableA); +} +test();`; + const testResource = resourceFactory.createResource({ + path: "/test.js", + string: content + }); + await reader.write(testResource); + + await minify({ + workspace, + options: { + pattern: "/test.js" + } + }); + + const expected = `function test(t){var o=t;console.log(o)}test();`; + const res = await writer.byPath("/test.js"); + if (!res) { + t.fail("Could not find /test.js in target locator"); + } + t.deepEqual(await res.getString(), expected, "Correct file content"); + + const resDbg = await writer.byPath("/test-dbg.js"); + if (!resDbg) { + t.fail("Could not find /test-dbg.js in target locator"); + } + t.deepEqual(await resDbg.getString(), content, "Correct debug-file content"); + + const expectedSourceMap = + `{"version":3,"sources":["test-dbg.js"],"names":["test","paramA","variableA","console","log"],` + + `"mappings":"AACA,SAASA,KAAKC,GACb,IAAIC,EAAYD,EAChBE,QAAQC,IAAIF,GAEbF","file":"test.js"}`; + + const resSourceMap = await writer.byPath("/test.js.map"); + if (!resSourceMap) { + t.fail("Could not find /test-dbg.js.map in target locator"); + } + t.deepEqual(await resSourceMap.getString(), expectedSourceMap, "Correct source map content"); +}); + +test("integration: minify omitSourceMapResources=false (without taskUtil)", async (t) => { + const {reader, writer, workspace} = createWorkspace(); + const content = ` +function test(paramA) { + var variableA = paramA; + console.log(variableA); +} +test();`; + const testResource = resourceFactory.createResource({ + path: "/test.js", + string: content + }); + await reader.write(testResource); + + await minify({ + workspace, + options: { + pattern: "/test.js", + omitSourceMapResources: false + } + }); + + const expected = `function test(t){var o=t;console.log(o)}test(); +${SOURCE_MAPPING_URL}=test.js.map`; + const res = await writer.byPath("/test.js"); + if (!res) { + t.fail("Could not find /test.js in target locator"); + } + t.deepEqual(await res.getString(), expected, "Correct file content"); + + const resDbg = await writer.byPath("/test-dbg.js"); + if (!resDbg) { + t.fail("Could not find /test-dbg.js in target locator"); + } + t.deepEqual(await resDbg.getString(), content, "Correct debug-file content"); + + const expectedSourceMap = + `{"version":3,"sources":["test-dbg.js"],"names":["test","paramA","variableA","console","log"],` + + `"mappings":"AACA,SAASA,KAAKC,GACb,IAAIC,EAAYD,EAChBE,QAAQC,IAAIF,GAEbF","file":"test.js"}`; + + const resSourceMap = await writer.byPath("/test.js.map"); + if (!resSourceMap) { + t.fail("Could not find /test-dbg.js.map in target locator"); + } + t.deepEqual(await resSourceMap.getString(), expectedSourceMap, "Correct source map content"); +}); From 1735719d616f0d1e0763b84aa1c9078a742ff032 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 24 Feb 2022 16:34:57 +0100 Subject: [PATCH 02/13] [INTERNAL] moduleBundler: Fix JSDoc returns type --- lib/processors/bundlers/moduleBundler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/processors/bundlers/moduleBundler.js b/lib/processors/bundlers/moduleBundler.js index e4080b2df..9ecf93b54 100644 --- a/lib/processors/bundlers/moduleBundler.js +++ b/lib/processors/bundlers/moduleBundler.js @@ -124,7 +124,8 @@ const log = require("@ui5/logger").getLogger("builder:processors:bundlers:module Optional mapping of resource paths to module name in order to overwrite the default determination * @param {ModuleBundleDefinition} parameters.options.bundleDefinition Module bundle definition * @param {ModuleBundleOptions} [parameters.options.bundleOptions] Module bundle options - * @returns {Promise} Promise resolving with module bundle resources + * @returns {Promise} + * Promise resolving with module bundle resources */ module.exports = function({resources, options: {bundleDefinition, bundleOptions, moduleNameMapping}}) { // Apply defaults without modifying the passed object From de78e9877b30e1a9705763faec75a4ccb58416a3 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 24 Feb 2022 17:47:58 +0100 Subject: [PATCH 03/13] [INTERNAL] Remove unused commented code --- lib/lbt/resources/LocatorResource.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/lbt/resources/LocatorResource.js b/lib/lbt/resources/LocatorResource.js index a39fbb37e..cb73c721e 100644 --- a/lib/lbt/resources/LocatorResource.js +++ b/lib/lbt/resources/LocatorResource.js @@ -1,9 +1,5 @@ const Resource = require("./Resource"); -// function extractName(path) { -// return path.slice( "/resources/".length); -// } - class LocatorResource extends Resource { constructor(pool, resource, moduleName) { super(pool, moduleName, null, resource.getStatInfo()); From 6c50450c1b07ee56790e76f5e0f979126052eea5 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 24 Feb 2022 17:49:28 +0100 Subject: [PATCH 04/13] [INTERNAL] generateBundle: Add unit test --- test/lib/tasks/bundlers/generateBundle.js | 292 ++++++++++++++++++++++ 1 file changed, 292 insertions(+) create mode 100644 test/lib/tasks/bundlers/generateBundle.js diff --git a/test/lib/tasks/bundlers/generateBundle.js b/test/lib/tasks/bundlers/generateBundle.js new file mode 100644 index 000000000..dc59f6e39 --- /dev/null +++ b/test/lib/tasks/bundlers/generateBundle.js @@ -0,0 +1,292 @@ +const test = require("ava"); +const sinon = require("sinon"); +const mock = require("mock-require"); + +test.beforeEach((t) => { + t.context.log = { + warn: sinon.stub(), + verbose: sinon.stub(), + error: sinon.stub() + }; + + t.context.taskUtil = { + getTag: sinon.stub(), + setTag: sinon.stub(), + clearTag: sinon.stub(), + STANDARD_TAGS: { + HasDebugVariant: "", + IsDebugVariant: "", + OmitFromBuildResult: "" + } + }; + + t.context.workspace = { + byGlob: sinon.stub().resolves([]), + write: sinon.stub().resolves() + }; + t.context.dependencies = {}; + t.context.combo = { + byGlob: sinon.stub().resolves([]), + filter: sinon.stub() + }; + + t.context.ReaderCollectionPrioritizedStub = sinon.stub(); + t.context.ReaderCollectionPrioritizedStub.returns(t.context.combo); + mock("@ui5/fs", { + ReaderCollectionPrioritized: t.context.ReaderCollectionPrioritizedStub + }); + + t.context.moduleBundlerStub = sinon.stub().resolves([]); + mock("../../../../lib/processors/bundlers/moduleBundler", t.context.moduleBundlerStub); + + t.context.generateBundle = mock.reRequire("../../../../lib/tasks/bundlers/generateBundle"); +}); + +test.afterEach.always(() => { + sinon.restore(); + mock.stopAll(); +}); + +test.serial("generateBundle: No taskUtil, no bundleOptions", async (t) => { + const { + generateBundle, moduleBundlerStub, ReaderCollectionPrioritizedStub, + workspace, dependencies, combo + } = t.context; + + const resources = [ + {"fake": "resource"} + ]; + combo.byGlob.resolves(resources); + + moduleBundlerStub.resolves([ + { + name: "my/app/customBundle.js", + bundle: {"fake": "bundle"}, + sourceMap: {"fake": "sourceMap"} + } + ]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + + await generateBundle({ + workspace, + dependencies, + options: { + projectName: "Test Application", + bundleDefinition + } + }); + + t.is(moduleBundlerStub.callCount, 1, "moduleBundler should have been called once"); + t.deepEqual(moduleBundlerStub.getCall(0).args, [{ + options: { + bundleDefinition, + bundleOptions: undefined, + moduleNameMapping: {} + }, + resources + }]); + + t.is(combo.byGlob.callCount, 1, + "combo.byGlob should have been called once"); + t.deepEqual(combo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], + "combo.byGlob should have been called with expected pattern"); + + t.is(combo.filter.callCount, 0, + "combo.filter should not have been called"); + + t.is(ReaderCollectionPrioritizedStub.callCount, 1, + "ReaderCollectionPrioritized should have been called once"); + t.true(ReaderCollectionPrioritizedStub.calledWithNew(), + "ReaderCollectionPrioritized should have been called with 'new'"); + + const bundleResources = await moduleBundlerStub.getCall(0).returnValue; + t.is(workspace.write.callCount, 2, + "workspace.write should have been called twice"); + t.deepEqual(workspace.write.getCall(0).args, [bundleResources[0].bundle], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(0).args[0], bundleResources[0].bundle, + "workspace.write should have been called with exact resource returned by moduleBundler"); + t.deepEqual(workspace.write.getCall(1).args, [bundleResources[0].sourceMap], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(1).args[0], bundleResources[0].sourceMap, + "workspace.write should have been called with exact resource returned by moduleBundler"); +}); + +test.serial("generateBundle: No bundleOptions, with taskUtil", async (t) => { + const { + generateBundle, moduleBundlerStub, ReaderCollectionPrioritizedStub, + workspace, dependencies, combo, + taskUtil + } = t.context; + + const resources = [ + {"fake": "resource"} + ]; + + const filteredCombo = { + byGlob: sinon.stub().resolves(resources) + }; + combo.filter.returns(filteredCombo); + + moduleBundlerStub.resolves([ + { + name: "my/app/customBundle.js", + bundle: {"fake": "bundle"}, + sourceMap: {"fake": "sourceMap"} + } + ]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + + await generateBundle({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "Test Application", + bundleDefinition + } + }); + + t.is(moduleBundlerStub.callCount, 1, "moduleBundler should have been called once"); + t.deepEqual(moduleBundlerStub.getCall(0).args, [{ + options: { + bundleDefinition, + bundleOptions: undefined, + moduleNameMapping: {} + }, + resources + }]); + + t.is(combo.byGlob.callCount, 0, + "combo.byGlob should not have been called"); + + t.is(combo.filter.callCount, 1, + "combo.filter should have been called once"); + t.is(combo.filter.getCall(0).args.length, 1, + "combo.filter should have been called with one argument"); + t.is(typeof combo.filter.getCall(0).args[0], "function", + "combo.filter should have been called with a function"); + + t.is(filteredCombo.byGlob.callCount, 1, + "filteredCombo.byGlob should have been called once"); + t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], + "filteredCombo.byGlob should have been called with expected pattern"); + + t.is(ReaderCollectionPrioritizedStub.callCount, 1, + "ReaderCollectionPrioritized should have been called once"); + t.true(ReaderCollectionPrioritizedStub.calledWithNew(), + "ReaderCollectionPrioritized should have been called with 'new'"); + + const bundleResources = await moduleBundlerStub.getCall(0).returnValue; + t.is(workspace.write.callCount, 2, + "workspace.write should have been called twice"); + t.deepEqual(workspace.write.getCall(0).args, [bundleResources[0].bundle], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(0).args[0], bundleResources[0].bundle, + "workspace.write should have been called with exact resource returned by moduleBundler"); + t.deepEqual(workspace.write.getCall(1).args, [bundleResources[0].sourceMap], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(1).args[0], bundleResources[0].sourceMap, + "workspace.write should have been called with exact resource returned by moduleBundler"); +}); + +test.serial("generateBundle: bundleOptions: optimize=false, with taskUtil", async (t) => { + const { + generateBundle, moduleBundlerStub, ReaderCollectionPrioritizedStub, + workspace, dependencies, combo, + taskUtil + } = t.context; + + const resources = [ + { + getPath: sinon.stub().returns("/resources/my/app/module-dbg.js") + }, + { + getPath: sinon.stub().returns("/resources/my/app/Main.view.xml") + } + ]; + + const filteredCombo = { + byGlob: sinon.stub().resolves(resources) + }; + combo.filter.returns(filteredCombo); + + taskUtil.getTag.returns(false) + .withArgs("/resources/my/app/module-dbg.js", taskUtil.STANDARD_TAGS.IsDebugVariant) + .returns(true); + + moduleBundlerStub.resolves([ + { + name: "my/app/customBundle.js", + bundle: {"fake": "bundle"}, + sourceMap: {"fake": "sourceMap"} + } + ]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + const bundleOptions = {optimize: false}; + + await generateBundle({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "Test Application", + bundleDefinition, + bundleOptions + } + }); + + t.is(moduleBundlerStub.callCount, 1, "moduleBundler should have been called once"); + t.deepEqual(moduleBundlerStub.getCall(0).args, [{ + options: { + bundleDefinition, + bundleOptions, + moduleNameMapping: { + "/resources/my/app/module-dbg.js": "my/app/module.js" + } + }, + resources + }]); + + t.is(combo.byGlob.callCount, 0, + "combo.byGlob should not have been called"); + + t.is(combo.filter.callCount, 1, + "combo.filter should have been called once"); + t.is(combo.filter.getCall(0).args.length, 1, + "combo.filter should have been called with one argument"); + t.is(typeof combo.filter.getCall(0).args[0], "function", + "combo.filter should have been called with a function"); + + t.is(filteredCombo.byGlob.callCount, 1, + "filteredCombo.byGlob should have been called once"); + t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], + "filteredCombo.byGlob should have been called with expected pattern"); + + t.is(ReaderCollectionPrioritizedStub.callCount, 1, + "ReaderCollectionPrioritized should have been called once"); + t.true(ReaderCollectionPrioritizedStub.calledWithNew(), + "ReaderCollectionPrioritized should have been called with 'new'"); + + const bundleResources = await moduleBundlerStub.getCall(0).returnValue; + t.is(workspace.write.callCount, 2, + "workspace.write should have been called twice"); + t.deepEqual(workspace.write.getCall(0).args, [bundleResources[0].bundle], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(0).args[0], bundleResources[0].bundle, + "workspace.write should have been called with exact resource returned by moduleBundler"); + t.deepEqual(workspace.write.getCall(1).args, [bundleResources[0].sourceMap], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(1).args[0], bundleResources[0].sourceMap, + "workspace.write should have been called with exact resource returned by moduleBundler"); +}); + +// TODO: Test filter function (optimize true/false) + +// TODO: Empty bundle ([undefined]). skipIfEmpty?! From 80159e70b6c6e5c9b366de6e4d9ab8c3688a64ea Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 25 Feb 2022 13:02:19 +0100 Subject: [PATCH 05/13] [INTERNAL] moduleBundler: Fix 'sourceMap' option default --- lib/processors/bundlers/moduleBundler.js | 4 ++-- test/lib/processors/bundlers/moduleBundler.js | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/processors/bundlers/moduleBundler.js b/lib/processors/bundlers/moduleBundler.js index 9ecf93b54..b9edca580 100644 --- a/lib/processors/bundlers/moduleBundler.js +++ b/lib/processors/bundlers/moduleBundler.js @@ -90,8 +90,7 @@ const log = require("@ui5/logger").getLogger("builder:processors:bundlers:module * @public * @typedef {object} ModuleBundleOptions * @property {boolean} [optimize=true] Whether the module bundle gets minified - * @property {boolean} [sourceMap] Whether to generate a source map file for the bundle. - * Defaults to true if optimize is set to true + * @property {boolean} [sourceMap=true] Whether to generate a source map file for the bundle * @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 @@ -131,6 +130,7 @@ module.exports = function({resources, options: {bundleDefinition, bundleOptions, // Apply defaults without modifying the passed object bundleOptions = Object.assign({}, { optimize: true, + sourceMap: true, decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false, diff --git a/test/lib/processors/bundlers/moduleBundler.js b/test/lib/processors/bundlers/moduleBundler.js index 174a95bec..aa01b37df 100644 --- a/test/lib/processors/bundlers/moduleBundler.js +++ b/test/lib/processors/bundlers/moduleBundler.js @@ -98,6 +98,7 @@ test.serial("Builder returns single bundle", async (t) => { t.deepEqual(builder.createBundle.getCall(0).args[1], { // default bundleOptions optimize: true, + sourceMap: true, decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false, @@ -209,6 +210,7 @@ test.serial("Builder returns multiple bundles", async (t) => { t.deepEqual(builder.createBundle.getCall(0).args[1], { // default bundleOptions optimize: true, + sourceMap: true, decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false, @@ -297,6 +299,7 @@ test.serial("bundleOptions default (no options passed)", async (t) => { t.deepEqual(builder.createBundle.getCall(0).args[1], { // default bundleOptions optimize: true, + sourceMap: true, decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false, @@ -361,6 +364,7 @@ test.serial("bundleOptions default (empty options passed)", async (t) => { t.deepEqual(builder.createBundle.getCall(0).args[1], { // default bundleOptions optimize: true, + sourceMap: true, decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false, @@ -383,6 +387,7 @@ test.serial("bundleOptions (all options passed)", async (t) => { }; const bundleOptions = { optimize: false, + sourceMap: false, decorateBootstrapModule: true, addTryCatchRestartWrapper: true, usePredefineCalls: true, @@ -440,6 +445,7 @@ test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", a const effectiveBundleOptions = { // Defaults "optimize": true, + "sourceMap": true, "decorateBootstrapModule": false, "addTryCatchRestartWrapper": false, "usePredefineCalls": false, @@ -527,6 +533,7 @@ test.serial("Verbose Logging", async (t) => { const effectiveBundleOptions = { // Defaults "optimize": true, + "sourceMap": true, "decorateBootstrapModule": false, "addTryCatchRestartWrapper": false, "usePredefineCalls": false, From 7c6673a00a3b0c12d3ca34d247601436e0fc5498 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 25 Feb 2022 13:50:33 +0100 Subject: [PATCH 06/13] [INTERNAL] minifier: Add missing 'options' parameter --- lib/processors/minifier.js | 8 ++- lib/tasks/minify.js | 4 +- test/lib/processors/minifier.js | 94 ++++++++++++++++++++++++++++++--- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/lib/processors/minifier.js b/lib/processors/minifier.js index 1a583fd23..25a2b3575 100644 --- a/lib/processors/minifier.js +++ b/lib/processors/minifier.js @@ -34,12 +34,16 @@ const debugFileRegex = /((?:\.view|\.fragment|\.controller|\.designtime|\.suppor * @alias module:@ui5/builder.processors.minifier * @param {object} parameters Parameters * @param {module:@ui5/fs.Resource[]} parameters.resources List of resources to be processed - * @param {boolean} [parameters.addSourceMappingUrl=true] + * @param {object} [parameters.options] Options + * @param {boolean} [parameters.options.addSourceMappingUrl=true] * Whether to add a sourceMappingURL reference to the end of the minified resource * @returns {Promise} * Promise resolving with object of resource, dbgResource and sourceMap */ -module.exports = async function({resources, addSourceMappingUrl = true}) { +module.exports = async function({resources, options: {addSourceMappingUrl} = {}}) { + if (addSourceMappingUrl === undefined) { + addSourceMappingUrl = true; + } return Promise.all(resources.map(async (resource) => { const dbgPath = resource.getPath().replace(debugFileRegex, "-dbg$1"); const dbgResource = await resource.clone(); diff --git a/lib/tasks/minify.js b/lib/tasks/minify.js index 3fe5c6ddf..c9e409ebf 100644 --- a/lib/tasks/minify.js +++ b/lib/tasks/minify.js @@ -18,7 +18,9 @@ module.exports = async function({workspace, taskUtil, options: {pattern, omitSou const resources = await workspace.byGlob(pattern); const processedResources = await minifier({ resources, - addSourceMappingUrl: !omitSourceMapResources + options: { + addSourceMappingUrl: !omitSourceMapResources + } }); return Promise.all(processedResources.map(async ({resource, dbgResource, sourceMapResource}) => { diff --git a/test/lib/processors/minifier.js b/test/lib/processors/minifier.js index e204ca268..3007b3a5c 100644 --- a/test/lib/processors/minifier.js +++ b/test/lib/processors/minifier.js @@ -4,6 +4,12 @@ const minifier = require("../../../lib/processors/minifier"); const ui5Fs = require("@ui5/fs"); const resourceFactory = ui5Fs.resourceFactory; +// Node.js itself tries to parse sourceMappingURLs in all JavaScript files. This is unwanted and might even lead to +// obscure errors when dynamically generating Data-URI soruceMappingURL values. +// Therefore use this constant to never write the actual string. +const SOURCE_MAPPING_URL = "//" + "# sourceMappingURL"; + + test("Basic minifier", async (t) => { const content = `/*! * \${copyright} @@ -26,7 +32,7 @@ myFun(); * \${copyright} */ function myFunc(e){jQuery.sap.require("something");console.log("Something required")}myFun(); -//# sourceMappingURL=test.controller.js.map`; +${SOURCE_MAPPING_URL}=test.controller.js.map`; t.deepEqual(await resource.getString(), expected, "Correct minified content"); t.deepEqual(await dbgResource.getString(), content, "Correct debug content"); const expectedSourceMap = `{"version":3,"sources":["test-dbg.controller.js"],` + @@ -76,11 +82,11 @@ test3();`; }); const expectedMinified1 = `function test1(t){var o=t;console.log(o)}test1(); -//# sourceMappingURL=test1.controller.js.map`; +${SOURCE_MAPPING_URL}=test1.controller.js.map`; const expectedMinified2 = `function test2(t){var o=t;console.log(o)}test2(); -//# sourceMappingURL=test2.fragment.js.map`; +${SOURCE_MAPPING_URL}=test2.fragment.js.map`; const expectedMinified3 = `function test3(t){var o=t;console.log(o)}test3(); -//# sourceMappingURL=test3.designtime.js.map`; +${SOURCE_MAPPING_URL}=test3.designtime.js.map`; const expectedSourceMap1 = `{"version":3,"sources":["test1-dbg.controller.js"],"names":["test1","paramA","variableA","console","log"],` + @@ -149,7 +155,7 @@ test(); * Copyright SAPUI5 Developers and other contributors */ function test(t){var o=t;console.log(o)}test(); -//# sourceMappingURL=test.view.js.map`; +${SOURCE_MAPPING_URL}=test.view.js.map`; t.deepEqual(await resource.getString(), expected, "Correct minified content"); t.deepEqual(await dbgResource.getString(), content, "Correct debug content"); const expectedSourceMap = @@ -177,7 +183,7 @@ test();`; const expected = `//@ui5-bundle-raw-include sap/ui/my/module.js function test(t){var o=t;console.log(o)}test(); -//# sourceMappingURL=test.js.map`; +${SOURCE_MAPPING_URL}=test.js.map`; t.deepEqual(await resource.getString(), expected, "Correct minified content"); }); @@ -200,10 +206,84 @@ test();`; const expected = `//@ui5-bundle sap/ui/my/module.js function test(t){var o=t;console.log(o)}test(); -//# sourceMappingURL=test.js.map`; +${SOURCE_MAPPING_URL}=test.js.map`; + t.deepEqual(await resource.getString(), expected, "Correct minified content"); +}); + +test("addSourceMappingUrl=false", async (t) => { + const content = ` +//@ui5-bundle sap/ui/my/module.js +function test(paramA) { + var variableA = paramA; + console.log(variableA); +} +test();`; + + const testResource = resourceFactory.createResource({ + path: "/test.js", + string: content + }); + const [{resource}] = await minifier({ + resources: [testResource], + options: { + addSourceMappingUrl: false + } + }); + + const expected = `//@ui5-bundle sap/ui/my/module.js +function test(t){var o=t;console.log(o)}test();`; + t.deepEqual(await resource.getString(), expected, "Correct minified content"); +}); + +test("addSourceMappingUrl=true", async (t) => { + const content = ` +//@ui5-bundle sap/ui/my/module.js +function test(paramA) { + var variableA = paramA; + console.log(variableA); +} +test();`; + + const testResource = resourceFactory.createResource({ + path: "/test.js", + string: content + }); + const [{resource}] = await minifier({ + resources: [testResource], + options: { + addSourceMappingUrl: true + } + }); + + const expected = `//@ui5-bundle sap/ui/my/module.js +function test(t){var o=t;console.log(o)}test(); +${SOURCE_MAPPING_URL}=test.js.map`; t.deepEqual(await resource.getString(), expected, "Correct minified content"); }); +test("empty options object (addSourceMappingUrl defaults to true)", async (t) => { + const content = ` +//@ui5-bundle sap/ui/my/module.js +function test(paramA) { + var variableA = paramA; + console.log(variableA); +} +test();`; + + const testResource = resourceFactory.createResource({ + path: "/test.js", + string: content + }); + const [{resource}] = await minifier({ + resources: [testResource], + options: {} + }); + + const expected = `//@ui5-bundle sap/ui/my/module.js +function test(t){var o=t;console.log(o)}test(); +${SOURCE_MAPPING_URL}=test.js.map`; + t.deepEqual(await resource.getString(), expected, "Correct minified content"); +}); test("minification error", async (t) => { const content = ` From b199e2c155989dd0af1f0248b907ed3d5f47a660 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 25 Feb 2022 14:23:29 +0100 Subject: [PATCH 07/13] [INTERNAL] generateBundle: Fix empty bundle, add more tests --- lib/tasks/bundlers/generateBundle.js | 6 +- test/lib/tasks/bundlers/generateBundle.js | 243 +++++++++++++++++++++- 2 files changed, 244 insertions(+), 5 deletions(-) diff --git a/lib/tasks/bundlers/generateBundle.js b/lib/tasks/bundlers/generateBundle.js index 2d83d1f43..3a6c50086 100644 --- a/lib/tasks/bundlers/generateBundle.js +++ b/lib/tasks/bundlers/generateBundle.js @@ -109,7 +109,11 @@ module.exports = function({ }, resources }).then((bundles) => { - return Promise.all(bundles.map(({bundle, sourceMap}) => { + return Promise.all(bundles.map(({bundle, sourceMap} = {}) => { + if (!bundle) { + // Skip empty bundles + return; + } if (taskUtil) { taskUtil.setTag(bundle, taskUtil.STANDARD_TAGS.IsBundle); if (sourceMap) { diff --git a/test/lib/tasks/bundlers/generateBundle.js b/test/lib/tasks/bundlers/generateBundle.js index dc59f6e39..defed6bac 100644 --- a/test/lib/tasks/bundlers/generateBundle.js +++ b/test/lib/tasks/bundlers/generateBundle.js @@ -168,7 +168,8 @@ test.serial("generateBundle: No bundleOptions, with taskUtil", async (t) => { "combo.filter should have been called once"); t.is(combo.filter.getCall(0).args.length, 1, "combo.filter should have been called with one argument"); - t.is(typeof combo.filter.getCall(0).args[0], "function", + const filterFunction = combo.filter.getCall(0).args[0]; + t.is(typeof filterFunction, "function", "combo.filter should have been called with a function"); t.is(filteredCombo.byGlob.callCount, 1, @@ -176,6 +177,11 @@ test.serial("generateBundle: No bundleOptions, with taskUtil", async (t) => { t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], "filteredCombo.byGlob should have been called with expected pattern"); + t.is(taskUtil.clearTag.callCount, 1); + t.deepEqual(taskUtil.clearTag.getCall(0).args, + [{"fake": "sourceMap"}, taskUtil.STANDARD_TAGS.OmitFromBuildResult], + "OmitFromBuildResult tag should be cleared on source map resource"); + t.is(ReaderCollectionPrioritizedStub.callCount, 1, "ReaderCollectionPrioritized should have been called once"); t.true(ReaderCollectionPrioritizedStub.calledWithNew(), @@ -192,6 +198,24 @@ test.serial("generateBundle: No bundleOptions, with taskUtil", async (t) => { "workspace.write should have been called with expected args"); t.is(workspace.write.getCall(1).args[0], bundleResources[0].sourceMap, "workspace.write should have been called with exact resource returned by moduleBundler"); + + t.is(taskUtil.getTag.callCount, 0, "taskUtil.getTag should not have been called by the task"); + + // Testing the combo.filter function + + const resourceForFilterTest = {}; + taskUtil.getTag.returns(true); + t.false(filterFunction(resourceForFilterTest), + "Filter function should return false if the tag is set"); + taskUtil.getTag.returns(false); + t.true(filterFunction(resourceForFilterTest), + "Filter function should return true if the tag is not set"); + + t.is(taskUtil.getTag.callCount, 2); + t.deepEqual(taskUtil.getTag.getCall(0).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.IsDebugVariant], + "Resource filtering should be done for debug variants as optimize=true is the default"); + t.deepEqual(taskUtil.getTag.getCall(1).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.IsDebugVariant], + "Resource filtering should be done for debug variants as optimize=true is the default"); }); test.serial("generateBundle: bundleOptions: optimize=false, with taskUtil", async (t) => { @@ -261,7 +285,8 @@ test.serial("generateBundle: bundleOptions: optimize=false, with taskUtil", asyn "combo.filter should have been called once"); t.is(combo.filter.getCall(0).args.length, 1, "combo.filter should have been called with one argument"); - t.is(typeof combo.filter.getCall(0).args[0], "function", + const filterFunction = combo.filter.getCall(0).args[0]; + t.is(typeof filterFunction, "function", "combo.filter should have been called with a function"); t.is(filteredCombo.byGlob.callCount, 1, @@ -269,6 +294,19 @@ test.serial("generateBundle: bundleOptions: optimize=false, with taskUtil", asyn t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], "filteredCombo.byGlob should have been called with expected pattern"); + t.is(taskUtil.getTag.callCount, 2); + t.deepEqual(taskUtil.getTag.getCall(0).args, + ["/resources/my/app/Main.view.xml", taskUtil.STANDARD_TAGS.IsDebugVariant], + "First resource should be checked whether it is a debug variant"); + t.deepEqual(taskUtil.getTag.getCall(1).args, + ["/resources/my/app/module-dbg.js", taskUtil.STANDARD_TAGS.IsDebugVariant], + "Second resource should be checked whether it is a debug variant"); + + t.is(taskUtil.clearTag.callCount, 1); + t.deepEqual(taskUtil.clearTag.getCall(0).args, + [{"fake": "sourceMap"}, taskUtil.STANDARD_TAGS.OmitFromBuildResult], + "OmitFromBuildResult tag should be cleared on source map resource"); + t.is(ReaderCollectionPrioritizedStub.callCount, 1, "ReaderCollectionPrioritized should have been called once"); t.true(ReaderCollectionPrioritizedStub.calledWithNew(), @@ -285,8 +323,205 @@ test.serial("generateBundle: bundleOptions: optimize=false, with taskUtil", asyn "workspace.write should have been called with expected args"); t.is(workspace.write.getCall(1).args[0], bundleResources[0].sourceMap, "workspace.write should have been called with exact resource returned by moduleBundler"); + + taskUtil.getTag.reset(); // Reset stub as it has already been called by generateBundle + t.is(taskUtil.getTag.callCount, 0); + + // Testing the combo.filter function + + const resourceForFilterTest = {}; + taskUtil.getTag.returns(true); + t.false(filterFunction(resourceForFilterTest), + "Filter function should return false if the tag is set"); + taskUtil.getTag.returns(false); + t.true(filterFunction(resourceForFilterTest), + "Filter function should return true if the tag is not set"); + + t.is(taskUtil.getTag.callCount, 2); + t.deepEqual(taskUtil.getTag.getCall(0).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.HasDebugVariant], + "Resource filtering should be done for resources that have a debug variant, as optimize=false is set"); + t.deepEqual(taskUtil.getTag.getCall(1).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.HasDebugVariant], + "Resource filtering should be done for resources that have a debug variant, as optimize=false is set"); }); -// TODO: Test filter function (optimize true/false) +test.serial("generateBundle: bundleOptions: sourceMap=false, with taskUtil", async (t) => { + const { + generateBundle, moduleBundlerStub, ReaderCollectionPrioritizedStub, + workspace, dependencies, combo, + taskUtil + } = t.context; + + const resources = [ + { + getPath: sinon.stub().returns("/resources/my/app/module-dbg.js") + }, + { + getPath: sinon.stub().returns("/resources/my/app/Main.view.xml") + } + ]; + + const filteredCombo = { + byGlob: sinon.stub().resolves(resources) + }; + combo.filter.returns(filteredCombo); + + taskUtil.getTag.returns(false) + .withArgs("/resources/my/app/module-dbg.js", taskUtil.STANDARD_TAGS.IsDebugVariant) + .returns(true); + + moduleBundlerStub.resolves([ + { + name: "my/app/customBundle.js", + bundle: {"fake": "bundle"} + } + ]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + const bundleOptions = {sourceMap: false}; + + await generateBundle({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "Test Application", + bundleDefinition, + bundleOptions + } + }); + + t.is(moduleBundlerStub.callCount, 1, "moduleBundler should have been called once"); + t.deepEqual(moduleBundlerStub.getCall(0).args, [{ + options: { + bundleDefinition, + bundleOptions, + moduleNameMapping: {} + }, + resources + }]); + + t.is(combo.byGlob.callCount, 0, + "combo.byGlob should not have been called"); + + t.is(combo.filter.callCount, 1, + "combo.filter should have been called once"); + t.is(combo.filter.getCall(0).args.length, 1, + "combo.filter should have been called with one argument"); + const filterFunction = combo.filter.getCall(0).args[0]; + t.is(typeof filterFunction, "function", + "combo.filter should have been called with a function"); + + t.is(filteredCombo.byGlob.callCount, 1, + "filteredCombo.byGlob should have been called once"); + t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], + "filteredCombo.byGlob should have been called with expected pattern"); -// TODO: Empty bundle ([undefined]). skipIfEmpty?! + t.is(taskUtil.getTag.callCount, 0); + + t.is(taskUtil.clearTag.callCount, 0, + "clearTag should not be called as no source map resource is created"); + + t.is(ReaderCollectionPrioritizedStub.callCount, 1, + "ReaderCollectionPrioritized should have been called once"); + t.true(ReaderCollectionPrioritizedStub.calledWithNew(), + "ReaderCollectionPrioritized should have been called with 'new'"); + + const bundleResources = await moduleBundlerStub.getCall(0).returnValue; + t.is(workspace.write.callCount, 1, + "workspace.write should have been called once"); + t.deepEqual(workspace.write.getCall(0).args, [bundleResources[0].bundle], + "workspace.write should have been called with expected args"); + t.is(workspace.write.getCall(0).args[0], bundleResources[0].bundle, + "workspace.write should have been called with exact resource returned by moduleBundler"); + + taskUtil.getTag.reset(); // Reset stub as it has already been called by generateBundle + t.is(taskUtil.getTag.callCount, 0); + + // Testing the combo.filter function + + const resourceForFilterTest = {}; + taskUtil.getTag.returns(true); + t.false(filterFunction(resourceForFilterTest), + "Filter function should return false if the tag is set"); + taskUtil.getTag.returns(false); + t.true(filterFunction(resourceForFilterTest), + "Filter function should return true if the tag is not set"); + + t.is(taskUtil.getTag.callCount, 2); + t.deepEqual(taskUtil.getTag.getCall(0).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.IsDebugVariant], + "Resource filtering should be done for debug variants as optimize=true is the default"); + t.deepEqual(taskUtil.getTag.getCall(1).args, [resourceForFilterTest, taskUtil.STANDARD_TAGS.IsDebugVariant], + "Resource filtering should be done for debug variants as optimize=true is the default"); +}); + +test.serial("generateBundle: Empty bundle (skipIfEmpty=true)", async (t) => { + const { + generateBundle, moduleBundlerStub, ReaderCollectionPrioritizedStub, + workspace, dependencies, combo, + taskUtil + } = t.context; + + const resources = []; + + const filteredCombo = { + byGlob: sinon.stub().resolves(resources) + }; + combo.filter.returns(filteredCombo); + + moduleBundlerStub.resolves([undefined]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + const bundleOptions = {skipIfEmpty: true}; + + await generateBundle({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "Test Application", + bundleDefinition, + bundleOptions + } + }); + + t.is(moduleBundlerStub.callCount, 1, "moduleBundler should have been called once"); + t.deepEqual(moduleBundlerStub.getCall(0).args, [{ + options: { + bundleDefinition, + bundleOptions, + moduleNameMapping: {} + }, + resources + }]); + + t.is(combo.byGlob.callCount, 0, + "combo.byGlob should not have been called"); + + t.is(combo.filter.callCount, 1, + "combo.filter should have been called once"); + t.is(combo.filter.getCall(0).args.length, 1, + "combo.filter should have been called with one argument"); + const filterFunction = combo.filter.getCall(0).args[0]; + t.is(typeof filterFunction, "function", + "combo.filter should have been called with a function"); + + t.is(filteredCombo.byGlob.callCount, 1, + "filteredCombo.byGlob should have been called once"); + t.deepEqual(filteredCombo.byGlob.getCall(0).args, ["/resources/**/*.{js,json,xml,html,properties,library,js.map}"], + "filteredCombo.byGlob should have been called with expected pattern"); + + t.is(taskUtil.getTag.callCount, 0); + + t.is(taskUtil.clearTag.callCount, 0, + "clearTag should not be called as no source map resource is created"); + + t.is(ReaderCollectionPrioritizedStub.callCount, 1, + "ReaderCollectionPrioritized should have been called once"); + t.true(ReaderCollectionPrioritizedStub.calledWithNew(), + "ReaderCollectionPrioritized should have been called with 'new'"); + + t.is(workspace.write.callCount, 0, + "workspace.write should have been called once"); +}); From 0e505dfa72b235a01049f2f91ff283aebecfd771 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 25 Feb 2022 14:34:03 +0100 Subject: [PATCH 08/13] [INTERNAL] generateBundle: Fix ReaderCollection name --- lib/tasks/bundlers/generateBundle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/bundlers/generateBundle.js b/lib/tasks/bundlers/generateBundle.js index 3a6c50086..b6922b47b 100644 --- a/lib/tasks/bundlers/generateBundle.js +++ b/lib/tasks/bundlers/generateBundle.js @@ -21,7 +21,7 @@ module.exports = function({ workspace, dependencies, taskUtil, options: {projectName, bundleDefinition, bundleOptions} }) { let combo = new ReaderCollectionPrioritized({ - name: `libraryBundler - prioritize workspace over dependencies: ${projectName}`, + name: `generateBundle - prioritize workspace over dependencies: ${projectName}`, readers: [workspace, dependencies] }); From 4ee7b0cc4e4b3fc2140af1743d35593a9a3bbef3 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 25 Feb 2022 14:46:28 +0100 Subject: [PATCH 09/13] [INTERNAL] generateBundle: Add test for exception --- test/lib/tasks/bundlers/generateBundle.js | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/lib/tasks/bundlers/generateBundle.js b/test/lib/tasks/bundlers/generateBundle.js index defed6bac..fb2619cdc 100644 --- a/test/lib/tasks/bundlers/generateBundle.js +++ b/test/lib/tasks/bundlers/generateBundle.js @@ -1,6 +1,7 @@ const test = require("ava"); const sinon = require("sinon"); const mock = require("mock-require"); +const ModuleName = require("../../../../lib/lbt/utils/ModuleName"); test.beforeEach((t) => { t.context.log = { @@ -525,3 +526,44 @@ test.serial("generateBundle: Empty bundle (skipIfEmpty=true)", async (t) => { t.is(workspace.write.callCount, 0, "workspace.write should have been called once"); }); + +test.serial("generateBundle: Throws error when non-debug name can't be resolved", async (t) => { + const { + generateBundle, moduleBundlerStub, + workspace, dependencies, combo, + taskUtil + } = t.context; + + const resources = [ + { + getPath: sinon.stub().returns("/resources/my/app/module.js") + } + ]; + + const filteredCombo = { + byGlob: sinon.stub().resolves(resources) + }; + combo.filter.returns(filteredCombo); + + moduleBundlerStub.resolves([undefined]); + + // bundleDefinition can be empty here as the moduleBundler is mocked + const bundleDefinition = {}; + const bundleOptions = {optimize: false}; + + taskUtil.getTag.returns(true); + sinon.stub(ModuleName, "getNonDebugName").returns(false); + + await t.throwsAsync(generateBundle({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "Test Application", + bundleDefinition, + bundleOptions + } + }), { + message: "Failed to resolve non-debug name for /resources/my/app/module.js" + }); +}); From d1d9b9f80b6bb1b0f8c8ab32ded78f7f8097405a Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 8 Mar 2022 15:42:23 +0100 Subject: [PATCH 10/13] [INTERNAL] Enhance generateStandaloneAppBundle tests --- .../bundlers/generateStandaloneAppBundle.js | 154 +++++++++++++++++- 1 file changed, 149 insertions(+), 5 deletions(-) diff --git a/test/lib/tasks/bundlers/generateStandaloneAppBundle.js b/test/lib/tasks/bundlers/generateStandaloneAppBundle.js index b7a4e9766..a24329633 100644 --- a/test/lib/tasks/bundlers/generateStandaloneAppBundle.js +++ b/test/lib/tasks/bundlers/generateStandaloneAppBundle.js @@ -9,9 +9,20 @@ let generateStandaloneAppBundle = require("../../../../lib/tasks/bundlers/genera test.beforeEach((t) => { // Stubbing processors/bundlers/moduleBundler t.context.moduleBundlerStub = sinon.stub(); - t.context.moduleBundlerStub.resolves(["I am a resource"]); + t.context.moduleBundlerStub.resolves([{bundle: "I am a resource", sourceMap: "I am a source map"}]); mock("../../../../lib/processors/bundlers/moduleBundler", t.context.moduleBundlerStub); + t.context.taskUtil = { + getTag: sinon.stub().returns(false), + setTag: sinon.stub(), + clearTag: sinon.stub(), + STANDARD_TAGS: { + HasDebugVariant: "", + IsDebugVariant: "", + OmitFromBuildResult: "" + } + }; + // Re-require tested module generateStandaloneAppBundle = mock.reRequire("../../../../lib/tasks/bundlers/generateStandaloneAppBundle"); }); @@ -24,7 +35,7 @@ test.afterEach.always((t) => { function createDummyResource(id) { return { getPath: function() { - return "ponyPath" + id; + return "/resources/ponyPath" + id; } }; } @@ -48,7 +59,7 @@ test.serial("execute module bundler and write results", async (t) => { }; await generateStandaloneAppBundle(params); - t.deepEqual(t.context.moduleBundlerStub.callCount, 2, "moduleBundler should be called once"); + t.deepEqual(t.context.moduleBundlerStub.callCount, 2); const {resources, options} = t.context.moduleBundlerStub.getCall(0).args[0]; t.deepEqual(resources.length, 4, "moduleBundler got supplied with 4 resources"); @@ -94,7 +105,7 @@ test.serial("execute module bundler and write results without namespace", async }; await generateStandaloneAppBundle(params); - t.deepEqual(t.context.moduleBundlerStub.callCount, 2, "moduleBundler should be called once"); + t.deepEqual(t.context.moduleBundlerStub.callCount, 2); const {resources, options} = t.context.moduleBundlerStub.getCall(0).args[0]; t.deepEqual(resources.length, 4, "moduleBundler got supplied with 4 resources"); @@ -140,7 +151,7 @@ test.serial("execute module bundler and write results in evo mode", async (t) => }; await generateStandaloneAppBundle(params); - t.deepEqual(t.context.moduleBundlerStub.callCount, 2, "moduleBundler should be called once"); + t.deepEqual(t.context.moduleBundlerStub.callCount, 2); const {resources, options} = t.context.moduleBundlerStub.getCall(0).args[0]; t.deepEqual(resources.length, 4, "moduleBundler got supplied with 4 resources"); @@ -156,3 +167,136 @@ test.serial("execute module bundler and write results in evo mode", async (t) => "sap/ui/core/Core.js" ], "Correct filter in second bundle definition section"); }); + +test.serial("execute module bundler with taskUtil", async (t) => { + const {taskUtil} = t.context; + + const dummyResource1 = createDummyResource("1.js"); + const dummyResource2 = createDummyResource("2-dbg.js"); + const dummyResource3 = createDummyResource("3.js"); + const dummyResource4 = createDummyResource("4-dbg.js"); + + taskUtil.getTag.withArgs(dummyResource1, taskUtil.STANDARD_TAGS.HasDebugVariant).returns(true); + taskUtil.getTag.withArgs(dummyResource2.getPath(), taskUtil.STANDARD_TAGS.IsDebugVariant).returns(true); + + const ui5LoaderDummyResource = { + getPath: function() { + return "/resources/ui5loader.js"; // Triggers evo mode + } + }; + const dummyReaderWriter = { + _byGlob: async function() { + return [ + ui5LoaderDummyResource, + dummyResource1, + dummyResource2, + dummyResource3, + dummyResource4, + ]; + }, + write: function() {} + }; + sinon.stub(dummyReaderWriter, "write").resolves(); + const params = { + workspace: dummyReaderWriter, + dependencies: dummyReaderWriter, + taskUtil, + options: { + projectName: "some.project.name", + namespace: "some/project/namespace" + } + }; + await generateStandaloneAppBundle(params); + + t.is(t.context.moduleBundlerStub.callCount, 2); + + t.is(t.context.moduleBundlerStub.getCall(0).args.length, 1); + t.deepEqual(t.context.moduleBundlerStub.getCall(0).args[0].options, { + bundleDefinition: { + defaultFileTypes: [ + ".js", + ".control.xml", + ".fragment.html", + ".fragment.json", + ".fragment.xml", + ".view.html", + ".view.json", + ".view.xml", + ".properties", + ], + name: "sap-ui-custom.js", + sections: [ + { + declareModules: false, + filters: [ + "ui5loader-autoconfig.js", + ], + mode: "raw", + resolve: true, + sort: true, + }, + { + filters: [ + "some/project/namespace/", + "some/project/namespace/**/manifest.json", + "some/project/namespace/changes/changes-bundle.json", + "some/project/namespace/changes/flexibility-bundle.json", + "!some/project/namespace/test/", + "sap/ui/core/Core.js", + ], + mode: "preload", + renderer: true, + resolve: true, + resolveConditional: true, + }, + { + filters: [ + "sap/ui/core/Core.js", + ], + mode: "require", + }, + ], + } + }); + + t.is(t.context.moduleBundlerStub.getCall(1).args.length, 1); + t.deepEqual(t.context.moduleBundlerStub.getCall(1).args[0].options, { + bundleDefinition: { + defaultFileTypes: [ + ".js", + ".control.xml", + ".fragment.html", + ".fragment.json", + ".fragment.xml", + ".view.html", + ".view.json", + ".view.xml", + ".properties", + ], + name: "sap-ui-custom-dbg.js", + sections: [ + { + declareModules: false, + filters: [ + "ui5loader-autoconfig.js", + ], + mode: "raw", + resolve: true, + sort: true, + }, + { + filters: [ + "sap/ui/core/Core.js", + ], + mode: "require", + }, + ], + }, + bundleOptions: { + optimize: false + }, + moduleNameMapping: { + "/resources/ponyPath2-dbg.js": "ponyPath2.js" + } + }); +}); From d61655e3e169778aaaefa43d08c8f9fbda464a11 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 9 Mar 2022 14:02:53 +0100 Subject: [PATCH 11/13] [INTERNAL] Add tests for error handling --- .../tasks/bundlers/generateLibraryPreload.js | 49 +++++++++++++++++-- .../bundlers/generateStandaloneAppBundle.js | 32 ++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/test/lib/tasks/bundlers/generateLibraryPreload.js b/test/lib/tasks/bundlers/generateLibraryPreload.js index b28c78d45..30f87a6ae 100644 --- a/test/lib/tasks/bundlers/generateLibraryPreload.js +++ b/test/lib/tasks/bundlers/generateLibraryPreload.js @@ -18,10 +18,13 @@ test.beforeEach((t) => { t.context.dependencies = {}; t.context.comboByGlob = sinon.stub().resolves([]); + t.context.combo = { + byGlob: t.context.comboByGlob, + }; + t.context.combo.filter = sinon.stub().returns(t.context.combo); + t.context.ReaderCollectionPrioritizedStub = sinon.stub(); - t.context.ReaderCollectionPrioritizedStub.returns({ - byGlob: t.context.comboByGlob - }); + t.context.ReaderCollectionPrioritizedStub.returns(t.context.combo); mock("@ui5/fs", { ReaderCollectionPrioritized: t.context.ReaderCollectionPrioritizedStub }); @@ -1367,6 +1370,46 @@ test.serial("generateLibraryPreload for sap.ui.core with own bundle configuratio "ReaderCollectionPrioritized should have been called with 'new'"); }); +test.serial("Error: Failed to resolve non-debug name", async (t) => { + const { + generateLibraryPreload, + workspace, dependencies, comboByGlob + } = t.context; + const resources = [ + {getPath: sinon.stub().returns("/resources/resource-tagged-as-debug-variant.js")} + ]; + comboByGlob.resolves(resources); + + workspace.byGlob.resolves([ + {getPath: sinon.stub().returns("/resources/sap/ui/core/.library")} + ]); + + const taskUtil = { + getTag: sinon.stub().returns(false), + STANDARD_TAGS: { + HasDebugVariant: "", + IsDebugVariant: "", + OmitFromBuildResult: "" + } + }; + taskUtil.getTag + .withArgs("/resources/resource-tagged-as-debug-variant.js", taskUtil.STANDARD_TAGS.IsDebugVariant) + .returns(true); + + await t.throwsAsync(generateLibraryPreload({ + workspace, + dependencies, + taskUtil, + options: { + projectName: "sap.ui.core", + // Should be ignored for hardcoded sap.ui.core bundle configuration + excludes: ["sap/ui/core/**"] + } + }), { + message: "Failed to resolve non-debug name for /resources/resource-tagged-as-debug-variant.js" + }); +}); + test.serial("generateLibraryPreload with excludes", async (t) => { const { generateLibraryPreload, moduleBundlerStub, ReaderCollectionPrioritizedStub, diff --git a/test/lib/tasks/bundlers/generateStandaloneAppBundle.js b/test/lib/tasks/bundlers/generateStandaloneAppBundle.js index a24329633..3d70ee55a 100644 --- a/test/lib/tasks/bundlers/generateStandaloneAppBundle.js +++ b/test/lib/tasks/bundlers/generateStandaloneAppBundle.js @@ -300,3 +300,35 @@ test.serial("execute module bundler with taskUtil", async (t) => { } }); }); + +test.serial("Error: Failed to resolve non-debug name", async (t) => { + // NOTE: This scenario is not expected to happen as the "minify" task sets the IsDebugVariant tag + // only for resources that adhere to the debug file name pattern + + const {taskUtil} = t.context; + const dummyResource1 = createDummyResource("1.js"); + taskUtil.getTag.withArgs(dummyResource1.getPath(), taskUtil.STANDARD_TAGS.IsDebugVariant).returns(true); + + const dummyReaderWriter = { + _byGlob: async function() { + return [ + dummyResource1, + ]; + }, + write: function() {} + }; + sinon.stub(dummyReaderWriter, "write").resolves(); + const params = { + workspace: dummyReaderWriter, + dependencies: dummyReaderWriter, + taskUtil, + options: { + projectName: "some.project.name", + namespace: "some/project/namespace" + } + }; + + await t.throwsAsync(generateStandaloneAppBundle(params), { + message: "Failed to resolve non-debug name for /resources/ponyPath1.js" + }); +}); From a3bcc5be1ce6bf9a8094701536cdab143f33e7fe Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 9 Mar 2022 14:22:07 +0100 Subject: [PATCH 12/13] [INTERNAL] Refactor code to create moduleNameMapping --- lib/tasks/bundlers/generateBundle.js | 30 +++---------------- lib/tasks/bundlers/generateLibraryPreload.js | 24 ++++----------- .../bundlers/generateStandaloneAppBundle.js | 24 ++++----------- .../bundlers/utils/createModuleNameMapping.js | 30 +++++++++++++++++++ test/lib/tasks/bundlers/generateBundle.js | 12 +++----- .../tasks/bundlers/generateLibraryPreload.js | 18 ++++------- 6 files changed, 56 insertions(+), 82 deletions(-) create mode 100644 lib/tasks/bundlers/utils/createModuleNameMapping.js diff --git a/lib/tasks/bundlers/generateBundle.js b/lib/tasks/bundlers/generateBundle.js index b6922b47b..02b9040fd 100644 --- a/lib/tasks/bundlers/generateBundle.js +++ b/lib/tasks/bundlers/generateBundle.js @@ -1,5 +1,5 @@ const moduleBundler = require("../../processors/bundlers/moduleBundler"); -const ModuleName = require("../../lbt/utils/ModuleName"); +const createModuleNameMapping = require("./utils/createModuleNameMapping"); const ReaderCollectionPrioritized = require("@ui5/fs").ReaderCollectionPrioritized; /** @@ -82,33 +82,11 @@ module.exports = function({ } return combo.byGlob("/resources/**/*.{js,json,xml,html,properties,library,js.map}").then((resources) => { - const moduleNameMapping = {}; + const options = {bundleDefinition, bundleOptions}; if (!optimize && taskUtil) { - // For "unoptimized" bundles, the non-debug files have already been filtered out above. - // Now we need to create a mapping from the debug-variant resource path to the respective module name, - // which is basically the non-debug resource path, minus the "/resources/"" prefix. - // This mapping overwrites internal logic of the LocatorResourcePool which would otherwise determine - // the module name from the resource path, which would contain "-dbg" in this case. That would be - // incorrect since debug-variants should still keep the original module name. - for (let i = resources.length - 1; i >= 0; i--) { - const resourcePath = resources[i].getPath(); - if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) { - const nonDbgPath = ModuleName.getNonDebugName(resourcePath); - if (!nonDbgPath) { - throw new Error(`Failed to resolve non-debug name for ${resourcePath}`); - } - moduleNameMapping[resourcePath] = nonDbgPath.slice("/resources/".length); - } - } + options.moduleNameMapping = createModuleNameMapping({resources, taskUtil}); } - return moduleBundler({ - options: { - bundleDefinition, - bundleOptions, - moduleNameMapping - }, - resources - }).then((bundles) => { + return moduleBundler({options, resources}).then((bundles) => { return Promise.all(bundles.map(({bundle, sourceMap} = {}) => { if (!bundle) { // Skip empty bundles diff --git a/lib/tasks/bundlers/generateLibraryPreload.js b/lib/tasks/bundlers/generateLibraryPreload.js index 654aa65ca..0a027620a 100644 --- a/lib/tasks/bundlers/generateLibraryPreload.js +++ b/lib/tasks/bundlers/generateLibraryPreload.js @@ -2,7 +2,7 @@ const log = require("@ui5/logger").getLogger("builder:tasks:bundlers:generateLib const moduleBundler = require("../../processors/bundlers/moduleBundler"); const ReaderCollectionPrioritized = require("@ui5/fs").ReaderCollectionPrioritized; const {negateFilters} = require("../../lbt/resources/ResourceFilterList"); -const ModuleName = require("../../lbt/utils/ModuleName"); +const createModuleNameMapping = require("./utils/createModuleNameMapping"); function getDefaultLibraryPreloadFilters(namespace, excludes) { const filters = [ @@ -311,7 +311,7 @@ module.exports = function({workspace, dependencies, taskUtil, options: {projectN return resource.getPath() === "/resources/ui5loader.js"; }); - const unoptimizedModuleNameMapping = {}; + let unoptimizedModuleNameMapping; let unoptimizedResources = resources; if (taskUtil) { unoptimizedResources = await new ReaderCollectionPrioritized({ @@ -322,22 +322,10 @@ module.exports = function({workspace, dependencies, taskUtil, options: {projectN return !taskUtil.getTag(resource, taskUtil.STANDARD_TAGS.HasDebugVariant); }).byGlob("/**/*.{js,json,xml,html,properties,library,js.map}"); - // For "unoptimized" bundles, the non-debug files have already been filtered out above. - // Now we need to create a mapping from the debug-variant resource path to the respective module - // name, which is basically the non-debug resource path, minus the "/resources/"" prefix. - // This mapping overwrites internal logic of the LocatorResourcePool which would otherwise determine - // the module name from the resource path, which would contain "-dbg" in this case. That would be - // incorrect since debug-variants should still keep the original module name. - for (let i = unoptimizedResources.length - 1; i >= 0; i--) { - const resourcePath = unoptimizedResources[i].getPath(); - if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) { - const nonDbgPath = ModuleName.getNonDebugName(resourcePath); - if (!nonDbgPath) { - throw new Error(`Failed to resolve non-debug name for ${resourcePath}`); - } - unoptimizedModuleNameMapping[resourcePath] = nonDbgPath.slice("/resources/".length); - } - } + unoptimizedModuleNameMapping = createModuleNameMapping({ + resources: unoptimizedResources, + taskUtil + }); } let filters; diff --git a/lib/tasks/bundlers/generateStandaloneAppBundle.js b/lib/tasks/bundlers/generateStandaloneAppBundle.js index 7621c447c..59ad6c0c2 100644 --- a/lib/tasks/bundlers/generateStandaloneAppBundle.js +++ b/lib/tasks/bundlers/generateStandaloneAppBundle.js @@ -1,7 +1,7 @@ const log = require("@ui5/logger").getLogger("builder:tasks:bundlers:generateStandaloneAppBundle"); const ReaderCollectionPrioritized = require("@ui5/fs").ReaderCollectionPrioritized; const moduleBundler = require("../../processors/bundlers/moduleBundler"); -const ModuleName = require("../../lbt/utils/ModuleName"); +const createModuleNameMapping = require("./utils/createModuleNameMapping"); function getBundleDefinition(config) { const bundleDefinition = { @@ -101,7 +101,7 @@ module.exports = async function({workspace, dependencies, taskUtil, options: {pr filters = ["jquery.sap.global.js"]; } - const unoptimizedModuleNameMapping = {}; + let unoptimizedModuleNameMapping; let unoptimizedResources = resources; if (taskUtil) { unoptimizedResources = await new ReaderCollectionPrioritized({ @@ -112,22 +112,10 @@ module.exports = async function({workspace, dependencies, taskUtil, options: {pr return !taskUtil.getTag(resource, taskUtil.STANDARD_TAGS.HasDebugVariant); }).byGlob("/resources/**/*.{js,json,xml,html,properties,library,js.map}"); - // For "unoptimized" bundles, the non-debug files have already been filtered out above. - // Now we need to create a mapping from the debug-variant resource path to the respective module name, - // which is basically the non-debug resource path, minus the "/resources/"" prefix. - // This mapping overwrites internal logic of the LocatorResourcePool which would otherwise determine - // the module name from the resource path, which would contain "-dbg" in this case. That would be - // incorrect since debug-variants should still keep the original module name. - for (let i = unoptimizedResources.length - 1; i >= 0; i--) { - const resourcePath = unoptimizedResources[i].getPath(); - if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) { - const nonDbgPath = ModuleName.getNonDebugName(resourcePath); - if (!nonDbgPath) { - throw new Error(`Failed to resolve non-debug name for ${resourcePath}`); - } - unoptimizedModuleNameMapping[resourcePath] = nonDbgPath.slice("/resources/".length); - } - } + unoptimizedModuleNameMapping = createModuleNameMapping({ + resources: unoptimizedResources, + taskUtil + }); } await Promise.all([ diff --git a/lib/tasks/bundlers/utils/createModuleNameMapping.js b/lib/tasks/bundlers/utils/createModuleNameMapping.js new file mode 100644 index 000000000..23544ab37 --- /dev/null +++ b/lib/tasks/bundlers/utils/createModuleNameMapping.js @@ -0,0 +1,30 @@ +const ModuleName = require("../../../lbt/utils/ModuleName"); + +/** + * For "unoptimized" bundles, the non-debug files have already been filtered out above. + * Now we need to create a mapping from the debug-variant resource path to the respective module + * name, which is basically the non-debug resource path, minus the "/resources/"" prefix. + * This mapping overwrites internal logic of the LocatorResourcePool which would otherwise determine + * the module name from the resource path, which would contain "-dbg" in this case. That would be + * incorrect since debug-variants should still keep the original module name. + * + * @private + * @param {object} parameters Parameters + * @param {module:@ui5/fs.Resource[]} parameters.resources List of resources + * @param {module:@ui5/builder.tasks.TaskUtil|object} parameters.taskUtil TaskUtil + * @returns {object} Module name mapping + */ +module.exports = function({resources, taskUtil}) { + const moduleNameMapping = {}; + for (let i = resources.length - 1; i >= 0; i--) { + const resourcePath = resources[i].getPath(); + if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) { + const nonDbgPath = ModuleName.getNonDebugName(resourcePath); + if (!nonDbgPath) { + throw new Error(`Failed to resolve non-debug name for ${resourcePath}`); + } + moduleNameMapping[resourcePath] = nonDbgPath.slice("/resources/".length); + } + } + return moduleNameMapping; +}; diff --git a/test/lib/tasks/bundlers/generateBundle.js b/test/lib/tasks/bundlers/generateBundle.js index fb2619cdc..e29478769 100644 --- a/test/lib/tasks/bundlers/generateBundle.js +++ b/test/lib/tasks/bundlers/generateBundle.js @@ -83,8 +83,7 @@ test.serial("generateBundle: No taskUtil, no bundleOptions", async (t) => { t.deepEqual(moduleBundlerStub.getCall(0).args, [{ options: { bundleDefinition, - bundleOptions: undefined, - moduleNameMapping: {} + bundleOptions: undefined }, resources }]); @@ -156,8 +155,7 @@ test.serial("generateBundle: No bundleOptions, with taskUtil", async (t) => { t.deepEqual(moduleBundlerStub.getCall(0).args, [{ options: { bundleDefinition, - bundleOptions: undefined, - moduleNameMapping: {} + bundleOptions: undefined }, resources }]); @@ -396,8 +394,7 @@ test.serial("generateBundle: bundleOptions: sourceMap=false, with taskUtil", asy t.deepEqual(moduleBundlerStub.getCall(0).args, [{ options: { bundleDefinition, - bundleOptions, - moduleNameMapping: {} + bundleOptions }, resources }]); @@ -491,8 +488,7 @@ test.serial("generateBundle: Empty bundle (skipIfEmpty=true)", async (t) => { t.deepEqual(moduleBundlerStub.getCall(0).args, [{ options: { bundleDefinition, - bundleOptions, - moduleNameMapping: {} + bundleOptions }, resources }]); diff --git a/test/lib/tasks/bundlers/generateLibraryPreload.js b/test/lib/tasks/bundlers/generateLibraryPreload.js index 30f87a6ae..4130f7098 100644 --- a/test/lib/tasks/bundlers/generateLibraryPreload.js +++ b/test/lib/tasks/bundlers/generateLibraryPreload.js @@ -253,8 +253,7 @@ test.serial("generateLibraryPreload for sap.ui.core (w/o ui5loader.js)", async ( decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); @@ -345,8 +344,7 @@ test.serial("generateLibraryPreload for sap.ui.core (w/o ui5loader.js)", async ( decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); @@ -515,8 +513,7 @@ test.serial("generateLibraryPreload for sap.ui.core (/w ui5loader.js)", async (t decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); @@ -607,8 +604,7 @@ test.serial("generateLibraryPreload for sap.ui.core (/w ui5loader.js)", async (t decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); @@ -835,8 +831,7 @@ test.serial("generateLibraryPreload for sap.ui.core with old specVersion defined decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); @@ -927,8 +922,7 @@ test.serial("generateLibraryPreload for sap.ui.core with old specVersion defined decorateBootstrapModule: false, addTryCatchRestartWrapper: false, usePredefineCalls: false - }, - moduleNameMapping: {} + } }, resources }]); From 267fafc0da8ed36f859423cb1e2010b84c9fb11a Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 10 Mar 2022 12:01:29 +0100 Subject: [PATCH 13/13] [INTERNAL] minifier: Adopt default param handling --- lib/processors/minifier.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/processors/minifier.js b/lib/processors/minifier.js index 25a2b3575..bc1768ee9 100644 --- a/lib/processors/minifier.js +++ b/lib/processors/minifier.js @@ -40,10 +40,7 @@ const debugFileRegex = /((?:\.view|\.fragment|\.controller|\.designtime|\.suppor * @returns {Promise} * Promise resolving with object of resource, dbgResource and sourceMap */ -module.exports = async function({resources, options: {addSourceMappingUrl} = {}}) { - if (addSourceMappingUrl === undefined) { - addSourceMappingUrl = true; - } +module.exports = async function({resources, options: {addSourceMappingUrl = true} = {}}) { return Promise.all(resources.map(async (resource) => { const dbgPath = resource.getPath().replace(debugFileRegex, "-dbg$1"); const dbgResource = await resource.clone();