fix(compiler-cli): downlevel angular decorators to static properties

In v7 of Angular we removed `tsickle` from the default `ngc` pipeline.
This had the negative potential of breaking ES2015 output and SSR:

(1): TypeScript has a limitation where `forwardRef` breaks in es2015 due to
`forwardRef` calls being invoked immediately as part of decorator metadata.
See: angular/angular-cli#14424 and
angular#30106.

(2): TypeScript preserves type information for class members, and
references the type values immediately (as with `forwardRef` above).
This means that using native DOM globals as property types could
break SSR. This is because loading such JS file requires these DOM
globals to exist in NodeJS globally too (and there is no support for DI mocking).
See: angular#30586. This is especially
relevant for libraries that do not want to use tsickle but ship SSR-compatible code.

The root cause for (1) is a TypeScript limitation as mentioned. This is
the related upstream ticket: microsoft/TypeScript#27519.
Downleveling decorators to static properties fixes the issues, as
outlined in (1) and (2), because we can defer metadata evaluation
to avoid direct evaluation on file load. Additionally, we have more
control and can discard unnnecessary metadata information, like class
member types that are not needed by Angular at all see (2).

One might wonder why this hasn't been an issue in the past since we disabled
this as part of version 7. These issues didn't surface at a large scale because
we added a custom transformer to CLI projects and to `ng-packagr`. Those
transformers downleveled constructor parameters/ore removed decorators
at all to fix (1) and (2). Also `emitMetadataDecorator` has been disabled
by default in CLI projects. For bazel release output this didn't surface
either because tsickle still ran by default in prodmode output.

This was never an ideal solution though, and we'd also like to not
run tsickle by default in the Bazel prodmode output. It was not ideal
because we just applied workarounds at Angular compiler derivatives.
Ideally, TypeScript would just emit proper metadata that isn't evaluated
at top-level, but given they marked it as limitation and the decorator
proposal is still stage 2, this won't happen any time soon (if at all).

The ideal solution is that we downlevel decorators (as previously done
with tsickle by default) as part of the Angular compiler (a level higher;
and one below the actual TypeScript compiler limitation). This fixes
the issues with the common `forwardRef` pattern (1), and also fixes (2).
It also allows us to reduce code duplication in the compiler derivatives
(e.g. ng-packagr), fixes the left-behind standalone ngc comsumers, and
we can disable tsickle in Angular bazel (as already done with this commit).

Fixes angular#30106. Fixes angular#30586. Fixes angular#30141.
Resolves FW-2196. Resolves FW-2199.

Author: devversion

packages/bazel/src/ngc-wrapped/index.ts

