From 2daf76d5143d163985f27b0cba29b86c83afbd66 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 17 Feb 2022 14:02:11 +0100 Subject: [PATCH 1/4] WIP: Add failing tests --- test/lib/lbt/analyzer/JSModuleAnalyzer.js | 57 +++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js index 10b5da6e5..e6ee64756 100644 --- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -684,3 +684,60 @@ jQuery.sap.registerPreloadedModules({ t.deepEqual(info.subModules, ["foo/bar.js"], "submodule from jQuery.sap.registerPreloadedModules"); }); + +test("Module that contains jQuery.sap.declare should not 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", "TBD"); + t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "TBD"); + t.is(info.rawModule, false, "TBD"); + // t.is(info.format, "ui5-define", "TBD"); + t.is(info.requiresTopLevelScope, false, "TBD"); + t.deepEqual(info.subModules, [], + "no subModules should be detected"); +}); + +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", + "TBD"); + t.is(info.rawModule, false, "TBD"); + // t.is(info.format, "ui5-declare", "TBD"); + t.is(info.requiresTopLevelScope, false, "TBD"); + t.deepEqual(info.subModules, ["test1/module1.js"], + "subModule via sap.ui.predefine should be detected"); + t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], + "TBD"); +}); + +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", + "TBD"); + t.is(info.rawModule, false, "TBD"); + // t.is(info.format, "ui5-declare", "TBD"); + t.is(info.requiresTopLevelScope, false, "TBD"); + t.deepEqual(info.subModules, ["test1/module1.js"], + "subModule via sap.ui.predefine should be detected"); + t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], + "TBD"); +}); From 57bcf31626baf4efc660c9d1fee109139bcf100e Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 21 Apr 2022 11:22:36 +0200 Subject: [PATCH 2/4] Fix + update tests --- lib/lbt/analyzer/JSModuleAnalyzer.js | 15 +++++++++++++-- test/lib/lbt/analyzer/JSModuleAnalyzer.js | 18 +++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/lbt/analyzer/JSModuleAnalyzer.js b/lib/lbt/analyzer/JSModuleAnalyzer.js index da1c494ad..c7053b3b0 100644 --- a/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -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); @@ -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}`); @@ -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 ) { diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js index e6ee64756..7da807f94 100644 --- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -685,7 +685,7 @@ jQuery.sap.registerPreloadedModules({ "submodule from jQuery.sap.registerPreloadedModules"); }); -test("Module that contains jQuery.sap.declare should not be derived as subModule", (t) => { +test("Module that contains jQuery.sap.declare should be derived as subModule", (t) => { const content = ` sap.ui.define([], function() { jQuery.sap.declare("foo.bar"); @@ -695,10 +695,10 @@ sap.ui.define([], function() { t.is(info.name, "modules/module-with-jquery-sap-declare.js", "TBD"); t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "TBD"); t.is(info.rawModule, false, "TBD"); - // t.is(info.format, "ui5-define", "TBD"); + t.is(info.format, "ui5-declare", "TBD"); // Shouldn't this be ui5-define? t.is(info.requiresTopLevelScope, false, "TBD"); - t.deepEqual(info.subModules, [], - "no subModules should be detected"); + t.deepEqual(info.subModules, ["foo/bar.js"], + "jQuery.sap.declare subModule should be detected"); }); test("Bundle that contains jQuery.sap.declare (sap.ui.predefine) should not be derived as module name", (t) => { @@ -711,11 +711,11 @@ sap.ui.predefine("test1/module1", [], function() { t.is(info.name, "test1/library-preload.js", "TBD"); t.is(info.rawModule, false, "TBD"); - // t.is(info.format, "ui5-declare", "TBD"); + t.is(info.format, "ui5-declare", "TBD"); // Shouldn't this be ui5-define? t.is(info.requiresTopLevelScope, false, "TBD"); - t.deepEqual(info.subModules, ["test1/module1.js"], + t.deepEqual(info.subModules, ["test1/module1.js"], // Shouldn't foo/bar.js be listed here? "subModule via sap.ui.predefine should be detected"); - t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], + t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "TBD"); }); @@ -734,9 +734,9 @@ sap.ui.require.preload({ t.is(info.name, "test1/library-preload.js", "TBD"); t.is(info.rawModule, false, "TBD"); - // t.is(info.format, "ui5-declare", "TBD"); + t.is(info.format, "ui5-define", "TBD"); t.is(info.requiresTopLevelScope, false, "TBD"); - t.deepEqual(info.subModules, ["test1/module1.js"], + t.deepEqual(info.subModules, ["test1/module1.js"], // Shouldn't foo/bar.js be listed here? "subModule via sap.ui.predefine should be detected"); t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], "TBD"); From a6ac5e65589dc72afcd289dd07d1987dc9ce7172 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 22 Apr 2022 12:52:40 +0200 Subject: [PATCH 3/4] Update and enhance tests, clarify TBDs --- test/lib/lbt/analyzer/JSModuleAnalyzer.js | 60 +++++++++++++++-------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js index 7da807f94..bfe7a2d55 100644 --- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -692,13 +692,13 @@ sap.ui.define([], function() { }); `; const info = analyzeString(content, "modules/module-with-jquery-sap-declare.js"); - t.is(info.name, "modules/module-with-jquery-sap-declare.js", "TBD"); - t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "TBD"); - t.is(info.rawModule, false, "TBD"); - t.is(info.format, "ui5-declare", "TBD"); // Shouldn't this be ui5-define? - t.is(info.requiresTopLevelScope, false, "TBD"); + t.is(info.name, "modules/module-with-jquery-sap-declare.js"); + t.is(info.rawModule, false); + t.is(info.format, "ui5-declare", "Format should be declare once a jQuery.sap.declare statement is found"); + 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) => { @@ -708,15 +708,14 @@ sap.ui.predefine("test1/module1", [], function() { }); `; const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js"); - t.is(info.name, "test1/library-preload.js", - "TBD"); - t.is(info.rawModule, false, "TBD"); - t.is(info.format, "ui5-declare", "TBD"); // Shouldn't this be ui5-define? - t.is(info.requiresTopLevelScope, false, "TBD"); - t.deepEqual(info.subModules, ["test1/module1.js"], // Shouldn't foo/bar.js be listed here? + 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", "Format should be declare once a jQuery.sap.declare statement is found"); + 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"], - "TBD"); + 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) => { @@ -731,13 +730,32 @@ sap.ui.require.preload({ `; const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js"); - t.is(info.name, "test1/library-preload.js", - "TBD"); - t.is(info.rawModule, false, "TBD"); - t.is(info.format, "ui5-define", "TBD"); - t.is(info.requiresTopLevelScope, false, "TBD"); - t.deepEqual(info.subModules, ["test1/module1.js"], // Shouldn't foo/bar.js be listed here? + 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"], - "TBD"); + 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)" + }); }); From 797ad6b0b3988a15176cf90dea62bca7e9988f4f Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 25 Apr 2022 16:39:33 +0200 Subject: [PATCH 4/4] Add FIXME comments --- test/lib/lbt/analyzer/JSModuleAnalyzer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js index bfe7a2d55..139431eed 100644 --- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -694,7 +694,7 @@ sap.ui.define([], function() { 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", "Format should be declare once a jQuery.sap.declare statement is found"); + 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"); @@ -710,7 +710,7 @@ sap.ui.predefine("test1/module1", [], function() { 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", "Format should be declare once a jQuery.sap.declare statement is found"); + 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"],