From 8f23f388f6d64f313ae8a89d0fcaf39ba905a70b Mon Sep 17 00:00:00 2001 From: Frank Weigel Date: Wed, 12 Jul 2023 21:24:59 +0200 Subject: [PATCH] [FIX] Don't report a missing module that's contained in another bundle (#915) --- lib/lbt/bundle/Resolver.js | 61 +++++++++++++++++++++++----- test/lib/lbt/bundle/Resolver.js | 72 +++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 11 deletions(-) diff --git a/lib/lbt/bundle/Resolver.js b/lib/lbt/bundle/Resolver.js index fd5ecf927..e14c40c7e 100644 --- a/lib/lbt/bundle/Resolver.js +++ b/lib/lbt/bundle/Resolver.js @@ -51,6 +51,16 @@ class BundleResolver { let selectedResources = Object.create(null); let selectedResourcesSequence = []; const pool = this.pool; + /** + * Names of modules that are required in some way but could not be found + * in the resource pool. + */ + const missingModules = Object.create(null); + /** + * Names of modules that are included in non-decomposable bundles. + * If they occur in the missingModules, then this is not an error. + */ + const includedModules = new Set(); /** * @param {JSModuleSectionDefinition} section @@ -71,11 +81,16 @@ class BundleResolver { } function checkForDecomposableBundle(resource) { - if ( resource == null || - resource.info == null || - resource.info.subModules.length === 0 || - /(?:^|\/)library.js$/.test(resource.info.name) ) { - return {resource, decomposable: false}; + const isBundle = + resource?.info?.subModules.length > 0 && + !/(?:^|\/)library.js$/.test(resource.info.name); + + if (!isBundle) { + return { + resource, + isBundle, + decomposable: false + }; } return Promise.all( @@ -83,7 +98,11 @@ class BundleResolver { ).then((modules) => { // it might look more natural to expect 'all' embedded modules to exist in the pool, // but expecting only 'some' module to exist is a more conservative approach - return ({resource, decomposable: modules.some(($) => ($))}); + return { + resource, + isBundle, + decomposable: modules.some(($) => ($)) + }; }); } @@ -105,20 +124,29 @@ class BundleResolver { .catch( (err) => { // if the caller provided an error message, log it if ( msg ) { - log.error(msg); + missingModules[resourceName] ??= []; + missingModules[resourceName].push(msg); } // return undefined }) .then( (resource) => checkForDecomposableBundle(resource) ) - .then( ({resource, decomposable}) => { + .then( ({resource, isBundle, decomposable}) => { const dependencyInfo = resource && resource.info; let promises = []; + if ( isBundle && !decomposable ) { + resource.info.subModules.forEach( + (included) => { + includedModules.add(included); + } + ); + } + if ( decomposable ) { // bundles are not added, only their embedded modules promises = dependencyInfo.subModules.map( (included) => { return checkAndAddResource(included, depth + 1, - "**** error: missing submodule " + included + ", included by " + resourceName); + `**** error: missing submodule ${included}, included by ${resourceName}`); }); } else if ( resource != null ) { // trace.trace(" checking dependencies of " + resource.name ); @@ -136,8 +164,8 @@ class BundleResolver { return; } - return checkAndAddResource( required, depth + 1, - "**** error: missing module " + required + ", required by " + resourceName); + return checkAndAddResource(required, depth + 1, + `**** error: missing module ${required}, required by ${resourceName}`); }); } @@ -317,6 +345,17 @@ class BundleResolver { // NODE-TODO if ( PerfMeasurement.ACTIVE ) PerfMeasurement.stop(PerfKeys.RESOLVE_MODULE); return previous.then( function() { + // ignore missing modules that have been found in non-decomposable bundles + includedModules.forEach((included) => delete missingModules[included]); + + // report the remaining missing modules + Object.keys(missingModules).sort().forEach((missing) => { + const messages = missingModules[missing]; + messages.sort().forEach((msg) => { + log.error(msg); + }); + }); + log.verbose(" Resolving bundle done"); return resolved; diff --git a/test/lib/lbt/bundle/Resolver.js b/test/lib/lbt/bundle/Resolver.js index 01551bea6..31bf7bc5f 100644 --- a/test/lib/lbt/bundle/Resolver.js +++ b/test/lib/lbt/bundle/Resolver.js @@ -1,4 +1,7 @@ import test from "ava"; +import sinon from "sinon"; +import esmock from "esmock"; + import Resolver from "../../../../lib/lbt/bundle/Resolver.js"; import ResourcePool from "../../../../lib/lbt/resources/ResourcePool.js"; @@ -229,3 +232,72 @@ test.serial("embedd a non-decomposable bundle", async (t) => { "vendor/non-decomposable-bundle.js" ], "new bundle should contain the non-decomposable bundle"); }); + +test.serial("no errors for dependencies between non-decomposable bundles", async (t) => { + const errorLogStub = sinon.stub(); + const myLoggerInstance = { + error: errorLogStub, + silly: sinon.stub(), + verbose: sinon.stub() + }; + const ResolverWithStub = await esmock("../../../../lib/lbt/bundle/Resolver", { + "@ui5/logger": { + getLogger: () => myLoggerInstance + } + }); + + const pool = new MockPool({ + "lib/mod1.js": TRIVIAL_MODULE, + "lib/mod2.js": TRIVIAL_MODULE, + "lib/mod3.js": "sap.ui.define(['lib/mod4'], function() {});", + "lib/mod4.js": TRIVIAL_MODULE, + "vendor/non-decomposable-bundle1.js": ` + define("external1/mod1", function() {}); + define("external1/mod2", function() {}); + define("external1/mod3", function() {}); + define("external1/mod4", function() {});`, + "vendor/non-decomposable-bundle2.js": ` + define("external2/mod1", ["lib/mod1"], function() {}); // exists in pool + define("external2/mod2", ["external1/mod1"], function() {}); // exists in previous bundle + define("external2/mod3", ["external2/mod2"], function() {}); // exists in this bundle + define("external2/mod4", ["external3/mod3"], function() {}); // exists in next bundle + define("external2/mod5", ["external4/mod1"], function() {}); // missing`, + "vendor/non-decomposable-bundle3.js": ` + define("external3/mod1", function() {}); + define("external3/mod2", function() {}); + define("external3/mod3", function() {});` + }); + + const bundleDefinition = { + name: "bundle.js", + sections: [ + { + mode: "preload", + filters: [ + "vendor/" + ], + resolve: true + } + ] + }; + + const resolver = new ResolverWithStub(pool); + + const resolvedBundle = await resolver.resolve(bundleDefinition); + + t.true(resolvedBundle != null, "resolve() should return a bundle"); + t.is(resolvedBundle.sections.length, 1, "bundle should contain 1 section"); + t.deepEqual( + sortedCopy(resolvedBundle.sections[0].modules), + [ + "lib/mod1.js", + "vendor/non-decomposable-bundle1.js", + "vendor/non-decomposable-bundle2.js", + "vendor/non-decomposable-bundle3.js" + ], "new bundle should contain the non-decomposable bundle"); + + t.is(errorLogStub.callCount, 1, "One error reported"); + t.is(errorLogStub.firstCall.args[0], + "**** error: missing module external4/mod1.js, required by vendor/non-decomposable-bundle2.js", + "only the expected missing module is reported"); +});