Skip to content
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.

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

fix(loader): Temporarily stop removing decorators from source files #2881

Closed

Conversation

MikeRyanDev
Copy link
Member

@MikeRyanDev MikeRyanDev commented Oct 25, 2016

Previous behavior was to remove all decorators from a source file when using the
@ngtools/webpack loader. This had the side effect of removing any third party
decorators, silently breaking apps when using the aot production flag. This skips
the decorator removal process until a better way can be determined to remove
Angular-specific decorators.

Closes #2799

Previous behavior was to remove all decorators from a source file when using the
@ngtools/webpack loader. This had the side effect of removing any third party
decorators, potentially breaking apps silently when using the aot produciton flag.
This skips the decorator removal process until a better way can be determined
to remove Angular-specific decorators.

Closes angular#2799
@filipesilva
Copy link
Contributor

@hansl can you weight in?

@hansl
Copy link
Contributor

hansl commented Nov 2, 2016

This is the wrong fix. The reason the decorators are removed is to allow for tree shaking. Otherwise you don't reduce the size by using AoT. The right fix is to use Tsickle (https://github.com/angular/tsickle), which moves decorators into a static property which can then be removed safely (without losing runtime information).

The right fix is to use Tsickle or the compiler-cli to remove the annotations (by moving them into static properties) and keep other decorators around.

This will not be merged in, but a fix for this issue is coming before final.

@hansl hansl closed this Nov 2, 2016
@matt-psaltis
Copy link

Might not be the right approach long-term, but essentially I now have to go and learn what Tsickle is and how to add it to my existing pipeline just to get my existing build ecosystem back online. If a proper fix for this issue is coming before final, why can't we actually simplify this problem in the interim rather than introducing additional moving parts?

A configuration switch would really suffice here with a disclaimer warning of the impacts.

@rthewhite
Copy link

@hansl is there an ETA for the fix you are talking about?

Because from what i understand everything will still work when not removing the decorators but tree shaking is less efficient? Then this temporarily solution at least gives people that are using decorators AOT, something which seems way more valuable then tree shaking to me? The size of the bundle will probably always be smaller with AOT then without? Not to mention the huge performance increase in runtime...

@hansl
Copy link
Contributor

hansl commented Nov 11, 2016

This is something we're talking about daily here on the team. It's part of a greater effort.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom decorators are stripped when using the AoT build flag
6 participants