Skip to content

Commit

Permalink
[FIX] Detect dynamic dependencies also when newer APIs are used (#391)
Browse files Browse the repository at this point in the history
[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.
  • Loading branch information
codeworrior authored Jan 7, 2020
1 parent a80dd43 commit ed1cc9d
Show file tree
Hide file tree
Showing 5 changed files with 61 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);
};
26 changes: 26 additions & 0 deletions test/lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ 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 +358,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 ed1cc9d

Please sign in to comment.