ng15 compiler output broken this in async field functions, with "useDefineForClassFields" unset

[johncrim]

Which @angular/* package(s) are the source of the bug?

compiler

Is this a regression?

Yes

Description

After upgrading to ng15, I'm seeing bad compile output in async functions that are assigned to fields - the this is not being captured, resulting in NPEs in a number of places. I've also seen other issues where field initialization is re-ordered after upgrading to ng15, which may be related.

A simple example is:

import { Inject, Injectable, InjectionToken, OnDestroy } from '@angular/core';
import { BehaviorSubject } from 'rxjs';
import { delay } from './delay';

export const WINDOW = new InjectionToken<Window>('window', { providedIn: 'platform', factory: () => window });

/**
 * Service that provides online/offline status.
 */
@Injectable({ providedIn: 'root' })
export class OnlineService2
  extends BehaviorSubject<boolean>
  implements OnDestroy {

  constructor(@Inject(WINDOW) private readonly _window: Window) {
    const browserOnline = _window.navigator.onLine;
    super(browserOnline);

    _window.addEventListener('online', this.toOnline);
    _window.addEventListener('offline', this.toOffline);
  }

  ngOnDestroy() {
    this._window.removeEventListener('online', this.toOnline);
    this._window.removeEventListener('offline', this.toOffline);
    this.complete();
  }

  private readonly toOnline = async () => {
    await delay(1);
    this.next(true);
  };

  private readonly toOffline = async () => {
    await delay(1);
    this.next(false);
  };
}

The compiled output is:

class OnlineService2 extends rxjs__WEBPACK_IMPORTED_MODULE_1__.BehaviorSubject {
  constructor() {
    const browserOnline = navigator.onLine;
    super(browserOnline);
    window.addEventListener('online', this.toOnline);
    window.addEventListener('offline', this.toOffline);
  }
  ngOnDestroy() {
    window.removeEventListener('online', this.toOnline);
    window.removeEventListener('offline', this.toOffline);
    this.complete();
  }
  toOnline = (0,prj_node_modules_angular_devkit_build_angular_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
    yield (0,_ease_util__WEBPACK_IMPORTED_MODULE_2__.delay)(1);
    _this.next(true);
  });
  toOffline = (0,prj_node_modules_angular_devkit_build_angular_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
    yield (0,_ease_util__WEBPACK_IMPORTED_MODULE_2__.delay)(1);
    _this2.next(false);
  });
  static ɵfac = function OnlineService2_Factory(t) {
    return new (t || OnlineService2)();
  };
  static ɵprov = /*@__PURE__*/_angular_core__WEBPACK_IMPORTED_MODULE_3__["ɵɵdefineInjectable"]({
    token: OnlineService2,
    factory: OnlineService2.ɵfac,
    providedIn: 'root'
  });
}

You can see that the _this and _this2 are not initialized before they are used. If I change my tsconfig settings from:

    "module": "esnext",

to

    "module": "ES2022",
    "useDefineForClassFields": false,

the compiled output works as expected:

class OnlineService2 extends rxjs__WEBPACK_IMPORTED_MODULE_2__.BehaviorSubject {
  constructor(_window) {
    var _this;
    const browserOnline = _window.navigator.onLine;
    super(browserOnline);
    _this = this;
    this._window = _window;
    this.toOnline = /*#__PURE__*/(0,prj_node_modules_angular_devkit_build_angular_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
      yield (0,_ease_util__WEBPACK_IMPORTED_MODULE_3__.delay)(1);
      _this.next(true);
    });
    this.toOffline = /*#__PURE__*/(0,prj_node_modules_angular_devkit_build_angular_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
      yield (0,_ease_util__WEBPACK_IMPORTED_MODULE_3__.delay)(1);
      _this.next(false);
    });
    _window.addEventListener('online', this.toOnline);
    _window.addEventListener('offline', this.toOffline);
  }
  ngOnDestroy() {
    this._window.removeEventListener('online', this.toOnline);
    this._window.removeEventListener('offline', this.toOffline);
    this.complete();
  }
  static #_ = this.ɵfac = function OnlineService2_Factory(t) {
    return new (t || OnlineService2)(_angular_core__WEBPACK_IMPORTED_MODULE_1__["ɵɵinject"](WINDOW));
  };
  static #_2 = this.ɵprov = /*@__PURE__*/_angular_core__WEBPACK_IMPORTED_MODULE_1__["ɵɵdefineInjectable"]({
    token: OnlineService2,
    factory: OnlineService2.ɵfac,
    providedIn: 'root'
  });
}

Please provide a link to a minimal reproduction of the bug

https://github.com/johncrim/repro-ng15-compile

Please provide the exception or error you saw

