Inability to avoid including source-map-support in Karma tests

[badeball]

🐞 Bug report

Command (mark with an x)

- [ ] new
- [ ] build
- [ ] serve
- [x] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Is this a regression?

I suspect this is a regression introduced by #13062.

Description

I believe one used to be able to configure through angular.json such that Karma tests would not include the karma-source-map-support plugin. It can now however seem like this plugin is mistakenly always included.

1. Even though I specify "sourceMap": false in angular.json

2. Config is normalized

3. sourceMap option is further normalized

4. Normalization of sourceMap always ends up with an object

5. source-map-support is conditionally included, but the condition is always true

🔬 Minimal Reproduction

$ ng new foo
$ cd foo
$ npm install karma-jsdom-launcher jsdom
$ sed -i 's/Chrome/jsdom/g' src/karma.conf.js
$ sed -i 's/chrome/jsdom/g' src/karma.conf.js

Then run ng test and watch the process never exit.

🔥 Exception or Error

I was made aware of this issue in badeball/karma-jsdom-launcher#27. Project of said issue aims to allow jsdom to be used as a target browser. Making synchronous requests like source-map-support does will create a deadlock and halt execution indefinitely.

🌍 Your Environment

$ ng --version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 7.3.0
Node: 11.8.0
OS: linux x64
Angular: 7.2.3
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.13.0
@angular-devkit/build-angular     0.13.0
@angular-devkit/build-optimizer   0.13.0
@angular-devkit/build-webpack     0.13.0
@angular-devkit/core              7.3.0
@angular-devkit/schematics        7.3.0
@angular/cli                      7.3.0
@ngtools/webpack                  7.3.0
@schematics/angular               7.3.0
@schematics/update                0.13.0
rxjs                              6.3.3
typescript                        3.2.4
webpack                           4.29.0
`` 

[filipesilva]

@badeball that's a pretty detailed investigation! As you mentioned in badeball/karma-jsdom-launcher#27 (comment), that really looks like it was introduced with #13062.

Should be a trivial fix on our side. Sorry about that!

[filipesilva]

Please take a look at #13584 to see if it matches your understanding of the problem.

[badeball]

Can confirm, this corrects the issue. Thanks the quick response!

[yogeshgadge]

I am also no longer able to use jsdom - waiting for this to merge.

[JohnLudlow]

@vikerman We're still getting karma-source-map-support included when we do npm install. I guess the 'fix' was to make it not fall over on build/test?

We have a different issue. They have no license file, which causes a compliance issue for us (yes, even for dev dep). There was an issue raised 14 months ago: tschaub/karma-source-map-support#16

There was a PR in September: tschaub/karma-source-map-support#22

We're a little worried the project might be inactive, and we were hoping that this issue would remove that dependency.

[filipesilva]

#13584 wasn't meant to remove the dependency, just to not use it depending on the provided options.

[JohnLudlow]

@filipesilva thanks for the quick response!

Yeah we're seeing that's the case now. What's the best way to proceed? I'm not sure raising an issue or PR for karma-source-map-support is worthwhile because that's already been done by other people.

Should I raise a new issue against angular-cli (or a different project?) to consider removing that as a hard dependency?

In general, it'd be nice to not have that as a hard dependency because maybe we don't use Karma or sourcemaps in that way, and a project kicking around 'just coz' will likely cause issues.

[filipesilva]

I'm sorry to say but we don't use a modular approach in @angular-devkit/build-angular. It's architecture is meant to include everything it might use. There are a few exceptions, like TypeScript, Protractor, Karma, and the Karma plugins referenced in the karma config.

But karma-source-map-support is not referenced in the config, and is something we add internally based on some conditions. It's somewhat like scss/less/stylus: you might not use them, but we don't add them on the fly.

Realistically speaking this could happen. But I don't think it will happen on the timeframe you'd like it to happen. It's a fair bit of work and not a priority.

So I think your best chance right now is to get the karma-source-map-support maintainer to accept the PR you need in. We're happy enough accepting a substitute for that plugin but would be fairly strict in accepting that, because source map support is hard to validate.

[JohnLudlow]

:grumble:

Ok, thanks. We'll try and get his attention

[JohnLudlow]

@filipesilva Looks like we succeeded - tschaub/karma-source-map-support#22 was merged.

Thanks

[filipesilva]

Awesome! Renovate bot has picked it up already #13623. Should go in the next major/minor version, which is 8.0 at this point.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}