Skip to content

Commit ed1cc9d

Browse files
authored
[FIX] Detect dynamic dependencies also when newer APIs are used (#391)
[FIX] Detect dynamic dependencies also when newer APIs are used So far, dynamic dependencies only had been detected for calls to jQuery.sap.require. Now, calls to sap.ui.require and sap.ui.requireSync are also checked.
1 parent a80dd43 commit ed1cc9d

File tree

5 files changed

+61
-5
lines changed

5 files changed

+61
-5
lines changed

lib/lbt/analyzer/JSModuleAnalyzer.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ class JSModuleAnalyzer {
546546
const requiredModuleName2 = ModuleName.fromUI5LegacyName( arg.alternate.value );
547547
info.addDependency(requiredModuleName2, true);
548548
} else {
549-
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg);
549+
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg && arg.type);
550550
info.dynamicDependencies = true;
551551
}
552552
}
@@ -558,13 +558,17 @@ class JSModuleAnalyzer {
558558
const nArgs = args.length;
559559
const i = 0;
560560

561-
if ( i < nArgs && isString(args[i]) ) {
562-
const moduleName = ModuleName.fromRequireJSName( args[i].value );
563-
info.addDependency(moduleName, conditional);
561+
if ( i < nArgs ) {
562+
if ( isString(args[i]) ) {
563+
const moduleName = ModuleName.fromRequireJSName( args[i].value );
564+
info.addDependency(moduleName, conditional);
565+
} else {
566+
log.verbose("sap.ui.requireSync: cannot evaluate dynamic arguments: ", args[i] && args[i].type);
567+
info.dynamicDependencies = true;
568+
}
564569
}
565570
}
566571

567-
568572
function onSapUiPredefine(predefineCall) {
569573
const args = predefineCall.arguments;
570574
const nArgs = args.length;
@@ -622,6 +626,9 @@ class JSModuleAnalyzer {
622626
requiredModule = ModuleName.resolveRelativeRequireJSName(name, item.value);
623627
}
624628
info.addDependency( requiredModule, conditional );
629+
} else {
630+
log.verbose("sap.ui.require/sap.ui.define: cannot evaluate dynamic argument: ", item && item.type);
631+
info.dynamicDependencies = true;
625632
}
626633
});
627634
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
sap.ui.define([], function() {
2+
return {
3+
load: function(modName) {
4+
sap.ui.require([modName], function(modExport) {
5+
// module was loaded
6+
}, function(err) {
7+
// error occurred
8+
});
9+
}
10+
}
11+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
sap.ui.define([], function() {
2+
return {
3+
load: function(modName) {
4+
return sap.ui.requireSync(modName);
5+
}
6+
}
7+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
jQuery.sap.declare("sap.ui.testmodule");
2+
3+
sap.ui.testmodule.load = function(modName) {
4+
jQuery.sap.require(modName);
5+
};

test/lib/lbt/analyzer/JSModuleAnalyzer.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ function analyzeModule(
9494
expectedSubmodules,
9595
"submodules should match");
9696
}
97+
t.false(info.dynamicDependencies,
98+
"no use of dynamic dependencies should have been detected");
9799
}).then(() => t.end(), (e) => t.fail(`failed to analyze module with error: ${e.message}`));
98100
}
99101

@@ -356,7 +358,31 @@ test("ES6 Syntax", (t) => {
356358
"only dependencies to 'conditional/*' modules should be conditional");
357359
t.is(info.isImplicitDependency(dep), !/^(?:conditional|static)\//.test(dep),
358360
"all dependencies other than 'conditional/*' and 'static/*' should be implicit");
361+
t.false(info.dynamicDependencies,
362+
"no use of dynamic dependencies should have been detected");
359363
});
360364
});
361365
});
362366

367+
test("Dynamic import (declare/require)", (t) => {
368+
return analyze("modules/declare_dynamic_require.js").then((info) => {
369+
t.true(info.dynamicDependencies,
370+
"the use of dynamic dependencies should have been detected");
371+
});
372+
});
373+
374+
test("Dynamic import (define/require)", (t) => {
375+
return analyze("modules/amd_dynamic_require.js").then((info) => {
376+
t.true(info.dynamicDependencies,
377+
"the use of dynamic dependencies should have been detected");
378+
});
379+
});
380+
381+
test("Dynamic import (define/requireSync)", (t) => {
382+
return analyze("modules/amd_dynamic_require_sync.js").then((info) => {
383+
t.true(info.dynamicDependencies,
384+
"the use of dynamic dependencies should have been detected");
385+
});
386+
});
387+
388+

0 commit comments

Comments
 (0)