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(ivy): avoid unnecessary i18n instructions generation for <ng-template> with structural directives #32623

Conversation

@AndrewKushnir
Copy link
Contributor

commented Sep 11, 2019

If an contains a structural directive (for example *ngIf), Ngtsc generates extra template function with 1 template instruction call. When tag also contains i18n attribute on it, we generate i18nStart and i18nEnd instructions around it, which is unnecessary and breaking runtime. This commit adds a logic to make sure we do not generate i18n instructions in case only template is present.

This PR resolves FW-1553.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No
@ngbot ngbot bot modified the milestone: needsTriage Sep 11, 2019
@googlebot googlebot added the cla: yes label Sep 11, 2019
@AndrewKushnir AndrewKushnir changed the title fix(ivy): avoid unnecessary i18ninstructions generation for <ng-template> with structural directives fix(ivy): avoid unnecessary i18n instructions generation for <ng-template> with structural directives Sep 11, 2019
@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:FW-1553_i18n_block_instructions branch 2 times, most recently from b5b0723 to feb5617 Sep 11, 2019
@AndrewKushnir AndrewKushnir marked this pull request as ready for review Sep 11, 2019
@AndrewKushnir AndrewKushnir requested review from angular/fw-compiler as code owners Sep 11, 2019
@alxhub
alxhub approved these changes Sep 11, 2019
@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:FW-1553_i18n_block_instructions branch from 1cef80a to 013401c Sep 11, 2019
@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

@kara
kara approved these changes Sep 13, 2019
Copy link
Contributor

left a comment

LGTM, one nit

…late> with structural directives

If an <ng-template> contains a structural directive (for example *ngIf), Ngtsc generates extra template function with 1 template instruction call. When <ng-template> tag also contains i18n attribute on it, we generate i18nStart and i18nEnd instructions around it, which is unnecessary and breaking runtime. This commit adds a logic to make sure we do not generate i18n instructions in case only `template` is present.
…ng-template> with structural directives
…ng-template> with structural directives
@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:FW-1553_i18n_block_instructions branch from cd9a775 to 38e79e7 Sep 13, 2019
…ng-template> with structural directives
@kara
kara approved these changes Sep 13, 2019
@kara kara closed this in 5328bb2 Sep 13, 2019
arnehoek added a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
…late> with structural directives (angular#32623)

If an <ng-template> contains a structural directive (for example *ngIf), Ngtsc generates extra template function with 1 template instruction call. When <ng-template> tag also contains i18n attribute on it, we generate i18nStart and i18nEnd instructions around it, which is unnecessary and breaking runtime. This commit adds a logic to make sure we do not generate i18n instructions in case only `template` is present.

PR Close angular#32623
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

commented Oct 14, 2019

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 Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.