fix(core): detect DI parameters in JIT mode for downleveled ES2015 classes (angular#38463)

In the Angular Package Format, we always shipped UMD bundles and previously even ES5 module output.
With V10, we removed the ES5 module output but kept the UMD ES5 output.

For this, we were able to remove our second TypeScript transpilation. Instead we started only
building ES2015 output and then downleveled it to ES5 UMD for the NPM packages. This worked
as expected but unveiled an issue in the `@angular/core` reflection capabilities.

In JIT mode, Angular determines constructor parameters (for DI) using the `ReflectionCapabilities`. The
reflection capabilities basically read runtime metadata of classes to determine the DI parameters. Such
metadata can be either stored in static class properties like `ctorParameters` or within TypeScript's `design:params`.

If Angular comes across a class that does not have any parameter metadata, it tries to detect if the
given class is actually delegating to an inherited class. It does this naively in JIT by checking if the
stringified class (function in ES5) matches a certain pattern. e.g.

```js
function MatTable() {
  var _this = _super.apply(this, arguments) || this;
```

These patterns are reluctant to changes of the class output. If a class is not recognized properly, the
DI parameters will be assumed empty and the class is **incorrectly** constructed without arguments.

This actually happened as part of v10 now. Since we downlevel ES2015 to ES5 (instead of previously
compiling sources directly to ES5), the class output changed slightly so that Angular no longer detects
it. e.g.

```js
var _this = _super.apply(this, __spread(arguments)) || this;
```

This happens because the ES2015 output will receive an auto-generated constructor if the class
defines class properties. This constructor is then already containing an explicit `super` call.

```js
export class MatTable extends CdkTable {
    constructor() {
        super(...arguments);
        this.disabled = true;
    }
}
```

If we then downlevel this file to ES5 with `--downlevelIteration`, TypeScript adjusts the `super` call so that
the spread operator is no longer used (not supported in ES5). The resulting super call is different to the
super call that would have been emitted if we would directly transpile to ES5. Ultimately, Angular no
longer detects such classes as having an delegate constructor -> and DI breaks.

We fix this by expanding the rather naive RegExp patterns used for the reflection capabilities
so that downleveled pass-through/delegate constructors are properly detected. There is a risk
of a false-positive as we cannot detect whether `__spread` is actually the TypeScript spread
helper, but given the reflection patterns already make lots of assumptions (e.g. that `super` is
actually the superclass, we should be fine making this assumption too. The false-positive would
not result in a broken app, but rather in unnecessary providers being injected (as a noop).

Fixes angular#38453

PR Close angular#38463

Author: devversion

.circleci/config.yml

@@ -656,6 +656,18 @@ jobs:
       - run: yarn tsc -p packages
       - run: yarn tsc -p modules
       - run: yarn bazel build //packages/zone.js:npm_package
+      # Build test fixtures for a test that rely on Bazel-generated fixtures. Note that disabling
+      # specific tests which are reliant on such generated fixtures is not an option as SystemJS
+      # in the Saucelabs legacy job always fetches referenced files, even if the imports would be
+      # guarded by an check to skip in the Saucelabs legacy job. We should be good running such
+      # test in all supported browsers on Saucelabs anyway until this job can be removed.
+      - run:
+          name: Preparing Bazel-generated fixtures required in legacy tests
+          command: |
+            yarn bazel build //packages/core/test:downleveled_es5_fixture
+            # Needed for the ES5 downlevel reflector test in `packages/core/test/reflection`.
+            cp dist/bin/packages/core/test/reflection/es5_downleveled_inheritance_fixture.js \
+              dist/all/@angular/core/test/reflection/es5_downleveled_inheritance_fixture.js
       - run:
           # Waiting on ready ensures that we don't run tests too early without Saucelabs not being ready.
           name: Waiting for Saucelabs tunnel to connect

karma-js.conf.js

@@ -37,6 +37,9 @@ module.exports = function(config) {
 
       'node_modules/core-js/client/core.js',
       'node_modules/jasmine-ajax/lib/mock-ajax.js',
+
+      // Dependencies built by Bazel. See `config.yml` for steps running before
+      // the legacy Saucelabs tests run.
       'dist/bin/packages/zone.js/npm_package/bundles/zone.umd.js',
       'dist/bin/packages/zone.js/npm_package/bundles/zone-testing.umd.js',
       'dist/bin/packages/zone.js/npm_package/bundles/task-tracking.umd.js',

packages/core/src/reflection/reflection_capabilities.ts

@@ -17,14 +17,45 @@ import {GetterFn, MethodFn, SetterFn} from './types';
 
 
 
+/*
+ * #########################
+ * Attention: These Regular expressions have to hold even if the code is minified!
+ * ##########################
+ */
+
 /**
- * Attention: These regex has to hold even if the code is minified!
+ * Regular expression that detects pass-through constructors for ES5 output. This Regex
+ * intends to capture the common delegation pattern emitted by TypeScript and Babel. Also
+ * it intends to capture the pattern where existing constructors have been downleveled from
+ * ES2015 to ES5 using TypeScript w/ downlevel iteration. e.g.
+ *
+ *  * ```
+ *   function MyClass() {
+ *     var _this = _super.apply(this, arguments) || this;
+ * ```
+ *
+ * ```
+ *   function MyClass() {
+ *     var _this = _super.apply(this, __spread(arguments)) || this;
+ * ```
+ *
+ * More details can be found in: https://github.com/angular/angular/issues/38453.
  */
-export const DELEGATE_CTOR = /^function\s+\S+\(\)\s*{[\s\S]+\.apply\(this,\s*arguments\)/;
-export const INHERITED_CLASS = /^class\s+[A-Za-z\d$_]*\s*extends\s+[^{]+{/;
-export const INHERITED_CLASS_WITH_CTOR =
+export const ES5_DELEGATE_CTOR =
+    /^function\s+\S+\(\)\s*{[\s\S]+\.apply\(this,\s*(arguments|[^()]+\(arguments\))\)/;
+/** Regular expression that detects ES2015 classes which extend from other classes. */
+export const ES2015_INHERITED_CLASS = /^class\s+[A-Za-z\d$_]*\s*extends\s+[^{]+{/;
+/**
+ * Regular expression that detects ES2015 classes which extend from other classes and
+ * have an explicit constructor defined.
+ */
+export const ES2015_INHERITED_CLASS_WITH_CTOR =
     /^class\s+[A-Za-z\d$_]*\s*extends\s+[^{]+{[\s\S]*constructor\s*\(/;
-export const INHERITED_CLASS_WITH_DELEGATE_CTOR =
+/**
+ * Regular expression that detects ES2015 classes which extend from other classes
+ * and inherit a constructor.
+ */
+export const ES2015_INHERITED_CLASS_WITH_DELEGATE_CTOR =
     /^class\s+[A-Za-z\d$_]*\s*extends\s+[^{]+{[\s\S]*constructor\s*\(\)\s*{\s*super\(\.\.\.arguments\)/;
 
 /**
@@ -36,8 +67,9 @@ export const INHERITED_CLASS_WITH_DELEGATE_CTOR =
  * an initialized instance property.
  */
 export function isDelegateCtor(typeStr: string): boolean {
-  return DELEGATE_CTOR.test(typeStr) || INHERITED_CLASS_WITH_DELEGATE_CTOR.test(typeStr) ||
-      (INHERITED_CLASS.test(typeStr) && !INHERITED_CLASS_WITH_CTOR.test(typeStr));
+  return ES5_DELEGATE_CTOR.test(typeStr) ||
+      ES2015_INHERITED_CLASS_WITH_DELEGATE_CTOR.test(typeStr) ||
+      (ES2015_INHERITED_CLASS.test(typeStr) && !ES2015_INHERITED_CLASS_WITH_CTOR.test(typeStr));
 }
 
 export class ReflectionCapabilities implements PlatformReflectionCapabilities {

packages/core/test/BUILD.bazel

@@ -15,13 +15,31 @@ circular_dependency_test(
     deps = ["//packages/core/testing"],
 )
 
+genrule(
+    name = "downleveled_es5_fixture",
+    srcs = ["reflection/es2015_inheritance_fixture.ts"],
+    outs = ["reflection/es5_downleveled_inheritance_fixture.js"],
+    cmd = """
+        es2015_out_dir="/tmp/downleveled_es5_fixture/"
+        es2015_out_file="$$es2015_out_dir/es2015_inheritance_fixture.js"
+
+        # Build the ES2015 output and then downlevel it to ES5.
+        $(execpath @npm//typescript/bin:tsc) $< --outDir $$es2015_out_dir --target ES2015 \
+            --types --module umd
+        $(execpath @npm//typescript/bin:tsc) --outFile $@ $$es2015_out_file --allowJs \
+            --types --target ES5
+    """,
+    tools = ["@npm//typescript/bin:tsc"],
+)
+
 ts_library(
     name = "test_lib",
     testonly = True,
     srcs = glob(
         ["**/*.ts"],
         exclude = [
             "**/*_node_only_spec.ts",
+            "reflection/es2015_inheritance_fixture.ts",
         ],
     ),
     # Visible to //:saucelabs_unit_tests_poc target
@@ -73,6 +91,9 @@ ts_library(
 jasmine_node_test(
     name = "test",
     bootstrap = ["//tools/testing:node_es5"],
+    data = [
+        ":downleveled_es5_fixture",
+    ],
     shard_count = 4,
     deps = [
         ":test_lib",
@@ -87,6 +108,7 @@ jasmine_node_test(
 
 karma_web_test_suite(
     name = "test_web",
+    runtime_deps = [":downleveled_es5_fixture"],
     deps = [
         ":test_lib",
     ],

packages/core/test/reflection/es2015_inheritance_fixture.ts

@@ -0,0 +1,22 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+// AMD module name is required so that this file can be loaded in the Karma tests.
+/// <amd-module name="angular/packages/core/test/reflection/es5_downleveled_inheritance_fixture" />
+
+class Parent {}
+
+export class ChildNoCtor extends Parent {}
+export class ChildWithCtor extends Parent {
+  constructor() {
+    super();
+  }
+}
+export class ChildNoCtorPrivateProps extends Parent {
+  x = 10;
+}

packages/core/test/reflection/reflector_spec.ts

@@ -201,6 +201,16 @@ class TestObj {
         expect(isDelegateCtor(ChildWithCtor.toString())).toBe(false);
       });
 
+      // See: https://github.com/angular/angular/issues/38453
+      it('should support ES2015 downleveled classes', () => {
+        const {ChildNoCtor, ChildNoCtorPrivateProps, ChildWithCtor} =
+            require('./es5_downleveled_inheritance_fixture');
+
+        expect(isDelegateCtor(ChildNoCtor.toString())).toBe(true);
+        expect(isDelegateCtor(ChildNoCtorPrivateProps.toString())).toBe(true);
+        expect(isDelegateCtor(ChildWithCtor.toString())).toBe(false);
+      });
+
       it('should support ES2015 classes when minified', () => {
         // These classes are ES2015 in minified form
         const ChildNoCtorMinified = 'class ChildNoCtor extends Parent{}';