@@ -191,23 +191,8 @@ export function compile({
     fileLoader = new UncachedFileLoader();
   }
 
-  compilerOpts.annotationsAs = 'static fields';
-  if (!bazelOpts.es5Mode) {
-    if (bazelOpts.workspaceName === 'google3') {
-      compilerOpts.annotateForClosureCompiler = true;
-    } else {
-      compilerOpts.annotateForClosureCompiler = false;
-    }
-  }
-
   // Detect from compilerOpts whether the entrypoint is being invoked in Ivy mode.
   const isInIvyMode = !!compilerOpts.enableIvy;
-
-  // Disable downleveling and Closure annotation if in Ivy mode.
-  if (isInIvyMode) {
-    compilerOpts.annotationsAs = 'decorators';
-  }
-
   if (!compilerOpts.rootDirs) {
     throw new Error('rootDirs is not set!');
   }
@@ -264,9 +249,6 @@ export function compile({
   }
 
   if (isInIvyMode) {
-    // Also need to disable decorator downleveling in the BazelHost in Ivy mode.
-    bazelHost.transformDecorators = false;
-
     const delegate = bazelHost.shouldSkipTsickleProcessing.bind(bazelHost);
     bazelHost.shouldSkipTsickleProcessing = (fileName: string) => {
       // The base implementation of shouldSkipTsickleProcessing checks whether `fileName` is part of
@@ -277,12 +259,23 @@ export function compile({
     };
   }
 
+  // Always disable tsickle decorator transforming in the tsickle compiler host. The
+  // Angular compilers have their own logic for decorator processing and we wouldn't
+  // want tsickle to interfere with that.
+  bazelHost.transformDecorators = false;
+
+  // By default, annotations for closure compiler are disabled in prodmode,
+  // unless the workspace is known to be `google3`.
+  if (!bazelOpts.es5Mode && compilerOpts.annotateForClosureCompiler !== undefined) {
+    compilerOpts.annotateForClosureCompiler = bazelOpts.workspaceName === 'google3';
+  }
+
+  // The `annotateForClosureCompiler` Angular compiler option is not respected by default
+  // as ngc-wrapped handles tsickle emit on its own. This means that we need to update
+  // the tsickle compiler host based on the `annotateForClosureCompiler` flag.
   if (compilerOpts.annotateForClosureCompiler) {
     bazelHost.transformTypesToClosure = true;
   }
-  if (compilerOpts.annotateForClosureCompiler || compilerOpts.annotationsAs === 'static fields') {
-    bazelHost.transformDecorators = true;
-  }
 
   const origBazelHostFileExist = bazelHost.fileExists;
   bazelHost.fileExists = (fileName: string) => {

packages/bazel/test/ng_package/example_package.golden

@@ -274,7 +274,6 @@ Hello
         MyService.decorators = [
             { type: i0.Injectable, args: [{ providedIn: 'root' },] }
         ];
-        /** @nocollapse */
         MyService.ctorParameters = function () { return [
             { type: MySecondService }
         ]; };
@@ -605,15 +604,14 @@ let MyService = /** @class */ (() => {
     MyService.decorators = [
         { type: Injectable, args: [{ providedIn: 'root' },] }
     ];
-    /** @nocollapse */
     MyService.ctorParameters = () => [
         { type: MySecondService }
     ];
     MyService.ɵprov = i0.ɵɵdefineInjectable({ factory: function MyService_Factory() { return new MyService(i0.ɵɵinject(i1.MySecondService)); }, token: MyService, providedIn: "root" });
     return MyService;
 })();
 export { MyService };
-//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicHVibGljLWFwaS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbIi4uLy4uLy4uLy4uLy4uLy4uLy4uLy4uLy4uL3BhY2thZ2VzL2JhemVsL3Rlc3QvbmdfcGFja2FnZS9leGFtcGxlL2ltcG9ydHMvcHVibGljLWFwaS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTs7Ozs7O0dBTUc7QUFFSCxPQUFPLEVBQUMsVUFBVSxFQUFDLE1BQU0sZUFBZSxDQUFDO0FBQ3pDLE9BQU8sRUFBQyxlQUFlLEVBQUMsTUFBTSxVQUFVLENBQUM7OztBQUV6QztJQUFBLE1BQ2EsU0FBUztRQUNwQixZQUFtQixhQUE4QjtZQUE5QixrQkFBYSxHQUFiLGFBQWEsQ0FBaUI7UUFBRyxDQUFDOzs7Z0JBRnRELFVBQVUsU0FBQyxFQUFDLFVBQVUsRUFBRSxNQUFNLEVBQUM7Ozs7Z0JBRnhCLGVBQWU7OztvQkFUdkI7S0FjQztTQUZZLFNBQVMiLCJzb3VyY2VzQ29udGVudCI6WyIvKipcbiAqIEBsaWNlbnNlXG4gKiBDb3B5cmlnaHQgR29vZ2xlIExMQyBBbGwgUmlnaHRzIFJlc2VydmVkLlxuICpcbiAqIFVzZSBvZiB0aGlzIHNvdXJjZSBjb2RlIGlzIGdvdmVybmVkIGJ5IGFuIE1JVC1zdHlsZSBsaWNlbnNlIHRoYXQgY2FuIGJlXG4gKiBmb3VuZCBpbiB0aGUgTElDRU5TRSBmaWxlIGF0IGh0dHBzOi8vYW5ndWxhci5pby9saWNlbnNlXG4gKi9cblxuaW1wb3J0IHtJbmplY3RhYmxlfSBmcm9tICdAYW5ndWxhci9jb3JlJztcbmltcG9ydCB7TXlTZWNvbmRTZXJ2aWNlfSBmcm9tICcuL3NlY29uZCc7XG5cbkBJbmplY3RhYmxlKHtwcm92aWRlZEluOiAncm9vdCd9KVxuZXhwb3J0IGNsYXNzIE15U2VydmljZSB7XG4gIGNvbnN0cnVjdG9yKHB1YmxpYyBzZWNvbmRTZXJ2aWNlOiBNeVNlY29uZFNlcnZpY2UpIHt9XG59XG4iXX0=
+//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicHVibGljLWFwaS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbIi4uLy4uLy4uLy4uLy4uLy4uLy4uLy4uLy4uL3BhY2thZ2VzL2JhemVsL3Rlc3QvbmdfcGFja2FnZS9leGFtcGxlL2ltcG9ydHMvcHVibGljLWFwaS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTs7Ozs7O0dBTUc7QUFFSCxPQUFPLEVBQUMsVUFBVSxFQUFDLE1BQU0sZUFBZSxDQUFDO0FBQ3pDLE9BQU8sRUFBQyxlQUFlLEVBQUMsTUFBTSxVQUFVLENBQUM7OztBQUV6QztJQUFBLE1BQ2EsU0FBUztRQUNwQixZQUFtQixhQUE4QjtZQUE5QixrQkFBYSxHQUFiLGFBQWEsQ0FBaUI7UUFBRyxDQUFDOzs7Z0JBRnRELFVBQVUsU0FBQyxFQUFDLFVBQVUsRUFBRSxNQUFNLEVBQUM7OztnQkFFSSxlQUFlOzs7b0JBYm5EO0tBY0M7U0FGWSxTQUFTIiwic291cmNlc0NvbnRlbnQiOlsiLyoqXG4gKiBAbGljZW5zZVxuICogQ29weXJpZ2h0IEdvb2dsZSBMTEMgQWxsIFJpZ2h0cyBSZXNlcnZlZC5cbiAqXG4gKiBVc2Ugb2YgdGhpcyBzb3VyY2UgY29kZSBpcyBnb3Zlcm5lZCBieSBhbiBNSVQtc3R5bGUgbGljZW5zZSB0aGF0IGNhbiBiZVxuICogZm91bmQgaW4gdGhlIExJQ0VOU0UgZmlsZSBhdCBodHRwczovL2FuZ3VsYXIuaW8vbGljZW5zZVxuICovXG5cbmltcG9ydCB7SW5qZWN0YWJsZX0gZnJvbSAnQGFuZ3VsYXIvY29yZSc7XG5pbXBvcnQge015U2Vjb25kU2VydmljZX0gZnJvbSAnLi9zZWNvbmQnO1xuXG5ASW5qZWN0YWJsZSh7cHJvdmlkZWRJbjogJ3Jvb3QnfSlcbmV4cG9ydCBjbGFzcyBNeVNlcnZpY2Uge1xuICBjb25zdHJ1Y3RvcihwdWJsaWMgc2Vjb25kU2VydmljZTogTXlTZWNvbmRTZXJ2aWNlKSB7fVxufVxuIl19
 
 --- esm2015/imports/second.js ---
 
@@ -835,7 +833,6 @@ let MyService = /** @class */ (() => {
     MyService.decorators = [
         { type: Injectable, args: [{ providedIn: 'root' },] }
     ];
-    /** @nocollapse */
     MyService.ctorParameters = () => [
         { type: MySecondService }
     ];

packages/compiler-cli/BUILD.bazel

@@ -29,6 +29,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/file_system",
         "//packages/compiler-cli/src/ngtsc/indexer",
         "//packages/compiler-cli/src/ngtsc/perf",
+        "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "@npm//@bazel/typescript",
         "@npm//@types/node",

packages/compiler-cli/src/main.ts

@@ -89,18 +89,9 @@ export function mainDiagnosticsForTest(
 }
 
 function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|undefined {
-  const transformDecorators =
-      (options.enableIvy === false && options.annotationsAs !== 'decorators');
-  const transformTypesToClosure = options.annotateForClosureCompiler;
-  if (!transformDecorators && !transformTypesToClosure) {
+  if (!options.annotateForClosureCompiler) {
     return undefined;
   }
-  if (transformDecorators) {
-    // This is needed as a workaround for https://github.com./angular/tsickle/issues/635
-    // Otherwise tsickle might emit references to non imported values
-    // as TypeScript elided the import.
-    options.emitDecoratorMetadata = true;
-  }
   const tsickleHost: Pick<
       tsickle.TsickleHost,
       'shouldSkipTsickleProcessing'|'pathToModuleName'|'shouldIgnoreWarningsForPath'|
@@ -115,41 +106,29 @@ function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|un
     googmodule: false,
     untyped: true,
     convertIndexImportShorthand: false,
-    transformDecorators,
-    transformTypesToClosure,
+    // Decorators are transformed as part of the Angular compiler programs. To avoid
+    // conflicts, we disable decorator transformations for tsickle.
+    transformDecorators: false,
+    transformTypesToClosure: true,
   };
 
-  if (options.annotateForClosureCompiler || options.annotationsAs === 'static fields') {
-    return ({
-             program,
-             targetSourceFile,
-             writeFile,
-             cancellationToken,
-             emitOnlyDtsFiles,
-             customTransformers = {},
-             host,
-             options
-           }) =>
-               // tslint:disable-next-line:no-require-imports only depend on tsickle if requested
-        require('tsickle').emitWithTsickle(
-            program, {...tsickleHost, options, host, moduleResolutionHost: host}, host, options,
-            targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, {
-              beforeTs: customTransformers.before,
-              afterTs: customTransformers.after,
-            });
-  } else {
-    return ({
-             program,
-             targetSourceFile,
-             writeFile,
-             cancellationToken,
-             emitOnlyDtsFiles,
-             customTransformers = {},
-           }) =>
-               program.emit(
-                   targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles,
-                   {after: customTransformers.after, before: customTransformers.before});
-  }
+  return ({
+           program,
+           targetSourceFile,
+           writeFile,
+           cancellationToken,
+           emitOnlyDtsFiles,
+           customTransformers = {},
+           host,
+           options
+         }) =>
+             // tslint:disable-next-line:no-require-imports only depend on tsickle if requested
+      require('tsickle').emitWithTsickle(
+          program, {...tsickleHost, options, host, moduleResolutionHost: host}, host, options,
+          targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, {
+            beforeTs: customTransformers.before,
+            afterTs: customTransformers.after,
+          });
 }
 
 export interface NgcParsedConfiguration extends ParsedConfiguration {

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -754,7 +754,7 @@ export class NgCompiler {
 /**
  * Determine if the given `Program` is @angular/core.
  */
-function isAngularCorePackage(program: ts.Program): boolean {
+export function isAngularCorePackage(program: ts.Program): boolean {
   // Look for its_just_angular.ts somewhere in the program.
   const r3Symbols = getR3SymbolsFile(program);
   if (r3Symbols === null) {