From 88b0985bad498f2fdd6bc8846279f63ed3acb205 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Fri, 3 Apr 2020 12:58:20 -0700 Subject: [PATCH] fix(compiler): avoid generating i18n attributes in plain form (#36422) Prior to this change, there was a problem while matching template attributes, which mistakenly took i18n attributes (that might be present in attrs array after template ones) into account. This commit updates the logic to avoid template attribute matching logic from entering the i18n section and as a result this also allows generating proper i18n attributes sections instead of keeping these attribute in plain form (with their values) in attribute arrays. PR Close #36422 --- .../compliance/r3_view_compiler_i18n_spec.ts | 8 ++------ .../compiler/src/render3/view/i18n/util.ts | 5 +++++ .../compiler/src/render3/view/template.ts | 19 +++++++------------ packages/compiler/src/util.ts | 18 ++++++++++++++++++ packages/compiler/test/util_spec.ts | 18 +++++++++++++++++- .../core/src/render3/node_selector_matcher.ts | 6 +++++- 6 files changed, 54 insertions(+), 20 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 eee7e684ed..d193d48970 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 @@ -337,8 +337,6 @@ describe('i18n support in the template compiler', () => { `; - // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form - // into the `consts` array on Component def. const output = String.raw` var $I18N_0$; if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { @@ -350,7 +348,7 @@ describe('i18n support in the template compiler', () => { } const $_c2$ = ["title", $I18N_0$]; … - consts: [["title", "Hello"]], + consts: [[${AttributeMarker.I18n}, "title"]], template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0); @@ -367,8 +365,6 @@ describe('i18n support in the template compiler', () => { Test `; - // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form - // into the `consts` array on Component def. const output = String.raw` var $I18N_0$; if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { @@ -391,7 +387,7 @@ describe('i18n support in the template compiler', () => { } } … - consts: [[${AttributeMarker.Template}, "ngIf"], ["title", "Hello"]], + consts: [[${AttributeMarker.Template}, "ngIf"], [${AttributeMarker.I18n}, "title"]], template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 0, undefined, 0); diff --git a/packages/compiler/src/render3/view/i18n/util.ts b/packages/compiler/src/render3/view/i18n/util.ts index c6b48ee840..06cfd840d7 100644 --- a/packages/compiler/src/render3/view/i18n/util.ts +++ b/packages/compiler/src/render3/view/i18n/util.ts @@ -9,6 +9,7 @@ import * as i18n from '../../../i18n/i18n_ast'; import {toPublicName} from '../../../i18n/serializers/xmb'; import * as html from '../../../ml_parser/ast'; import * as o from '../../../output/output_ast'; +import * as t from '../../r3_ast'; /* Closure variables holding messages must be named `MSG_[A-Z0-9]+` */ const CLOSURE_TRANSLATION_PREFIX = 'MSG_'; @@ -41,6 +42,10 @@ export function isSingleI18nIcu(meta?: i18n.I18nMeta): boolean { return isI18nRootNode(meta) && meta.nodes.length === 1 && meta.nodes[0] instanceof i18n.Icu; } +export function hasI18nMeta(node: t.Node&{i18n?: i18n.I18nMeta}): boolean { + return !!node.i18n; +} + export function hasI18nAttrs(element: html.Element): boolean { return element.attrs.some((attr: html.Attribute) => isI18nAttribute(attr.name)); } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index b75e162fe7..0b477ef57d 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -26,7 +26,7 @@ import {ParseError, ParseSourceSpan} from '../../parse_util'; import {DomElementSchemaRegistry} from '../../schema/dom_element_schema_registry'; import {CssSelector, SelectorMatcher} from '../../selector'; import {BindingParser} from '../../template_parser/binding_parser'; -import {error} from '../../util'; +import {error, partitionArray} from '../../util'; import * as t from '../r3_ast'; import {Identifiers as R3} from '../r3_identifiers'; import {htmlAstToRender3Ast} from '../r3_template_transform'; @@ -36,7 +36,7 @@ import {I18nContext} from './i18n/context'; import {createGoogleGetMsgStatements} from './i18n/get_msg_utils'; import {createLocalizeStatements} from './i18n/localize_utils'; import {I18nMetaVisitor} from './i18n/meta'; -import {assembleBoundTextPlaceholders, assembleI18nBoundString, declareI18nVariable, getTranslationConstPrefix, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_PREFIX, wrapI18nPlaceholder} from './i18n/util'; +import {assembleBoundTextPlaceholders, assembleI18nBoundString, declareI18nVariable, getTranslationConstPrefix, hasI18nMeta, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_PREFIX, wrapI18nPlaceholder} from './i18n/util'; import {StylingBuilder, StylingInstruction} from './styling_builder'; import {asLiteral, chainedInstruction, CONTEXT_NAME, getAttrsForDirectiveMatching, getInterpolationArgsLength, IMPLICIT_REFERENCE, invalid, NON_BINDABLE_ATTR, REFERENCE_PREFIX, RENDER_FLAGS, trimTrailingNulls, unsupported} from './util'; @@ -848,11 +848,10 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.matchDirectives(NG_TEMPLATE_TAG_NAME, template); // prepare attributes parameter (including attributes used for directive matching) - // TODO (FW-1942): exclude i18n attributes from the main attribute list and pass them - // as an `i18nAttrs` argument of the `getAttributeExpressions` function below. + const [i18nStaticAttrs, staticAttrs] = partitionArray(template.attributes, hasI18nMeta); const attrsExprs: o.Expression[] = this.getAttributeExpressions( - template.attributes, template.inputs, template.outputs, undefined, template.templateAttrs, - undefined); + staticAttrs, template.inputs, template.outputs, undefined /* styles */, + template.templateAttrs, i18nStaticAttrs); parameters.push(this.addAttrsToConsts(attrsExprs)); // local refs (ex.: ) @@ -896,12 +895,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // Only add normal input/output binding instructions on explicit elements. if (template.tagName === NG_TEMPLATE_TAG_NAME) { - const inputs: t.BoundAttribute[] = []; - const i18nAttrs: (t.TextAttribute|t.BoundAttribute)[] = - template.attributes.filter(attr => !!attr.i18n); - - template.inputs.forEach( - (input: t.BoundAttribute) => (input.i18n ? i18nAttrs : inputs).push(input)); + const [i18nInputs, inputs] = partitionArray(template.inputs, hasI18nMeta); + const i18nAttrs = [...i18nStaticAttrs, ...i18nInputs]; // Add i18n attributes that may act as inputs to directives. If such attributes are present, // generate `i18nAttributes` instruction. Note: we generate it only for explicit diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 1cd8db7061..82b87ef97d 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -270,4 +270,22 @@ export function newArray(size: number, value?: T): T[] { list.push(value!); } return list; +} + +/** + * Partitions a given array into 2 arrays, based on a boolean value returned by the condition + * function. + * + * @param arr Input array that should be partitioned + * @param conditionFn Condition function that is called for each item in a given array and returns a + * boolean value. + */ +export function partitionArray( + arr: T[], conditionFn: (value: K) => boolean): [T[], T[]] { + const truthy: T[] = []; + const falsy: T[] = []; + arr.forEach(item => { + (conditionFn(item) ? truthy : falsy).push(item); + }); + return [truthy, falsy]; } \ No newline at end of file diff --git a/packages/compiler/test/util_spec.ts b/packages/compiler/test/util_spec.ts index aa8423df2a..dfa0455443 100644 --- a/packages/compiler/test/util_spec.ts +++ b/packages/compiler/test/util_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {escapeRegExp, splitAtColon, stringify, utf8Encode} from '../src/util'; +import {escapeRegExp, partitionArray, splitAtColon, stringify, utf8Encode} from '../src/util'; { describe('util', () => { @@ -83,5 +83,21 @@ import {escapeRegExp, splitAtColon, stringify, utf8Encode} from '../src/util'; expect(stringify(Object.create(null))).toEqual('object'); }); }); + + describe('partitionArray()', () => { + it('should handle empty arrays', () => { + expect(partitionArray([], () => true)).toEqual([[], []]); + }); + + it('should handle arrays with primitive type values', () => { + expect(partitionArray([1, 2, 3], (el: number) => el < 2)).toEqual([[1], [2, 3]]); + }); + + it('should handle arrays of objects', () => { + expect(partitionArray([{id: 1}, {id: 2}, {id: 3}], (el: any) => el.id < 2)).toEqual([ + [{id: 1}], [{id: 2}, {id: 3}] + ]); + }); + }); }); } diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts index 5d724774b3..78e7aa78bb 100644 --- a/packages/core/src/render3/node_selector_matcher.ts +++ b/packages/core/src/render3/node_selector_matcher.ts @@ -290,7 +290,11 @@ function matchTemplateAttribute(attrs: TAttributes, name: string): number { if (i > -1) { i++; while (i < attrs.length) { - if (attrs[i] === name) return i; + const attr = attrs[i]; + // Return in case we checked all template attrs and are switching to the next section in the + // attrs array (that starts with a number that represents an attribute marker). + if (typeof attr === 'number') return -1; + if (attr === name) return i; i++; } }