forwardRef breaks at runtime with ES2015

[filipesilva]

🐞 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

No.

Description

Using forwardRef as described in https://angular.io/api/core/forwardRef while targeting ES2015 results on a Uncaught ReferenceError: Cannot access 'Lock' before initialization runtime error.

🔬 Minimal Reproduction

• make a new project ng new forward-ref-project && cd forward-ref-project
• ensure tsconfig.json contains "target": "es2015",
• replace the contents of src/main.ts with:

import { Inject, forwardRef, ReflectiveInjector } from '@angular/core';
class Door {
  lock: Lock;

  // Door attempts to inject Lock, despite it not being defined yet.
  // forwardRef makes this possible.
  constructor(@Inject(forwardRef(() => Lock)) lock: Lock) { this.lock = lock; }
}

// Only at this point Lock is defined.
class Lock { }

const injector = ReflectiveInjector.resolveAndCreate([Door, Lock]);
const door = injector.get(Door);
console.log(door instanceof Door);
console.log(door.lock instanceof Lock);

• ng serve -o

🔥 Exception or Error

Uncaught ReferenceError: Cannot access 'Lock' before initialization
    at Module../src/main.ts (main.ts:21)
    at __webpack_require__ (bootstrap:78)
    at Object.2 (main.ts:30)
    at __webpack_require__ (bootstrap:78)
    at checkDeferredModules (bootstrap:45)
    at Array.webpackJsonpCallback [as push] (bootstrap:32)
    at main.js:1

🌍 Your Environment

Angular Version:

Angular CLI: 8.0.0-beta.18
Node: 10.10.0
OS: win32 x64
Angular: 8.0.0-beta.14
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.800.0-beta.18
@angular-devkit/build-angular     0.800.0-beta.18
@angular-devkit/build-optimizer   0.800.0-beta.18
@angular-devkit/build-webpack     0.800.0-beta.18
@angular-devkit/core              8.0.0-beta.18
@angular-devkit/schematics        8.0.0-beta.18
@angular/cli                      8.0.0-beta.18
@ngtools/webpack                  8.0.0-beta.18
@schematics/angular               8.0.0-beta.18
@schematics/update                0.800.0-beta.18
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.30.0

Anything else relevant?

forwardRef is provided specifically for the purpose of referencing something that isn't defined. This is useful in breaking circular dependencies, and when declaring both services and components in the same file.

forwardRef works because it delays the resolution of the reference to a time at which it is already declared through the callback indirection. In the API example, the symbol we want to delay resolution is Lock:

class Door {
  lock: Lock;

  // Door attempts to inject Lock, despite it not being defined yet.
  // forwardRef makes this possible.
  constructor(@Inject(forwardRef(() => Lock)) lock: Lock) { this.lock = lock; }
}

// Only at this point Lock is defined.
class Lock { }

But Lock is actually being referenced in more places than just inside forwardRef. It is also being used as a TS type in the class property, and in the constructor parameter.

Types don't usually have a runtime representation so that shouldn't be a problem. But constructor types are an exception and actually do have a runtime representation. We can see this by looking at the transpiled code:

import * as tslib_1 from "tslib";
import { Inject, forwardRef, ReflectiveInjector } from '@angular/core';
let Door = class Door {
    // Door attempts to inject Lock, despite it not being defined yet.
    // forwardRef makes this possible.
    constructor(lock) { this.lock = lock; }
};
Door = tslib_1.__decorate([
    tslib_1.__param(0, Inject(forwardRef(() => Lock))),
    tslib_1.__metadata("design:paramtypes", [Lock])
], Door);
// Only at this point Lock is defined.
class Lock {
}
const injector = ReflectiveInjector.resolveAndCreate([Door, Lock]);
const door = injector.get(Door);
console.log(door instanceof Door);
console.log(door.lock instanceof Lock);

The Lock type in the for the constructor parameter was transpiled into tslib_1.__metadata("design:paramtypes", [Lock]). This reference does not have a delayed resolution like the injected forwardRef and is instead immediately resolved, resulting in Uncaught ReferenceError: Cannot access 'Lock' before initialization.

This error isn't observed when targetting ES5 however. We can understand why by looking at the code when transpiled to ES5 :

import * as tslib_1 from "tslib";
import { Inject, forwardRef, ReflectiveInjector } from '@angular/core';
var Door = /** @class */ (function () {
    // Door attempts to inject Lock, despite it not being defined yet.
    // forwardRef makes this possible.
    function Door(lock) {
        this.lock = lock;
    }
    Door = tslib_1.__decorate([
        tslib_1.__param(0, Inject(forwardRef(function () { return Lock; }))),
        tslib_1.__metadata("design:paramtypes", [Lock])
    ], Door);
    return Door;
}());
// Only at this point Lock is defined.
var Lock = /** @class */ (function () {
    function Lock() {
    }
    return Lock;
}());
var injector = ReflectiveInjector.resolveAndCreate([Door, Lock]);
var door = injector.get(Door);
console.log(door instanceof Door);
console.log(door.lock instanceof Lock);

In ES5 there are no class declarations, so TS instead uses a var. One important different between var and class/let/const is that the latter are all subject to the Temporal Dead Zone.

In practical terms the TDZ means that using a var before it is declared resolves to undefined, but using a class/let/const instead throws a ReferenceError. This is the error we are seeing here.

A possible workaround is to not declare the type in the constructor:

// Instead of adding the type in the parameter
constructor(@Inject(forwardRef(() => Lock)) lock: Lock) { 
  this.lock = lock;
}

// Add it as a cast in the constructor body
constructor(@Inject(forwardRef(() => Lock)) lock) {
  this.lock = lock as Lock;
}

