From 5328bb223a1482cf06518e7fafcee6f931ed39f8 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Wed, 11 Sep 2019 14:00:59 -0700 Subject: [PATCH] fix(ivy): avoid unnecessary i18n instructions generation for with structural directives (#32623) 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. PR Close #32623 --- .../compliance/r3_view_compiler_i18n_spec.ts | 58 +++++++++++++++++++ .../src/render3/r3_template_transform.ts | 10 +++- packages/core/test/acceptance/i18n_spec.ts | 11 ++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index 61a91de6b5..4fdaab682f 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -2462,6 +2462,64 @@ describe('i18n support in the template compiler', () => { verify(input, output); }); + // Note: applying structural directives to is typically user error, but it is + // technically allowed, so we need to support it. + it('should handle structural directives', () => { + const input = ` + Content A + Content B + `; + + const output = String.raw ` + const $_c0$ = [4, "ngIf"]; + var $I18N_1$; + if (ngI18nClosureMode) { + const $MSG_EXTERNAL_3308216566145348998$$APP_SPEC_TS___2$ = goog.getMsg("Content A"); + $I18N_1$ = $MSG_EXTERNAL_3308216566145348998$$APP_SPEC_TS___2$; + } else { + $I18N_1$ = $localize \`Content A\`; + } + function MyComponent_0_ng_template_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18n(0, $I18N_1$); + } + } + function MyComponent_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_ng_template_0_Template, 1, 0, "ng-template"); + } + } + var $I18N_3$; + if (ngI18nClosureMode) { + const $MSG_EXTERNAL_8349021389088127654$$APP_SPEC_TS__4$ = goog.getMsg("Content B"); + $I18N_3$ = $MSG_EXTERNAL_8349021389088127654$$APP_SPEC_TS__4$; + } else { + $I18N_3$ = $localize \`Content B\`; + } + function MyComponent_ng_container_1_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelementContainerStart(0); + $r3$.ɵɵi18n(1, $I18N_3$); + $r3$.ɵɵelementContainerEnd(); + } + } + … + consts: 2, + vars: 2, + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, undefined, $_c0$); + $r3$.ɵɵtemplate(1, MyComponent_ng_container_1_Template, 2, 0, "ng-container", $_c0$); + } + if (rf & 2) { + $r3$.ɵɵproperty("ngIf", ctx.someFlag); + $r3$.ɵɵadvance(1); + $r3$.ɵɵproperty("ngIf", ctx.someFlag); + } + } + `; + verify(input, output); + }); }); describe('whitespace preserving mode', () => { diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 67e30b0e72..e53e587d81 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -18,7 +18,7 @@ import {PreparsedElementType, preparseElement} from '../template_parser/template import {syntaxError} from '../util'; import * as t from './r3_ast'; -import {I18N_ICU_VAR_PREFIX} from './view/i18n/util'; +import {I18N_ICU_VAR_PREFIX, isI18nRootNode} from './view/i18n/util'; const BIND_NAME_REGEXP = /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/; @@ -205,12 +205,18 @@ class HtmlAstToIvyAst implements html.Visitor { outputs: parsedElement.outputs, } : {attributes: [], inputs: [], outputs: []}; + + // For s with structural directives on them, avoid passing i18n information to + // the wrapping template to prevent unnecessary i18n instructions from being generated. The + // necessary i18n meta information will be extracted from child elements. + const i18n = isTemplateElement && isI18nRootNode(element.i18n) ? undefined : element.i18n; + // TODO(pk): test for this case parsedElement = new t.Template( (parsedElement as t.Element).name, hoistedAttrs.attributes, hoistedAttrs.inputs, hoistedAttrs.outputs, templateAttrs, [parsedElement], [/* no references */], templateVariables, element.sourceSpan, element.startSourceSpan, element.endSourceSpan, - element.i18n); + i18n); } return parsedElement; } diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index ac77ee1dde..04ceee3d02 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -301,6 +301,17 @@ onlyInIvy('Ivy i18n logic') expect(element).toHaveText('Bonjour Angular'); }); + // Note: applying structural directives to is typically user error, but it + // is technically allowed, so we need to support it. + it('should handle structural directives on ng-template', () => { + loadTranslations({'Hello {$INTERPOLATION}': 'Bonjour {$INTERPOLATION}'}); + const fixture = initWithTemplate( + AppComp, `Hello {{ name }}`); + + const element = fixture.nativeElement; + expect(element).toHaveText('Bonjour Angular'); + }); + it('should be able to act as child elements inside i18n block (plain text content)', () => { loadTranslations({