ts_web_test_suite doesn't load npm dependencies or AMD/UMD modules

[jbedard]

I'm trying to run tests which (es6)import from the npm jasmine-marbles library. I've tried a few options but have had no luck. Importing other libraries like core-js/es6/object seem to have the exact same issue.

This library has:

• index.js - CommonJS, this is the main/default npm file
• bundles/jasmine-marbles.umd.js - UMD, including the AMD definition without a name
• src/*.{js,d.ts} - compiled source in CommonJS style

Ideally just putting @npm//jasmine-marbles into the ts_library(name=test_lib) would be all that's required. This would require converting/wrapping the CommonJS module before concatenating.

From what I've found:

• Putting @npm//jasmine-marbles into the test_lib ts_library(deps) does nothing. There is no reference to it in window.__karma__.files, it is not in the concatjs file. This causes a GET /jasmine-marbles.js 404.
• Putting @npm//node_modules/jasmine-marbles:index.js in the ts_web_test_suite(deps) causes "ReferenceError: exports is not defined" because this CommonJS file is just placed into the concatjs file (it needs a wrapper to define exports and require).
• Putting @npm//node_modules/jasmine-marbles:bundles/jasmine-marbles.umd.js in the ts_web_test_suite(deps) and mapping "jasmine-marbles" to this using require.config(...). This gets it into the window.__karma__.files and the concatjs file, but the AMD (within UMD) does not contain the optional name param so it fails with the requirejs "Mismatched anonymous define() module" error.
  It doesn't even get to the point where it is tries to load this file and using the require.config alias etc.
  Normally r.js or similar tools concatenating the files would insert the name param. Interesting to note that the ngrx/store umd file already has this first param, which I think is wrong?
• Putting any of those @npm/* into the ts_web_test_suite(runtime_deps) fails with "labels in runtime_deps must be created by ts_library"

So, I think...

1. npm deps like @npm//jasmine-marbles not being listed in __karma__.files and concatjs seems like a bug?
2. CommonJS modules should be wrapped in a define(name, ["require", "exports"], function(require, exports) { ... }).
3. AMD should have the name inserted into the define call.
4. UMD should do one of the above 2.

I'll try creating a test repo if this isn't clear...

[Toxicable]

Just came accross this issue myself
made a reproduction here: https://github.com/Toxicable/bazel_web_test_bugs

Without this being fixed karma_web_test is unusable for us since we cant use 3rd party libs

[Toxicable]

Would be solved by this #761
Since it would be able to pre process npm deps from esm into umd

[bryanjlee]

Feel free to file a new issue if you can still reproduce this.