Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] JSModuleAnalyzer: Fix detection of bundle name #705

Merged
merged 4 commits into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ class JSModuleAnalyzer {
*/
let bIsUi5Module = false;

// Module name via @ui5-bundle comment in first line. Overrides all other main module candidates.
let firstLineBundleName;

// first analyze the whole AST...
visit(ast, false);

Expand All @@ -314,7 +317,12 @@ class JSModuleAnalyzer {
log.verbose(`bundle include directive ${subModule}`);
} else if ( comment.value.startsWith("@ui5-bundle ") ) {
const bundleName = comment.value.slice("@ui5-bundle ".length);
setMainModuleInfo(bundleName, null);
if (comment.start === 0) {
// Remember the name from the first line to use it as final name
firstLineBundleName = bundleName;
} else {
setMainModuleInfo(bundleName, null);
}
log.verbose(`bundle name directive ${bundleName}`);
} else {
log.warn(`unrecognized bundle directive ${comment.value}`);
Expand All @@ -324,7 +332,10 @@ class JSModuleAnalyzer {
}

// ...and finally take conclusions about the file's content
if ( !mainModuleFound ) {
if ( firstLineBundleName ) {
// If the first line has a bundle name, use it and override all other found names
info.name = firstLineBundleName;
} else if ( !mainModuleFound ) {
// if there's exactly one module definition in this file but it didn't
// immediately qualify as main module, make it now the main module
if ( candidateName != null && nModuleDeclarations == 1 ) {
Expand Down
75 changes: 75 additions & 0 deletions test/lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,78 @@ jQuery.sap.registerPreloadedModules({
t.deepEqual(info.subModules, ["foo/bar.js"],
"submodule from jQuery.sap.registerPreloadedModules");
});

test("Module that contains jQuery.sap.declare should be derived as subModule", (t) => {
const content = `
sap.ui.define([], function() {
jQuery.sap.declare("foo.bar");
});
`;
const info = analyzeString(content, "modules/module-with-jquery-sap-declare.js");
t.is(info.name, "modules/module-with-jquery-sap-declare.js");
t.is(info.rawModule, false);
t.is(info.format, "ui5-declare"); // FIXME: Format should actually be ui5-define
t.is(info.requiresTopLevelScope, false);
t.deepEqual(info.subModules, ["foo/bar.js"],
"jQuery.sap.declare subModule should be detected");
t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "Implicit dependency");
});

test("Bundle that contains jQuery.sap.declare (sap.ui.predefine) should not be derived as module name", (t) => {
const content = `//@ui5-bundle test1/library-preload.js
sap.ui.predefine("test1/module1", [], function() {
jQuery.sap.declare("foo.bar");
});
`;
const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js");
t.is(info.name, "test1/library-preload.js", "Module name should be taken from @ui5-bundle comment");
t.is(info.rawModule, false);
t.is(info.format, "ui5-declare"); // FIXME: Format should actually be ui5-define
t.is(info.requiresTopLevelScope, false);
// Note: foo/bar.js is not listed as the predefine body is not analyzed
t.deepEqual(info.subModules, ["test1/module1.js"],
"subModule via sap.ui.predefine should be detected");
t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "Implicit dependency");
});

test("Bundle that contains jQuery.sap.declare (sap.ui.require.preload) should not be derived as module name", (t) => {
const content = `//@ui5-bundle test1/library-preload.js
sap.ui.require.preload({
"test1/module1.js": function() {
sap.ui.define([], function() {
jQuery.sap.declare("foo.bar");
});
}
});

`;
const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js");
t.is(info.name, "test1/library-preload.js", "Module name should be taken from @ui5-bundle comment");
t.is(info.rawModule, false);
t.is(info.format, "ui5-define");
t.is(info.requiresTopLevelScope, false);
// Note: foo/bar.js is not listed as the sap.ui.define body is not analyzed
t.deepEqual(info.subModules, ["test1/module1.js"],
"subModule via sap.ui.predefine should be detected");
t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], "Implicit dependency");
});

test("@ui5-bundle comment: Multiple comments", (t) => {
const content = `//@ui5-bundle test/bundle1.js
//@ui5-bundle test/bundle2.js
`;
const info = analyzeString(content, "modules/ui5-bundle-comments.js");
t.is(info.name, "test/bundle1.js", "Comment from first line should be used");
t.deepEqual(info.subModules, []);
t.deepEqual(info.dependencies, []);
});

test("@ui5-bundle comment: Multiple comments (Not in first line)", (t) => {
const content = `console.log('Foo');
//@ui5-bundle test/bundle1.js
//@ui5-bundle test/bundle2.js
`;
t.throws(() => analyzeString(content, "modules/ui5-bundle-comments.js"), {
message: "conflicting main modules found (unnamed + named)"
});
});