Skip to content

Wrap classes with decorators or static properties in an IIFE, even for ES2015+#32011

Merged
sandersn merged 2 commits intomicrosoft:masterfrom
clydin:wrap-es2015
Feb 15, 2020
Merged

Wrap classes with decorators or static properties in an IIFE, even for ES2015+#32011
sandersn merged 2 commits intomicrosoft:masterfrom
clydin:wrap-es2015

Conversation

@clydin
Copy link
Contributor

@clydin clydin commented Jun 20, 2019 •
edited
Loading

Currently only script targets less than or equal to ES5 will wrap classes with decorators and/or static properties. However, the wrapping is also crucial to file size optimizations for ES2015+ as well. Without the IIFE wrapper, minification tools do not elide the class. This is due to references to the class being present within the downlevelled decorator and static property code.
This change represents the full completion of issue #15857

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.6.1 milestone Jul 30, 2019
@clydin clydin force-pushed the wrap-es2015 branch 3 times, most recently from 52eb471 to 7b5cbf6 Compare August 29, 2019 15:23
sandersn
sandersn requested changes Feb 3, 2020
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clydin Can you open a separate issue to explain how this fixes #15857? It was already fixed by #16120 when you opened this, so I think that it must be an extension of that somehow.

@rbuckton you might want to take a look now and see how this relates to #16120.

@clydin
Copy link
Contributor Author

clydin commented Feb 3, 2020 •
edited
Loading

@sandersn Issue #15857 was only partially fixed by #16120. That PR only fixed the issue when the tsconfig target was ES5. It did not fix the issue when the target was ES2015 or higher.
This was not so much of a problem at the time since targeting ES2015+ was uncommon. However, now that ES2015 browser support is commonplace, targeting such versions is now the default in many cases.

@sandersn sandersn changed the title Always wrap classes with decorators or static properties in an IIFE Wrap classes with decorators or static properties in an IIFE, even for ES2015+ Feb 3, 2020
@rbuckton
Copy link
Contributor

rbuckton commented Feb 4, 2020

For ES5, we were asked to mark the IIFE with a /** @class */ leading comment for some minifiers. Should we do this here too?

TypeScript/src/compiler/transformers/es2015.ts

Line 790 in 2e97918

addSyntheticLeadingComment(result, SyntaxKind.MultiLineCommentTrivia, "* @class ");

@clydin
Copy link
Contributor Author

clydin commented Feb 5, 2020

Good question. I think that makes sense considering that the IIFE represents the totality of the Typescript class definition.

clydin added 2 commits February 6, 2020 10:59
@clydin
Always wrap classes with decorators or static properties in an IIFE
6ef2923 
Currently only script targets less than or equal to ES5 will wrap classes.  However, the wrapping is also crucial to file size optimizations for ES2015+ as well.  Without the IIFE wrapper, minification tools do not elide the class.  This is due to references to the class being present within the downlevelled decorator and static property code.
This change represents the full completion of issue microsoft#15857
@clydin
Accept new baselines
9ff43af
@clydin
Copy link
Contributor Author

clydin commented Feb 11, 2020

Updated to add the @class comment.

@sandersn sandersn merged commit 7cc4a8d into microsoft:master Feb 15, 2020
@clydin clydin deleted the wrap-es2015 branch February 15, 2020 00:37
@weswigham weswigham mentioned this pull request Feb 20, 2020
Merged
@rrdelaney rrdelaney mentioned this pull request May 6, 2020
Open
5 tasks
@alan-agius4 alan-agius4 mentioned this pull request May 8, 2020
Merged
clydin pushed a commit to angular/angular-cli that referenced this pull request May 11, 2020
@alan-agius4 @clydin
build: vendor typescript 3.6 in build optimizer
56ffa1c 
With this change we "vendor" TypeScript 3.6 in build optimizer instead of using directly the npm package.

Reasons:
- We cannot update to a more recent version of TypeScript due to microsoft/TypeScript#38412
- Yarn workspace are not supported under Bazel. This means currently we are running unit testswhich uses a different TypeScript version at runtime.
- In TypeScript 3.9, the shape of ES2015 classes changed microsoft/TypeScript#32011, this requires some changes in the UT expects, but this again will not be correct to change now.
@robpalme robpalme mentioned this pull request May 13, 2020
Closed
@rrdelaney rrdelaney mentioned this pull request May 15, 2020
Closed
@IgorMinar IgorMinar mentioned this pull request May 20, 2020
Closed
@IgorMinar IgorMinar mentioned this pull request May 21, 2020
Closed
IgorMinar added a commit to IgorMinar/angular that referenced this pull request May 21, 2020
@IgorMinar
fix(core): disable tsickle pass when producing APF packages
eb6cf23 
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.
kara pushed a commit to angular/angular that referenced this pull request May 21, 2020
@IgorMinar @kara
fix(core): disable tsickle pass when producing APF packages (#37221)
a1001f2 
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.

PR Close #37221
@DanielRosenwasser DanielRosenwasser mentioned this pull request May 21, 2020
Closed
sandersn added a commit that referenced this pull request May 21, 2020
@sandersn
Revert "Wrap classes with decorators or static properties in an IIFE,…
79f1ec3 
… even for ES2015+ (#32011)"

This reverts commit 7cc4a8d.
@sandersn sandersn mentioned this pull request May 21, 2020
Merged
cangSDARM added a commit to cangSDARM/TypeScript that referenced this pull request May 26, 2020
@cangSDARM
Merge remote-tracking branch 'upstream/master'
470bcf6 
* upstream/master:
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Fix relation between generic mapped types and types with index signatures (microsoft#38761)
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Revert "Wrap classes with decorators or static properties in an IIFE, even for ES2015+ (microsoft#32011)"
  Preserve module marker in es2015+ module emit for tool compatability (microsoft#38712)
  Fix recently added test so it is cross-plat
  fix(33286): add outlining for arrow function with one parameter (microsoft#38631)
  fix(38463): show completions for index access properties
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
@IgorMinar @profanis
fix(core): disable tsickle pass when producing APF packages (angular#…
f879596 
…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
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn sandersn sandersn approved these changes

+1 more reviewer

@rbuckton rbuckton rbuckton approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rbuckton rbuckton

Labels

None yet

Projects

PR Backlog
Archived in project

Milestone

TypeScript 3.7.1

Development

Successfully merging this pull request may close these issues.

5 participants

@clydin @rbuckton @sandersn @DanielRosenwasser @RyanCavanaugh

Comments