forked from angular/angular
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
1 parent
1b55da1
commit d884e61
Showing
13 changed files
with
1,220 additions
and
362 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.