Angular-cli: Inability to avoid including `source-map-support` in Karma tests

Created on 2 Feb 2019  ·  12Comments  ·  Source: angular/angular-cli

🐞 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 https://github.com/angular/angular-cli/pull/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 https://github.com/badeball/karma-jsdom-launcher/issues/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
``

devkibuild-angular low regression bufix

Most helpful comment

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

All 12 comments

@badeball that's a pretty detailed investigation! As you mentioned in https://github.com/badeball/karma-jsdom-launcher/issues/27#issuecomment-458970440, that really looks like it was introduced with https://github.com/angular/angular-cli/pull/13062.

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

Please take a look at https://github.com/angular/angular-cli/pull/13584 to see if it matches your understanding of the problem.

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

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

@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: https://github.com/tschaub/karma-source-map-support/issues/16

There was a PR in September: https://github.com/tschaub/karma-source-map-support/pull/22

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

https://github.com/angular/angular-cli/pull/13584 wasn't meant to remove the dependency, just to not use it depending on the provided options.

@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.

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.

:grumble:

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

@filipesilva Looks like we succeeded - https://github.com/tschaub/karma-source-map-support/pull/22 was merged.

Thanks

Awesome! Renovate bot has picked it up already https://github.com/angular/angular-cli/pull/13623. Should go in the next major/minor version, which is 8.0 at this point.

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._

Was this page helpful?
0 / 5 - 0 ratings

Related issues

JanStureNielsen picture JanStureNielsen  ·  3Comments

rajjejosefsson picture rajjejosefsson  ·  3Comments

purushottamjha picture purushottamjha  ·  3Comments

rwillmer picture rwillmer  ·  3Comments

hartjo picture hartjo  ·  3Comments