Skip to content

Commit

Permalink
[FIX] Don't report a missing module that's contained in another bundle (
Browse files Browse the repository at this point in the history
  • Loading branch information
codeworrior committed Jul 12, 2023
1 parent 41fa2d6 commit 8f23f38
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 11 deletions.
61 changes: 50 additions & 11 deletions lib/lbt/bundle/Resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -71,19 +81,28 @@ 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(
resource.info.subModules.map((sub) => pool.findResource(sub).catch(() => false))
).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(($) => ($))
};
});
}

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

Expand Down Expand Up @@ -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;
Expand Down
72 changes: 72 additions & 0 deletions test/lib/lbt/bundle/Resolver.js
Original file line number Diff line number Diff line change
@@ -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";

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

0 comments on commit 8f23f38

Please sign in to comment.