From b9d397ed049ad3aa273c0efeed576a1a59fc6570 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Tue, 24 Nov 2020 19:37:40 +0100 Subject: [PATCH 1/5] [FIX] generateResourcesJson: Make resources.json generation deterministic Stop analyzing debug-resources if they correspond to a non-debug resource. Only analyze the non-debug resource and copy any dependency information to the corresponding debug-resource. While this might not be the best approach given that analyzing the source ("debug") version of a resource might result in finding more dependencies (i.e. those mangled during minification), this change ensures that we generate the same resources.json, independently from the order of debug and non-debug resources supplied to the resourceListCreator processor. Resolves https://github.com/SAP/ui5-tooling/issues/274 --- lib/lbt/resources/ResourceCollector.js | 45 ++++++++++++++++++-------- lib/lbt/resources/ResourceInfoList.js | 27 +--------------- lib/processors/resourceListCreator.js | 10 +++--- 3 files changed, 37 insertions(+), 45 deletions(-) diff --git a/lib/lbt/resources/ResourceCollector.js b/lib/lbt/resources/ResourceCollector.js index 9b6361a96..79790fb18 100644 --- a/lib/lbt/resources/ResourceCollector.js +++ b/lib/lbt/resources/ResourceCollector.js @@ -150,16 +150,25 @@ class ResourceCollector { }); } - async determineResourceDetails({pool, debugResources, mergedResources, designtimeResources, supportResources}) { + async determineResourceDetails({ + debugResources, mergedResources, designtimeResources, supportResources, debugBundles + }) { const baseNames = new Set(); const debugFilter = new ResourceFilterList(debugResources); const mergeFilter = new ResourceFilterList(mergedResources); const designtimeFilter = new ResourceFilterList(designtimeResources); const supportFilter = new ResourceFilterList(supportResources); + const debugBundleFilter = new ResourceFilterList(debugBundles); const promises = []; + const nonBundledDebugResources = []; for (const [name, info] of this._resources.entries()) { + if ( debugFilter.matches(name) ) { + info.isDebug = true; + log.verbose(` found potential debug resource '${name}'`); + } + // log.verbose(` checking ${name}`); let m; if ( m = LOCALE.exec(name) ) { @@ -180,9 +189,14 @@ class ResourceCollector { } if ( /(?:\.js|\.view\.xml|\.control\.xml|\.fragment\.xml)$/.test(name) ) { - promises.push( - this.enrichWithDependencyInfo(info) - ); + if ( (!info.isDebug || debugBundleFilter.matches(name)) ) { + // Only analyze non-debug files which are not special debug bundles (like sap-ui-core-dbg.js) + promises.push( + this.enrichWithDependencyInfo(info) + ); + } else { + nonBundledDebugResources.push(info); + } } // set the module name for .properties and .json @@ -197,11 +211,6 @@ class ResourceCollector { })); } - if ( debugFilter.matches(name) ) { - info.isDebug = true; - log.verbose(` found potential debug resource '${name}'`); - } - if ( mergeFilter.matches(name) ) { info.merged = true; log.verbose(` found potential merged resource '${name}'`); @@ -226,7 +235,17 @@ class ResourceCollector { } } - return Promise.all(promises); + await Promise.all(promises); + + for (let i = nonBundledDebugResources.length - 1; i >= 0; i--) { + const dbgInfo = nonBundledDebugResources[i]; + const nonDebugName = ResourceInfoList.getNonDebugName(dbgInfo.name); + const nonDbgInfo = this._resources.get(nonDebugName); + const newDbgInfo = new ResourceInfo(dbgInfo.name); + newDbgInfo.copyFrom(null, nonDbgInfo); + newDbgInfo.copyFrom(null, dbgInfo); + this._resources.set(dbgInfo.name, newDbgInfo); + } } createOrphanFilters() { @@ -253,19 +272,17 @@ class ResourceCollector { groupResourcesByComponents(options) { const orphanFilters = this.createOrphanFilters(); - const debugBundlesFilter = new ResourceFilterList(options.debugBundles); for (const resource of this._resources.values()) { let contained = false; for (const [prefix, list] of this._components.entries()) { - const isDebugBundle = debugBundlesFilter.matches(resource.name); if ( resource.name.startsWith(prefix) ) { - list.add(resource, !isDebugBundle); + list.add(resource); contained = true; } else if ( orphanFilters.has(prefix) ) { // log.verbose(` checking '${resource.name}' against orphan filter ` + // `'${orphanFilters.get(prefix)}' (${prefix})`); if ( orphanFilters.get(prefix).matches(resource.name) ) { - list.add(resource, !isDebugBundle); + list.add(resource); contained = true; } } diff --git a/lib/lbt/resources/ResourceInfoList.js b/lib/lbt/resources/ResourceInfoList.js index af5b655ea..2a0413898 100644 --- a/lib/lbt/resources/ResourceInfoList.js +++ b/lib/lbt/resources/ResourceInfoList.js @@ -39,37 +39,12 @@ class ResourceInfoList { * Add ResourceInfo to list * * @param {ResourceInfo} info - * @param {boolean} shareDebugInformation */ - add(info, shareDebugInformation=true) { + add(info) { const relativeName = ResourceInfoList.makePathRelativeTo(this.name, info.name); - // search for a resource with the same name let myInfo = this.resourcesByName.get(relativeName); - if ( myInfo == null && shareDebugInformation) { - // when not found, check if the given resource is a debug resource and - // share the information with the non-dbg version - const nonDbgName = ResourceInfoList.getNonDebugName(relativeName); - const dbgName = ResourceInfoList.getDebugName(relativeName); - if ( nonDbgName != null && this.resourcesByName.has(nonDbgName) ) { - // copy from source - myInfo = new ResourceInfo(relativeName); - const source = this.resourcesByName.get(nonDbgName); - myInfo.copyFrom(this.name, source); - this.resources.push(myInfo); - this.resourcesByName.set(relativeName, myInfo); - } else if (dbgName != null && this.resourcesByName.has(dbgName)) { - // copy from debug - myInfo = new ResourceInfo(relativeName); - const source = this.resourcesByName.get(dbgName); - myInfo.copyFrom(this.name, source); - myInfo.module = ResourceInfoList.getNonDebugName(source.module); - this.resources.push(myInfo); - this.resourcesByName.set(relativeName, myInfo); - } - } - // this is the assumption, that the debug one is the same as the non-dbg one if ( myInfo == null ) { myInfo = new ResourceInfo(relativeName); diff --git a/lib/processors/resourceListCreator.js b/lib/processors/resourceListCreator.js index 0ac67e1e6..11645fc17 100644 --- a/lib/processors/resourceListCreator.js +++ b/lib/processors/resourceListCreator.js @@ -73,7 +73,8 @@ const DEFAULT_SUPPORT_RESOURCES_FILTER = [ * @type {string[]} */ const DEBUG_BUNDLES = [ - "sap-ui-core-dbg.js" + "sap-ui-core-dbg.js", + "sap-ui-core-nojQuery-dbg.js" ]; /** @@ -159,13 +160,12 @@ module.exports = async function({resources, options}) { debugResources: options.debugResources, mergedResources: options.mergedResources, designtimeResources: options.designtimeResources, - supportResources: options.supportResources + supportResources: options.supportResources, + debugBundles: options.debugBundles }); // group resources by components and create ResourceInfoLists - collector.groupResourcesByComponents({ - debugBundles: options.debugBundles - }); + collector.groupResourcesByComponents(); const resourceLists = []; From 7ec38a489244e47643ac3abf978e2919863034c7 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Tue, 24 Nov 2020 19:45:58 +0100 Subject: [PATCH 2/5] [INTERNAL] ResourceCollector: Fix existing tests --- test/lib/lbt/resources/ResourceCollector.js | 6 ++--- test/lib/lbt/resources/ResourceInfoList.js | 26 +++++++-------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/test/lib/lbt/resources/ResourceCollector.js b/test/lib/lbt/resources/ResourceCollector.js index 6e5b46153..97cc856c5 100644 --- a/test/lib/lbt/resources/ResourceCollector.js +++ b/test/lib/lbt/resources/ResourceCollector.js @@ -80,7 +80,7 @@ test.serial("groupResourcesByComponents: debugBundles", async (t) => { }); await resourceCollector.visitResource({getPath: () => "/resources/testcomp/Component.js", getSize: async () => 13}); await resourceCollector.visitResource({getPath: () => "/resources/my/file.js", getSize: async () => 13}); - resourceCollector.groupResourcesByComponents({debugBundles: [".*-dbg.js"]}); + resourceCollector.groupResourcesByComponents(); t.is(resourceCollector.resources.size, 0, "all resources were deleted"); }); @@ -89,7 +89,7 @@ test.serial("groupResourcesByComponents: theme", async (t) => { await resourceCollector.visitResource({getPath: () => "/resources/themes/a/.theming", getSize: async () => 13}); t.is(resourceCollector.themePackages.size, 1, "1 theme was added"); await resourceCollector.determineResourceDetails({}); - resourceCollector.groupResourcesByComponents({}); + resourceCollector.groupResourcesByComponents(); t.is(resourceCollector.themePackages.get("themes/a/").resources.length, 1, "1 theme was grouped"); }); @@ -111,7 +111,7 @@ test.serial("determineResourceDetails: properties", async (t) => { getPath: () => "/resources/mylib/i18n/i18n.properties", getSize: async () => 13 }); await resourceCollector.determineResourceDetails({}); - resourceCollector.groupResourcesByComponents({}); + resourceCollector.groupResourcesByComponents(); const resources = resourceCollector.components.get("mylib/").resources; t.deepEqual(resources.map((res) => res.i18nName), [null, "i18n/i18n.properties", "i18n/i18n.properties"], "i18nName was set"); diff --git a/test/lib/lbt/resources/ResourceInfoList.js b/test/lib/lbt/resources/ResourceInfoList.js index 8a0be81af..a11d19ac1 100644 --- a/test/lib/lbt/resources/ResourceInfoList.js +++ b/test/lib/lbt/resources/ResourceInfoList.js @@ -16,38 +16,28 @@ test("add: add new resources", (t) => { t.falsy(result.module, "module is not set"); }); -test("add: add source then debug resources", (t) => { +test("add: add non-debug resource", (t) => { const resourceInfoList = new ResourceInfoList("prefix"); - resourceInfoList.add({name: "myfile.js", module: "myfile.js", size: 13}); + resourceInfoList.add({name: "myfile.js", module: "myfile.js", size: 13, required: new Set(["some-dep.js"])}); - const myInfo = {name: "myfile-dbg.js", size: 13}; - resourceInfoList.add(myInfo); - - t.is(resourceInfoList.resources.length, 2, "two entries"); + t.is(resourceInfoList.resources.length, 1, "one resource added"); const result = resourceInfoList.resourcesByName.get("../myfile.js"); t.is(result.module, "myfile.js", "module is set"); - - const resultDbg = resourceInfoList.resourcesByName.get("../myfile-dbg.js"); - t.is(resultDbg.module, "myfile.js", "module is set"); + t.deepEqual(result.required, new Set(["some-dep.js"]), "module is set"); }); -test("add: add debug then source resources", (t) => { +test("add: add debug resources", (t) => { const resourceInfoList = new ResourceInfoList("prefix"); - resourceInfoList.add({name: "myfile-dbg.js", size: 13}); + resourceInfoList.add({name: "myfile-dbg.js", size: 13, required: new Set(["some-dep.js"])}); - const myInfo = {name: "myfile.js", module: "myfile.js", size: 13}; - resourceInfoList.add(myInfo); - - t.is(resourceInfoList.resources.length, 2, "two entries"); - - const result = resourceInfoList.resourcesByName.get("../myfile.js"); - t.is(result.module, "myfile.js", "module is set"); + t.is(resourceInfoList.resources.length, 1, "one resource added"); const resultDbg = resourceInfoList.resourcesByName.get("../myfile-dbg.js"); t.is(resultDbg.module, "myfile.js", "module is set"); + t.deepEqual(resultDbg.required, new Set(["some-dep.js"]), "module is set"); }); test("add: add i18n resource", (t) => { From 40a90327dcd84fa9fa106e41c8cc1d928042beaf Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Thu, 26 Nov 2020 17:06:19 +0100 Subject: [PATCH 3/5] [INTERNAL] ResourceCollector: Add tests for handling of dbg files and -bundles --- test/lib/lbt/resources/ResourceCollector.js | 74 +++++++++++++++++++-- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/test/lib/lbt/resources/ResourceCollector.js b/test/lib/lbt/resources/ResourceCollector.js index 97cc856c5..9d213ea26 100644 --- a/test/lib/lbt/resources/ResourceCollector.js +++ b/test/lib/lbt/resources/ResourceCollector.js @@ -118,13 +118,7 @@ test.serial("determineResourceDetails: properties", async (t) => { }); test.serial("determineResourceDetails: view.xml", async (t) => { - const resourceCollector = new ResourceCollector({ - getModuleInfo: async (moduleInfo) => { - return { - name: "myName" - }; - } - }); + const resourceCollector = new ResourceCollector(); const enrichWithDependencyInfoStub = sinon.stub(resourceCollector, "enrichWithDependencyInfo") .returns(Promise.resolve()); await resourceCollector.visitResource({getPath: () => "/resources/mylib/my.view.xml", getSize: async () => 13}); @@ -133,6 +127,72 @@ test.serial("determineResourceDetails: view.xml", async (t) => { t.is(enrichWithDependencyInfoStub.getCall(0).args[0].name, "mylib/my.view.xml", "is called with view"); }); +test.serial("determineResourceDetails: Debug bundle", async (t) => { + const resourceCollector = new ResourceCollector(); + + const enrichWithDependencyInfoStub = sinon.stub(resourceCollector, "enrichWithDependencyInfo").resolves(); + await resourceCollector.visitResource({getPath: () => "/resources/MyBundle-dbg.js", getSize: async () => 13}); + + await resourceCollector.determineResourceDetails({ + debugBundles: ["MyBundle-dbg.js"] + }); + t.is(enrichWithDependencyInfoStub.callCount, 1, "enrichWithDependencyInfo is called once"); + t.is(enrichWithDependencyInfoStub.getCall(0).args[0].name, "MyBundle-dbg.js", + "enrichWithDependencyInfo is called with debug bundle"); +}); + +test.serial("determineResourceDetails: Debug files and non-debug files", async (t) => { + const resourceCollector = new ResourceCollector(); + + const enrichWithDependencyInfoStub = sinon.stub(resourceCollector, "enrichWithDependencyInfo") + .callsFake((resourceInfo) => { + // Simulate enriching resource info with dependency info to test whether it gets shared + // with the dbg resource alter on + resourceInfo.dynRequired = true; + }); + await Promise.all([ + "/resources/MyBundle-dbg.js", + "/resources/mylib/MyControlA-dbg.js", + "/resources/mylib/MyControlA.js", + "/resources/mylib/MyControlB.js", + "/resources/mylib/MyControlB-dbg.js" + ].map((resourcePath) => { + return resourceCollector.visitResource({getPath: () => resourcePath, getSize: async () => 13}); + })); + + await resourceCollector.determineResourceDetails({ + debugResources: ["**/*-dbg.js"], + debugBundles: ["MyBundle-dbg.js"] + }); + t.is(enrichWithDependencyInfoStub.callCount, 3, "enrichWithDependencyInfo is called three times"); + t.is(enrichWithDependencyInfoStub.getCall(0).args[0].name, "MyBundle-dbg.js", + "enrichWithDependencyInfo called with debug bundle"); + t.is(enrichWithDependencyInfoStub.getCall(1).args[0].name, "mylib/MyControlA.js", + "enrichWithDependencyInfo called with non-debug control A"); + t.is(enrichWithDependencyInfoStub.getCall(2).args[0].name, "mylib/MyControlB.js", + "enrichWithDependencyInfo called with non-debug control B"); + + t.is(resourceCollector._resources.get("MyBundle-dbg.js").isDebug, true, "MyBundle-dbg is a debug file"); + t.is(resourceCollector._resources.get("MyBundle-dbg.js").dynRequired, true, + "MyBundle-dbg is flagged as dynRequired"); + + t.is(resourceCollector._resources.get("mylib/MyControlA.js").isDebug, false, "MyControlA is no debug file"); + t.is(resourceCollector._resources.get("mylib/MyControlA.js").dynRequired, true, + "MyControlA is flagged as dynRequired"); + + t.is(resourceCollector._resources.get("mylib/MyControlA-dbg.js").isDebug, true, "MyControlA-dbg is a debug file"); + t.is(resourceCollector._resources.get("mylib/MyControlA-dbg.js").dynRequired, true, + "MyControlA-dbg is flagged as dynRequired"); + + t.is(resourceCollector._resources.get("mylib/MyControlB.js").isDebug, false, "MyControlB is no debug file"); + t.is(resourceCollector._resources.get("mylib/MyControlB.js").dynRequired, true, + "MyControlB is flagged as dynRequired"); + + t.is(resourceCollector._resources.get("mylib/MyControlB-dbg.js").isDebug, true, "MyControlB-dbg is a debug file"); + t.is(resourceCollector._resources.get("mylib/MyControlB-dbg.js").dynRequired, true, + "MyControlB-dbg is flagged as dynRequired"); +}); + test.serial("enrichWithDependencyInfo: add infos to resourceinfo", async (t) => { const resourceCollector = new ResourceCollector({ getModuleInfo: async () => { From 6bbc8e9dec272787eb09f8d39ed04838d85bb47f Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Thu, 26 Nov 2020 17:21:29 +0100 Subject: [PATCH 4/5] [INTERNAL] ResourceInfoList: Remove obsolete function getDebugName --- lib/lbt/resources/ResourceInfoList.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/lbt/resources/ResourceInfoList.js b/lib/lbt/resources/ResourceInfoList.js index 2a0413898..1a87b2f8e 100644 --- a/lib/lbt/resources/ResourceInfoList.js +++ b/lib/lbt/resources/ResourceInfoList.js @@ -1,7 +1,6 @@ const ResourceInfo = require("./ResourceInfo"); const DEBUG_RESOURCES_PATTERN = /-dbg((?:\.view|\.fragment|\.controller|\.designtime|\.support)?\.js|\.css)$/; -const RESOURCES_PATTERN = /((?:\.view|\.fragment|\.controller|\.designtime|\.support)?\.js|\.css)$/; /** * A list of ResourceInfo objects, suitable for (but not dependent on) JSON serialization. @@ -114,15 +113,6 @@ class ResourceInfoList { } return null; } - - static getDebugName(path) { - if ( RESOURCES_PATTERN.test(path) ) { - if (!path.replace(RESOURCES_PATTERN, "").endsWith("-dbg")) { - return path.replace( RESOURCES_PATTERN, "-dbg$1"); - } - } - return null; - } } module.exports = ResourceInfoList; From 44af975f2382519209f2d3bd508404960c7292e9 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 30 Nov 2020 13:44:45 +0100 Subject: [PATCH 5/5] [INTERNAL] Apply suggestions from code review --- lib/lbt/resources/ResourceCollector.js | 8 ++++++-- test/lib/lbt/resources/ResourceCollector.js | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/lbt/resources/ResourceCollector.js b/lib/lbt/resources/ResourceCollector.js index 79790fb18..5eb6095ad 100644 --- a/lib/lbt/resources/ResourceCollector.js +++ b/lib/lbt/resources/ResourceCollector.js @@ -190,7 +190,7 @@ class ResourceCollector { if ( /(?:\.js|\.view\.xml|\.control\.xml|\.fragment\.xml)$/.test(name) ) { if ( (!info.isDebug || debugBundleFilter.matches(name)) ) { - // Only analyze non-debug files which are not special debug bundles (like sap-ui-core-dbg.js) + // Only analyze non-debug files and special debug bundles (like sap-ui-core-dbg.js) promises.push( this.enrichWithDependencyInfo(info) ); @@ -242,8 +242,12 @@ class ResourceCollector { const nonDebugName = ResourceInfoList.getNonDebugName(dbgInfo.name); const nonDbgInfo = this._resources.get(nonDebugName); const newDbgInfo = new ResourceInfo(dbgInfo.name); + + // First copy info of analysis from non-dbg file (included, required, condRequired, ...) newDbgInfo.copyFrom(null, nonDbgInfo); + // Then copy over info from dbg file to properly set name, isDebug, etc. newDbgInfo.copyFrom(null, dbgInfo); + this._resources.set(dbgInfo.name, newDbgInfo); } } @@ -270,7 +274,7 @@ class ResourceCollector { return filtersByComponent; } - groupResourcesByComponents(options) { + groupResourcesByComponents() { const orphanFilters = this.createOrphanFilters(); for (const resource of this._resources.values()) { let contained = false; diff --git a/test/lib/lbt/resources/ResourceCollector.js b/test/lib/lbt/resources/ResourceCollector.js index 9d213ea26..ae27afa81 100644 --- a/test/lib/lbt/resources/ResourceCollector.js +++ b/test/lib/lbt/resources/ResourceCollector.js @@ -147,7 +147,7 @@ test.serial("determineResourceDetails: Debug files and non-debug files", async ( const enrichWithDependencyInfoStub = sinon.stub(resourceCollector, "enrichWithDependencyInfo") .callsFake((resourceInfo) => { // Simulate enriching resource info with dependency info to test whether it gets shared - // with the dbg resource alter on + // with the dbg resource later on resourceInfo.dynRequired = true; }); await Promise.all([