CP-1061 Import modules in the order they were specified in config #102
Conversation
| // Load everything specified in loadFiles in the specified order | ||
| var promiseChain = karma.config.jspm.expandedFiles.reduce(function (prev, next) { | ||
| return prev.then(function () { | ||
| return System.import(extractModuleName(next)); |
There was a problem hiding this comment.
this needs to use the System['import'] syntax from above for IE8 compatibility
|
+1 once Trent's comment has been addressed |
|
@ahelmberger This pull request has merge conflicts, please resolve. |
|
Thanks for the feedback, I rebased on the current master and used the IE8-compatible syntax. |
|
+1 |
| // Promise comes from the systemjs polyfills | ||
| Promise.all(promises).then(function() { | ||
| // Load everything specified in loadFiles in the specified order | ||
| var promiseChain = karma.config.jspm.expandedFiles.reduce(function (prev, next) { |
There was a problem hiding this comment.
If we're still aiming to support IE8, we can't use reduce here. @trentgrover-wf thoughts?
There was a problem hiding this comment.
it would be nice to maintain that backwards compatibility (though I'm not sure how many users still require it). could just include the polyfill:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/Reduce#Polyfill
There was a problem hiding this comment.
If you want, I could convert the reduce functionality into a for loop. No need for including the polyfill just for one call. Please let me know what you think.
There was a problem hiding this comment.
Yeah let's go with that. Including a polyfill could theoretically cause some tests to pass that shouldn't in older browsers.
There was a problem hiding this comment.
Sorry it took a little longer, I've updated the branch. It should work also in older browsers now.
jayudey-wf
changed the title
|
+1 |
|
QA Resource Approval: +10
Merging into master. |
CP-1061 Import modules in the order they were specified in config
We would like to make the order, in which modules are loaded, deterministic. They should be imported in the order specified in
karma.conf.js. E.g. given this configuration:The current implementation would attempt to load all modules simultaneously. This can lead to different behavior between test runs, especially in the above case, where the first module adds some global polyfills other modules rely on.
With this changeset, all files specified in the
loadFilesarray will be imported sequentially.From a perfomance perspective: we could not notice any difference between simultaneous/sequential import, tested in a project with about 400 tests and about 170 modules.