This will change the transpiled code and remove the reference, avoiding the ReferenceError:

Door = tslib_1.__decorate([
    tslib_1.__param(0, Inject(forwardRef(() => Lock))),
    tslib_1.__metadata("design:paramtypes", [Object])
                                             ^^^^^^ was Lock before
], Door);

One important note is that the ReferenceError does not come up on Angular CLI projects compiled with AOT. This is because there we actually transform transpiled TS code and remove Angular decorators, so the metadata reference (tslib_1.__metadata("design:paramtypes", [Lock])) never reaches the browser and thus there is no ReferenceError.

[Airblader]

This sounds like something that Angular simply cannot fix since it's a limitation of how JS works, right?

[elvisbegovic]

I think (in v8) at runtime, a lot of people may be suprised targeting es2015 by default due to the fact CLI historically allowed "showCircularDependencies": false and consider circular deps as warning.

This will breaks a lot of existing angular project, for sure

[dawidgarus]

I've found a TypeScript issue regarding this: microsoft/TypeScript#27519
And I think it should be fixed in TypeScript, although Angular could make a workaround for angular cli. You could add a transformer in AngularWebpackPlugin that would add forwardRef to __metadata params.

[filipesilva]

This seems to be the same as #29107, although that one was initially observed in Bazel toolchains only.

[LanderBeeuwsaert]

I think (in v8) at runtime, a lot of people may be suprised targeting es2015 by default due to the fact CLI historically allowed "showCircularDependencies": false and consider circular deps as warning.

This will breaks a lot of existing angular project, for sure

well it breaks our application that's for sure, on multiple places ;)

[filipesilva]

@LanderBeeuwsaert we actually fixed this on angular/angular-cli#14473. Or at least we thought we did. Can you tell us more about what you are seeing? A repro would be great too.

cc @clydin

[LanderBeeuwsaert]

@filipesilva
Hey, sorry for the late answer, busy times. A repro will not be possible for the moment, sorry.

Certainly also because my guess is that it's related to some circular dependency somewhere and we have a lot of them. (the blocking ones we solve by combinations of forwardRef and using Injector.get())
So a repro would have to include a lot of code from the project is my guess.

If I solve the first error, by changing the code in AppReadyEvent service, then the issue appears in a component. So I actually guess that's progress ;). And, now it seems to not be related to forwardRef:

Ha, I'm updating this as I go along :), So I was thinking that it could be related to the fact that I give the SheetViewComponent as type, in a component that SheetViewComponent is connected to already. (so they are circularely referencing each other through multiple other components in between).
If I change to: sheetViewComponent: any instead of sheetViewComponent: SheetViewComponent it works.

So the change from 7 to 8 seems to be that somehow inputs cannot have circular references?

[LanderBeeuwsaert]

New update. Everytime I solve one of these, a new (similar) issue pops up. So for me, there seems to be a real consistent change in the way forwardRef/Input usage of types are compiled/handled. Seems that it's maybe related to that it sees input() types also in compiling/referencing of the types?

I'm pretty sure that if I would put any as type instead of SheetViewComponent, it would run without failing. Do you want me to try? (but of course as a permanent solution I would prefer not to have to change everything to type any.)

[LanderBeeuwsaert]

Extra update, I do have to say that in polyfills.ts I had to do:
/** Evergreen browsers require these. **/
// import 'core-js/es6/reflect';
// import 'core-js/es7/reflect';
import 'core-js/proposals/reflect-metadata';

as compilation didn't find the es6/reflect ; es7/reflect

so maybe could be related to that?

[elvisbegovic]

As workaround, I think you can have both type SheetViewComponent | any to keep autocompletion

[filipesilva]

@LanderBeeuwsaert it isn't as much ForwardRef/Input itself, as much as it is differential loading.

The problems comes around with circular dependencies when using ES2015. And using differential loading (which happens automatically on the update to 8) uses ES2015 by default.

In https://angular.io/guide/deployment#opting-out-of-differential-loading you can find more information about opting out. For you, the right option is Set the target in the compilerOptions to es5. because it makes all of your compilations (both build, serve and test) use ES5 instead of ES2015.

This isn't great but at least you can go back to a stable usage with this single switch, and still use the rest of Angular 8 while you look at addressing the circular dependencies.

[LanderBeeuwsaert]

Thanks @istiti , good idea, I'll wait for now, see if the issue gets resolved during the coming weeks. If not we'll weigh the investment of time. Anyway, crazy coincidence but seems that we have mutual connections. (we're in Genèva now for the GIAC)

[LanderBeeuwsaert]

@filipesilva , ok, thanks for the solution.
But if I understand correctly it's not something that is considered a bug so you'll not adapt it?
So if we want to use differential loading (which we do :)) we'll have to invest in solving our circular dependencies?

[filipesilva]

We thought we had taken care of it already, but it looks like we haven't :/

To be honest it's not 100% clear to me if the problem is exactly the same as forwardRef. I imagine it's still related to circular deps, but your problem might be a bit more involved.

At the end of the day the circular dep thing comes down to microsoft/TypeScript#27519. In ES5, a circular dependency would not be a problem when loading modules, it would just undefined at some point. In ES2015 it will be an error at runtime.

So I think that yes, if you want to use ES2015 you will need to break up circular dependencies at some point. There's a couple of good strategies to do it, as it is not a Angular specific thing.

Since circular dependencies involving Types are a bit more frequent, they come up more often. In this particular case we cared a lot about forwardRef because it was supposed to get around this problem to begin with. But it's not the only case of circular deps.

[LanderBeeuwsaert]

Ok, thanks for all the info, we'll let this sink in for a while.
Once we commit the time to solve this I'll report back later what approach(es) we took.

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