Skip to content

Commit

Permalink
[FIX] Detect dynamic dependencies also when newer APIs are used
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
codeworrior committed Dec 29, 2019
1 parent 53908c8 commit d0b2c3f
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 5 deletions.
17 changes: 12 additions & 5 deletions lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ class JSModuleAnalyzer {
const requiredModuleName2 = ModuleName.fromUI5LegacyName( arg.alternate.value );
info.addDependency(requiredModuleName2, true);
} else {
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg);
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg && arg.type);
info.dynamicDependencies = true;
}
}
Expand All @@ -558,13 +558,17 @@ class JSModuleAnalyzer {
const nArgs = args.length;
const i = 0;

if ( i < nArgs && isString(args[i]) ) {
const moduleName = ModuleName.fromRequireJSName( args[i].value );
info.addDependency(moduleName, conditional);
if ( i < nArgs ) {
if ( isString(args[i]) ) {
const moduleName = ModuleName.fromRequireJSName( args[i].value );
info.addDependency(moduleName, conditional);
} else {
log.verbose("sap.ui.requireSync: cannot evaluate dynamic arguments: ", args[i] && args[i].type);
info.dynamicDependencies = true;
}
}
}


function onSapUiPredefine(predefineCall) {
const args = predefineCall.arguments;
const nArgs = args.length;
Expand Down Expand Up @@ -622,6 +626,9 @@ class JSModuleAnalyzer {
requiredModule = ModuleName.resolveRelativeRequireJSName(name, item.value);
}
info.addDependency( requiredModule, conditional );
} else {
log.verbose("sap.ui.require/sap.ui.define: cannot evaluate dynamic argument: ", item && item.type);
info.dynamicDependencies = true;
}
});
}
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/lbt/modules/amd_dynamic_require.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
sap.ui.define([], function() {
return {
load: function(modName) {
sap.ui.require([modName], function(modExport) {
// module was loaded
}, function(err) {
// error occurred
});
}
}
});
7 changes: 7 additions & 0 deletions test/fixtures/lbt/modules/amd_dynamic_require_sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
sap.ui.define([], function() {
return {
load: function(modName) {
return sap.ui.requireSync(modName);
}
}
});
5 changes: 5 additions & 0 deletions test/fixtures/lbt/modules/declare_dynamic_require.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
jQuery.sap.declare("sap.ui.testmodule");

sap.ui.testmodule.load = function(modName) {
jQuery.sap.require(modName);
};
27 changes: 27 additions & 0 deletions test/lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ function analyzeModule(
expectedSubmodules,
"submodules should match");
}
t.false(info.dynamicDependencies,
"no use of dynamic dependencies should have been detected");

}).then(() => t.end(), (e) => t.fail(`failed to analyze module with error: ${e.message}`));
}

Expand Down Expand Up @@ -356,7 +359,31 @@ test("ES6 Syntax", (t) => {
"only dependencies to 'conditional/*' modules should be conditional");
t.is(info.isImplicitDependency(dep), !/^(?:conditional|static)\//.test(dep),
"all dependencies other than 'conditional/*' and 'static/*' should be implicit");
t.false(info.dynamicDependencies,
"no use of dynamic dependencies should have been detected");
});
});
});

test("Dynamic import (declare/require)", (t) => {
return analyze("modules/declare_dynamic_require.js").then((info) => {
t.true(info.dynamicDependencies,
"the use of dynamic dependencies should have been detected");
});
});

test("Dynamic import (define/require)", (t) => {
return analyze("modules/amd_dynamic_require.js").then((info) => {
t.true(info.dynamicDependencies,
"the use of dynamic dependencies should have been detected");
});
});

test("Dynamic import (define/requireSync)", (t) => {
return analyze("modules/amd_dynamic_require_sync.js").then((info) => {
t.true(info.dynamicDependencies,
"the use of dynamic dependencies should have been detected");
});
});


0 comments on commit d0b2c3f

Please sign in to comment.