Angular 8 build contains ctorParameters

[sam-s4s]

🐞 Bug report

Command (mark with an x)

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

Is this a regression?

Yes, the previous version in which this bug was not present was:

7.3.9

Description

I'm getting a lot of circular dependency warnings, and the app is not running after doing a dev build using ng build.
When doing a prod build using ng build --prod the build compiles and runs fine.
Also doing both types of builds worked perfectly in Angular 7, this problem has only started since I upgraded to Angular 8 yesterday.

🔥 Exception or Error

WARNING in Circular dependency detected:
a4\a4-action.ts -> a4\a4.service.ts -> a4\a4-action.ts

(above for example - there are about 50)

🌍 Your Environment

Angular CLI: 8.2.2
Node: 10.15.1
OS: win32 x64
Angular: 8.2.2
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.802.2
@angular-devkit/build-angular     0.802.2
@angular-devkit/build-optimizer   0.802.2
@angular-devkit/build-webpack     0.802.2
@angular-devkit/core              8.2.2
@angular-devkit/schematics        8.2.2
@ngtools/webpack                  8.2.2
@schematics/angular               8.2.2
@schematics/update                0.802.2
rxjs                              6.5.2
typescript                        3.5.3
webpack                           4.38.0

Anything else relevant?

I'm not sure why this problem has sprung up after an upgrade? It was working perfectly in Angular 7, and i haven't changed anything, other than running the upgrade to Angular 8.

I'm a bit stuck, as it's not easy to fix these problems, and there are about 50 of them - and as they weren't a problem before, I'm not sure why they would be now? Seems like a pretty massive regression?

[sam-s4s]

I've have a further look into the problem, and compared the previous Angular 7 build with the new Angular 8 build. The interesting thing is that the new build contains a chunk of code that was NOT present in the Angular 7 build:

Angular 8 (bad):

var A4Action = /** @class */ (function () {
    function A4Action(a4Service, a4PatientService) {
        var _this = this;
        this.a4Service = a4Service;
        this.a4PatientService = a4PatientService;
    }

    A4Action.ctorParameters = function () { return [
        { type: _a4_service__WEBPACK_IMPORTED_MODULE_3__["A4Service"] },
        { type: _a4_patient_service__WEBPACK_IMPORTED_MODULE_4__["A4PatientService"] }
    ]; };
    return A4Action;
}());

Angular 7 (good):

var A4Action = /** @class */ (function () {
    function A4Action(a4Service, a4PatientService) {
        var _this = this;
        this.a4Service = a4Service;
        this.a4PatientService = a4PatientService;
    }
        
    return A4Action;
}());

So it's actually creating a circular dependency where there was none before... Why has this been done? It breaks a lot of code. I feel like there's something I'm missing, maybe a compiler setting or something?

Possibly of note
A4Action is NOT an Angular @Component, it's just a normal class.
A4Service is an Angular @Component.

Short story:
This (seemingly unnecessary) piece of code is being inserted at the bottom of all of my classes (with the appropriate constructor params), breaking my JiT/dev builds:

    A4Action.ctorParameters = function () { return [
        { type: _a4_service__WEBPACK_IMPORTED_MODULE_3__["A4Service"] },
        { type: _a4_patient_service__WEBPACK_IMPORTED_MODULE_4__["A4PatientService"] }
    ]; };

Angular 7 build contains no instances of ctorParameters. Angular 8 build contains hundreds, many of which cause circular dependencies.

[sam-s4s]

Ah I was hoping someone would've taken a look at this by now.
I'll have to revert our projects back to Angular 7 for now :(

Just in case this is related (sounds like it is), I found this:
https://www.npmjs.com/package/@angular-devkit/build-optimizer#scrub-file

Scrub file
Angular decorators, property decorators and constructor parameters are removed, while leaving non-Angular ones intact.

[clydin]

Does the A4Action class have any decorators?

Also, the scrub file optimization pass is not actually related.

[sam-s4s]

@clydin Thank you for the reply :)

A4Action is just a normal class:

export class A4Action { }

I just did a test - I made a brand new cli project (latest version)... added in a plain class, and a service, and instantly the ng build fails with circular dependencies - because it's including the ctorParameters things in the dist files. Again, this does not happen in Angular 7 builds.

import { Injectable } from "@angular/core";
import { MyThing } from './my-thing';

@Injectable()
export class MyService {
    myThing = new MyThing(this);

    constructor() { }
}

import { MyService } from './my.service';

export class MyThing {
    seven: number = 7;

    constructor(private myService: MyService) { }
}

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent {
    title = 'test';

    constructor(myService: MyService) {
        console.log(myService.myThing.seven);
    }
}

ng build -> circular deps
So it's very easy to re-pro... I'm just so confused as to why other people haven't run into this problem before.

[alxhub]

This is a bug where @ngtools/webpack attempts to downlevel ctorParameters when building for ES2015. However, unlijke emitDecoratorMetadata, the CLI's generation of ctorParameters runs for every class regardless of whether it contains any decorators.

This has the effect of converting type-position dependencies into runtime dependencies in arbitrary classes in the program, which can cause circular references that otherwise would not exist when generating ES2015 code.

A workaround is to "hide" the type from the transformer:

import { MyService } from './my.service';

export type Hidden<T> = T;

export class MyThing {
    seven: number = 7;

    constructor(private myService: Hidden<MyService>) { }
}

This works because the transformer uses getSymbolAtLocation on the type name of the parameter, and checks whether it has a value declaration. Since the symbol for Hidden is type-only, the transformer gives up on this parameter.

[sam-s4s]

So upon talking to Alex, I think the code in question is
https://github.com/angular/angular-cli/blob/master/packages/ngtools/webpack/src/transformers/ctor-parameters.ts#L275-L281

Just needs to check for decorators, similar to
https://github.com/angular/tsickle/blob/master/src/decorator_downlevel_transformer.ts#L526-L531

@clydin / @mgechev - is this something you could help with? <3

[clydin]

The analysis is correct. There’s already a PR fix available. I just wanted to confirm that the class in question didn’t have any decorators before marking the PR as a fix for this issue.
#15416

Also of note is that this only affects JIT mode. The transform as a whole is not needed for AOT which is why production doesn’t exhibit the original issue.

Something else to consider (and a reason others probably haven’t encountered this) is that the application technically has circular dependencies. They are just design time rather than runtime.

[sam-s4s]

@clydin Yes, confirmed that the class didn't have a decorator.

Thanks :)

[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.}