Disable the tsickle pass from ng_package/APF/ngc-wrapped#37221
Disable the tsickle pass from ng_package/APF/ngc-wrapped#37221IgorMinar wants to merge 1 commit intoangular:masterfrom
Conversation
c7627d0 to
62e84a5
Compare
|
You can preview 62e84a5 at https://pr37221-62e84a5.ngbuilds.io/. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
6962b4d to
afbf3d2
Compare
|
You can preview afbf3d2 at https://pr37221-afbf3d2.ngbuilds.io/. |
|
You can preview 9958227 at https://pr37221-9958227.ngbuilds.io/. |
|
Just sharing as it seems relevant: For the We ran into something similar in the components repo when tsickle had been disabled in the compiler CLI: 29761ea. Clearly this is not a common case since build-optimizer is used by default in CLI apps, and AFAIK the output from famework packages is not affected, but components repo would be affected. We can either make the switch away from tsickle now too, or keep tsickle enabled there. To be clear: I'm not advocating for keeping tsickle enabled, I'm just sharing an related issue that came up in the past. I thought it would be worth mentioning here. I think it's reasonable moving forward with this as there is a straightforward solution for affected apps (i.e. using build optimizer or providing global mocks), but it might be breaking in some cases. More context on the general issue: #30586 |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
c3d3aa5 to
5a8de37
Compare
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
@devversion thanks for the comment. I agree that we should try to decouple the two. My main objective here is not to use tsickle in the npm package output. Do you believe that the change as currently proposed will become a blocker for the components repo? |
|
You can preview 5a8de37 at https://pr37221-5a8de37.ngbuilds.io/. |
5a8de37 to
96c150b
Compare
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
d65f30c to
3f7c5c4
Compare
|
You can preview 3f7c5c4 at https://pr37221-3f7c5c4.ngbuilds.io/. |
3f7c5c4 to
08bec1b
Compare
|
You can preview 08bec1b at https://pr37221-08bec1b.ngbuilds.io/. |
f0ea310 to
b027ccb
Compare
As of TypeScript 3.9, the tsc emit is not compatible with Closure Compiler due to microsoft/TypeScript#32011. There is some hope that this will be fixed by a solution like the one proposed in microsoft/TypeScript#38374 but currently it's unclear if / when that will happen. Since the Closure support has been somewhat already broken, and the tsickle pass has been a source of headaches for some time for Angular packages, we are removing it for now while we rethink our strategy to make Angular Closure compatible outside of Google. This change has no effect on our Closure compatibility within Google which work well because all the code is compiled from sources and passed through tsickle. This change only disables the tsickle pass but doesn't remove it. A follow up PR should either remove all the traces of tscikle or re-enable the fixed version. BREAKING CHANGE: Angular npm packages no longer contain jsdoc comments to support Closure Compiler's advanced optimizations The support for Closure compiler in Angular packages has been experimental and broken for quite some time. As of TS3.9 Closure is unusable with the JavaScript emit. Please follow microsoft/TypeScript#38374 for more information and updates. If you used Closure compiler with Angular in the past, you will likely be better off consuming Angular packages built from sources directly rather than consuming the version we publish on npm which is primarily optimized for Webpack/Rollup + Terser build pipeline. As a temporary workaround you might consider using your current build pipeline with Closure flag `--compilation_level=SIMPLE`. This flag will ensure that your build pipeline produces buildable and runnable artifacts, at the cost of increased payload size due to advanced optimizations being disabled. If you were affected by this change, please help us understand your needs by leaving a comment on angular#37234.
b027ccb to
eb6cf23
Compare
|
You can preview eb6cf23 at https://pr37221-eb6cf23.ngbuilds.io/. |
kara
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: global-approvers
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
…37317) In #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 #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 #37317
…37317) In #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 #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 #37317
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…37221) As of TypeScript 3.9, the tsc emit is not compatible with Closure Compiler due to microsoft/TypeScript#32011. There is some hope that this will be fixed by a solution like the one proposed in microsoft/TypeScript#38374 but currently it's unclear if / when that will happen. Since the Closure support has been somewhat already broken, and the tsickle pass has been a source of headaches for some time for Angular packages, we are removing it for now while we rethink our strategy to make Angular Closure compatible outside of Google. This change has no effect on our Closure compatibility within Google which work well because all the code is compiled from sources and passed through tsickle. This change only disables the tsickle pass but doesn't remove it. A follow up PR should either remove all the traces of tscikle or re-enable the fixed version. BREAKING CHANGE: Angular npm packages no longer contain jsdoc comments to support Closure Compiler's advanced optimizations The support for Closure compiler in Angular packages has been experimental and broken for quite some time. As of TS3.9 Closure is unusable with the JavaScript emit. Please follow microsoft/TypeScript#38374 for more information and updates. If you used Closure compiler with Angular in the past, you will likely be better off consuming Angular packages built from sources directly rather than consuming the version we publish on npm which is primarily optimized for Webpack/Rollup + Terser build pipeline. As a temporary workaround you might consider using your current build pipeline with Closure flag `--compilation_level=SIMPLE`. This flag will ensure that your build pipeline produces buildable and runnable artifacts, at the cost of increased payload size due to advanced optimizations being disabled. If you were affected by this change, please help us understand your needs by leaving a comment on angular#37234. PR Close angular#37221
As of TypeScript 3.9, the tsc emit is not compatible with Closure Compiler due to microsoft/TypeScript#32011.
There is some hope that this will be fixed by a solution like the one proposed in microsoft/TypeScript#38374 but currently it's unclear if / when that will happen.
Since the Closure support has been somewhat already broken, and the
tsickle pass has been a source of headaches for some time for Angular packages, we are removing it for now while we rethink our strategy to make Angular Closure compatible outside of Google.
This change has no effect on our Closure compatibility within Google which work well because all the code is compiled from sources and passed through tsickle.
This change only disables the tsickle pass but doesn't remove it.
A follow up PR should either remove all the traces of tscikle or re-enable the fixed version.
BREAKING CHANGE: Angular npm packages no longer contain jsdoc comments to support Closure Compiler's advanced optimizations
The support for Closure compiler in Angular packages has been experimental and broken for quite some time.
As of TS3.9 Closure is unusable with the JavaScript emit. Please follow microsoft/TypeScript#38374 for more information and updates.
If you used Closure compiler with Angular in the past, you will likely
be better off consuming Angular packages built from sources directly rather than consuming the version we publish on npm which is primarily optimized for Webpack/Rollup + Terser build pipeline.
As a temporary workaround you might consider using your current build pipeline with Closure flag
--compilation_level=SIMPLE. This flag will ensure that your build pipeline produces buildable and runnable artifacts, at the cost of increased payload size due to advanced optimizations being disabled.If you were affected by this change, please help us understand your needs by leaving a comment on #37234.