Skip to content

Commit

Permalink
fix(core): reenable decorator downleveling for Angular npm packages (a…
Browse files Browse the repository at this point in the history
…ngular#37317)

In angular#37221 we disabled tsickle passes from transforming the tsc output that is used to publish all
Angular framework and components packages (@angular/*).

This change however revealed a bug in the ngc that caused __decorate and __metadata calls to still
be emitted in the JS code even though we don't depend on them.

Additionally it was these calls that caused code in @angular/material packages to fail at runtime
due to circular dependency in the emitted decorator code documeted as
microsoft/TypeScript#27519.

This change partially rolls back angular#37221 by reenabling the decorator to static fields (static
properties) downleveling.

This is just a temporary workaround while we are also fixing root cause in `ngc` - tracked as
FW-2199.

Resolves FW-2198.
Related to FW-2196

PR Close angular#37317
  • Loading branch information
IgorMinar authored and ngwattcos committed Jun 25, 2020
1 parent 2280c16 commit 021d94a
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 1,355 deletions.
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
"bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated",
"bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.",
"bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338",
"bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1218969, see PR#37221 for more info",
"bundle": 1218969
"bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1200475, see PR#37221 and PR#37317 for more info",
"bundle": 1200475
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions integration/ngcc/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,9 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'."
grep "ApplicationModule.ɵmod = ɵngcc0.ɵɵdefineNgModule" node_modules/@angular/core/esm2015/src/application_module.js
assertSucceeded "Expected 'ngcc' to correctly compile 'ApplicationModule' in '@angular/core' (esm2015)."

# TODO: This assertion is disabled because @angular/core is no longer processed by tsickle which creates the static properties.
# We should either remove this assertion or use a syntentic JS file as input.
# Discuss with the ngcc folks.
#
# Did it place the `setClassMetadata` call correctly?
# cat node_modules/@angular/core/fesm2015/core.js | awk 'ORS=" "' | grep "ApplicationRef.ctorParameters.*setClassMetadata(ApplicationRef"
# assertSucceeded "Expected 'ngcc' to place 'setClassMetadata' after static properties like 'ctorParameters' in '@angular/core' (fesm2015)."
cat node_modules/@angular/core/fesm2015/core.js | awk 'ORS=" "' | grep "ApplicationRef.ctorParameters.*setClassMetadata(ApplicationRef"
assertSucceeded "Expected 'ngcc' to place 'setClassMetadata' after static properties like 'ctorParameters' in '@angular/core' (fesm2015)."


# Did it transform @angular/core typing files correctly?
Expand All @@ -123,13 +119,19 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'."
assertSucceeded "Expected 'ngcc' to generate an abstract directive definition for 'MatMenuBase' in '@angular/material' (esm5)."


# TODO: This assertion is disabled because @angular/common no longer contains __decorate calls.
# We should either remove this assertion or use a syntentic JS file as input.
# Discuss with the ngcc folks.
# Did it handle namespace imported decorators in UMD using `__decorate` syntax?
grep "type: i0.Injectable" node_modules/@angular/common/bundles/common.umd.js
assertSucceeded "Expected 'ngcc' to correctly handle '__decorate' syntax in '@angular/common' (umd)."
#grep "type: i0.Injectable" node_modules/@angular/common/bundles/common.umd.js
#assertSucceeded "Expected 'ngcc' to correctly handle '__decorate' syntax in '@angular/common' (umd)."

# TODO: This assertion is disabled because @angular/common no longer contains __decorate calls.
# We should either remove this assertion or use a syntentic JS file as input.
# Discuss with the ngcc folks.
# (and ensure the @angular/common package is indeed using `__decorate` syntax)
grep "JsonPipe = __decorate(" node_modules/@angular/common/bundles/common.umd.js.__ivy_ngcc_bak
assertSucceeded "Expected '@angular/common' (umd) to actually use '__decorate' syntax."
#grep "JsonPipe = __decorate(" node_modules/@angular/common/bundles/common.umd.js.__ivy_ngcc_bak
#assertSucceeded "Expected '@angular/common' (umd) to actually use '__decorate' syntax."


# Did it handle namespace imported decorators in UMD using static properties?
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import "tslib";

import "@angular/animations";

import "@angular/core";
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/common/esm2015.js
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
import "tslib";

import "@angular/core";
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/core/esm2015.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import "rxjs";

import "tslib";

import "rxjs/operators";

const __globalThis = "undefined" !== typeof globalThis && globalThis;
Expand Down
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/forms/esm2015.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import "tslib";

import "@angular/core";

import "@angular/common";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import "tslib";

import "@angular/common";

import "@angular/core";
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/router/esm2015.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import "tslib";

import "@angular/common";

import "@angular/core";
Expand Down
7 changes: 4 additions & 3 deletions packages/bazel/src/ngc-wrapped/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,12 @@ export function compile({
fileLoader = new UncachedFileLoader();
}

compilerOpts.annotationsAs = 'static fields';
if (!bazelOpts.es5Mode) {
if (bazelOpts.workspaceName === 'google3') {
compilerOpts.annotateForClosureCompiler = true;
compilerOpts.annotationsAs = 'static fields';
} else {
compilerOpts.annotateForClosureCompiler = false;
compilerOpts.annotationsAs = 'decorators';
}
}

Expand Down Expand Up @@ -278,11 +277,13 @@ export function compile({
};
}

// Prevent tsickle adding any types at all if we don't want closure compiler annotations.
if (compilerOpts.annotateForClosureCompiler) {
bazelHost.transformTypesToClosure = true;
}
if (compilerOpts.annotateForClosureCompiler || compilerOpts.annotationsAs === 'static fields') {
bazelHost.transformDecorators = true;
}

const origBazelHostFileExist = bazelHost.fileExists;
bazelHost.fileExists = (fileName: string) => {
if (NGC_ASSETS.test(fileName)) {
Expand Down
Loading

0 comments on commit 021d94a

Please sign in to comment.