Error: Uncaught (in promise): ReferenceError: _this is not defined
ReferenceError: _this is not defined
    at http://localhost:9876/_karma_webpack_/webpack:/apps/ease/src/modules/app/Online.service2.ts:37:5
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (http://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular-devkit/build-angular/node_modules/@babel/runtime/helpers/esm/asyncToGenerator.js:3:1)
    at apply (http://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular-devkit/build-angular/node_modules/@babel/runtime/helpers/esm/asyncToGenerator.js:22:1)
    at _ZoneDelegate.invoke (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:375:26)
    at FakeAsyncTestZoneSpec.onInvoke (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1938:33)
    at ProxyZoneSpec.onInvoke (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:284:39)
    at _ZoneDelegate.invoke (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:374:52)
    at Zone.run (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:134:43)
    at apply (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:1278:36)
    at _ZoneDelegate.invokeTask (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:409:31)
    at ProxyZoneSpec.onInvokeTask (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:318:39)
    at _ZoneDelegate.invokeTask (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:408:60)
    at Zone.runTask (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:178:47)
    at invokeTask (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:490:34)

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 15.0.5
Node: 19.3.0 (Unsupported)
Package Manager: yarn 3.2.4
OS: win32 x64

Angular: 15.0.4
... animations, cdk, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1500.5
@angular-devkit/build-angular   15.0.5
@angular-devkit/core            15.0.5
@angular-devkit/schematics      15.0.5
@angular/cli                    15.0.5
@schematics/angular             15.0.5
ng-packagr                      15.0.3
rxjs                            7.8.0
typescript                      4.8.4
webpack                         5.75.0

Anything else?

I have not been able to cause the error in my standalone project (https://github.com/johncrim/repro-ng15-compile) - I've tried tweaking dependencies eg the typescript version and tsconfig setting to match my upgraded project, but no luck generating the failure. However if I move the same code into my project I am seeing the compile output described above.

I have a fix (in tsconfig set "module": "ES2022" and "useDefineForClassFields": false) which is easy to apply, though I'm not done testing it everywhere. Since those tsconfig settings weren't changed by migrations I'm raising the issue for the benefit of the community.

[alan-agius4]

Hi @johncrim,

Thanks for opening the issue but without a reproduction it's not possible for us to investigate.

Couple of things you could try:

• Remove yarn lock file and run a fresh node install
• Verify that this is not an issue caused by a custom browserslist configuration.

[johncrim]

Thanks for the advice @alan-agius4 - I added our browserslist and refreshed the yarn info. The build output is now closer to the buggy output (the this capture is distinct per function), but is still not yielding the bug (the _this variable is set):

class OnlineService2 extends rxjs__WEBPACK_IMPORTED_MODULE_3__.BehaviorSubject {
  _window;
  constructor(_window) {
    const browserOnline = _window.navigator.onLine;
    super(browserOnline);
    this._window = _window;
    _window.addEventListener('online', this.toOnline);
    _window.addEventListener('offline', this.toOffline);
  }
  ngOnDestroy() {
    this._window.removeEventListener('online', this.toOnline);
    this._window.removeEventListener('offline', this.toOffline);
    this.complete();
  }
  toOnline = (() => {
    var _this = this;
    return (0,ng15_compile_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
      yield (0,_delay__WEBPACK_IMPORTED_MODULE_1__.delay)(1);
      _this.next(true);
    });
  })();
  toOffline = (() => {
    var _this2 = this;
    return (0,ng15_compile_node_modules_babel_runtime_helpers_esm_asyncToGenerator_js__WEBPACK_IMPORTED_MODULE_0__["default"])(function* () {
      yield (0,_delay__WEBPACK_IMPORTED_MODULE_1__.delay)(1);
      _this2.next(false);
    });
  })();
  static ɵfac = function OnlineService2_Factory(t) {
    return new (t || OnlineService2)(_angular_core__WEBPACK_IMPORTED_MODULE_2__["ɵɵinject"](WINDOW));
  };
  static ɵprov = /*@__PURE__*/_angular_core__WEBPACK_IMPORTED_MODULE_2__["ɵɵdefineInjectable"]({
    token: OnlineService2,
    factory: OnlineService2.ɵfac,
    providedIn: 'root'
  });
}

I'll give it some more thought to see if I can figure out how to get the repro to work.

[alan-agius4]

Do this happen when using @angular/compiler-cli@15.1.0-next.3?

[johncrim]

Hi @alan-agius4 - thank you for the follow up. I just tested both codebases with @angular/compiler-cli@15.1.0-next.3 and @angular/compiler@15.1.0-next.3 (all other ng dependencies are 15.0.4), and unfortunately no progress in resolving or reproducing this bug:

• In our private project, with "module": "esnext", I still get the bad compiler output shown above (this is not captured)
• The (attempted) repro project is still not reproducing the bug

I could conceivably try removing everything but the repro case from our private repo, but it's a monorepo so would be a bunch of work to do so - kind of difficult to justify since I have a workaround. This is not a "only on my machine" bug, b/c we had build failures on Ubuntu from test failures from this bug.

I figure another useful factor is whether this issue bites others. Since no one else has chimed in on this bug, it seems like the impact may be isolated.

[johncrim]

@alan-agius4 - could there be a different compilation code path between a new (ng new) project and an older project? I know that esbuild is being used in some cases, and it does seem that the new project builds relatively faster (though it's really hard to tell - tiny project vs large project). If so, is there a way to trigger the old compilation code path on a new project?

[alan-agius4]

@alan-agius4 - could there be a different compilation code path between a new (ng new) project and an older project? I know that esbuild is being used in some cases, and it does seem that the new project builds relatively faster (though it's really hard to tell - tiny project vs large project). If so, is there a way to trigger the old compilation code path on a new project?

No, the code paths for ng new and existing applications is the same. Esbuild builder is only used when users opt-in manually by changing the builder in angular.json.

At this point there is not much which we can action here from our end given that it is not reproducible. Feel free to open a new issue if you manage to reproduce.

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