Skip to content

fix(core): reenable decorator downleveling for Angular npm packages #37317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
IgorMinar wants to merge 1 commit into from
Closed

fix(core): reenable decorator downleveling for Angular npm packages #37317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/animations-browser/esm2015.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import "tslib";

import "@angular/core";

import "@angular/common";
Expand Down
2 changes: 0 additions & 2 deletions integration/side-effects/snapshots/platform-browser/esm2015.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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';
Comment thread
IgorMinar marked this conversation as resolved.
Outdated
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