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 1de26927d0..67bc8ae16c 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 @@ -153,7 +153,14 @@ ${output} `); }; -const verify = (input: string, output: string, extra: any = {}): void => { +const verify = (input: string, output: string, extra: { + inputArgs?: any, + compilerOptions?: any, + skipPathBasedCheck?: boolean, + skipIdBasedCheck?: boolean, + verbose?: boolean, + exceptions?: {} +} = {}): void => { const files = getAppFilesWithTemplate(input, extra.inputArgs); const opts = (i18nUseExternalIds: boolean) => ({i18nUseExternalIds, ...(extra.compilerOptions || {})}); @@ -161,7 +168,7 @@ const verify = (input: string, output: string, extra: any = {}): void => { // invoke with file-based prefix translation names if (!extra.skipPathBasedCheck) { const result = compile(files, angularFiles, opts(false)); - maybePrint(result.source, extra.verbose); + maybePrint(result.source, !!extra.verbose); expect(verifyPlaceholdersIntegrity(result.source)).toBe(true); expect(verifyUniqueConsts(result.source)).toBe(true); expectEmit(result.source, output, 'Incorrect template'); @@ -170,7 +177,7 @@ const verify = (input: string, output: string, extra: any = {}): void => { // invoke with translation names based on external ids if (!extra.skipIdBasedCheck) { const result = compile(files, angularFiles, opts(true)); - maybePrint(result.source, extra.verbose); + maybePrint(result.source, !!extra.verbose); const interpolationConfig = extra.inputArgs && extra.inputArgs.interpolation ? InterpolationConfig.fromArray(extra.inputArgs.interpolation) : undefined; @@ -346,7 +353,6 @@ describe('i18n support in the template compiler', () => { ${i18n_7} return [ $i18n_0$, - [${AttributeMarker.I18n}, "title"], ["title", $i18n_1$], ["title", $i18n_2$], ["title", $i18n_3$], @@ -362,31 +368,25 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵi18n(1, 0); $r3$.ɵɵelementEnd(); $r3$.ɵɵelementStart(2, "div", 1); - $r3$.ɵɵi18nAttributes(3, 2); - $r3$.ɵɵtext(4, "Content B"); + $r3$.ɵɵtext(3, "Content B"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(5, "div", 1); - $r3$.ɵɵi18nAttributes(6, 3); - $r3$.ɵɵtext(7, "Content C"); + $r3$.ɵɵelementStart(4, "div", 2); + $r3$.ɵɵtext(5, "Content C"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(8, "div", 1); - $r3$.ɵɵi18nAttributes(9, 4); - $r3$.ɵɵtext(10, "Content D"); + $r3$.ɵɵelementStart(6, "div", 3); + $r3$.ɵɵtext(7, "Content D"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(11, "div", 1); - $r3$.ɵɵi18nAttributes(12, 5); - $r3$.ɵɵtext(13, "Content E"); + $r3$.ɵɵelementStart(8, "div", 4); + $r3$.ɵɵtext(9, "Content E"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(14, "div", 1); - $r3$.ɵɵi18nAttributes(15, 6); - $r3$.ɵɵtext(16, "Content F"); + $r3$.ɵɵelementStart(10, "div", 5); + $r3$.ɵɵtext(11, "Content F"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(17, "div", 1); - $r3$.ɵɵi18nAttributes(18, 7); - $r3$.ɵɵtext(19, "Content G"); + $r3$.ɵɵelementStart(12, "div", 6); + $r3$.ɵɵtext(13, "Content G"); $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(20, "div"); - $r3$.ɵɵi18n(21, 8); + $r3$.ɵɵelementStart(14, "div"); + $r3$.ɵɵi18n(15, 7); $r3$.ɵɵelementEnd(); } } @@ -405,14 +405,12 @@ describe('i18n support in the template compiler', () => { consts: function () { ${i18n_0} return [ - [${AttributeMarker.I18n}, "title"], ["title", $i18n_0$] ]; }, template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0); - $r3$.ɵɵi18nAttributes(1, 1); } } `; @@ -436,7 +434,6 @@ describe('i18n support in the template compiler', () => { function MyComponent_0_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵtemplate(0, MyComponent_0_ng_template_0_Template, 1, 0, "ng-template", 1); - $r3$.ɵɵi18nAttributes(1, 2); } } … @@ -444,13 +441,12 @@ describe('i18n support in the template compiler', () => { ${i18n_0} return [ [${AttributeMarker.Template}, "ngIf"], - [${AttributeMarker.I18n}, "title"], ["title", $i18n_0$] ]; }, template: function MyComponent_Template(rf, ctx) { if (rf & 1) { - $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 0, undefined, 0); + $r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, undefined, 0); } if (rf & 2) { $r3$.ɵɵproperty("ngIf", ctx.visible); @@ -584,15 +580,12 @@ describe('i18n support in the template compiler', () => { consts: function() { ${i18n_0} return [ - ["id", "static", ${AttributeMarker.I18n}, "title"], - ["title", $i18n_0$] + ["id", "static", "title", $i18n_0$] ]; }, template: function MyComponent_Template(rf, ctx) { if (rf & 1) { - $r3$.ɵɵelementStart(0, "div", 0); - $r3$.ɵɵi18nAttributes(1, 1); - $r3$.ɵɵelementEnd(); + $r3$.ɵɵelement(0, "div", 0); } } `; @@ -640,9 +633,9 @@ describe('i18n support in the template compiler', () => { ${i18n_3} ${i18n_4} return [ - ["id", "dynamic-1", ${AttributeMarker.I18n}, "aria-roledescription", - "title", "aria-label"], - ["aria-roledescription", $i18n_0$, "title", $i18n_1$, "aria-label", $i18n_2$], + ["id", "dynamic-1", "aria-roledescription", $i18n_0$, ${AttributeMarker.I18n}, + "title", "aria-label"], + ["title", $i18n_1$, "aria-label", $i18n_2$], ["id", "dynamic-2", ${AttributeMarker.I18n}, "title", "aria-roledescription"], ["title", $i18n_3$, "aria-roledescription", $i18n_4$] ]; @@ -790,76 +783,6 @@ describe('i18n support in the template compiler', () => { verify(input, output); }); - it('should support interpolation', () => { - const input = ` -
-
- `; - - const i18n_0 = i18nMsg('static text'); - const i18n_1 = i18nMsg( - 'intro {$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]], - {meaning: 'm', desc: 'd'}); - const i18n_2 = i18nMsg( - '{$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]], - {meaning: 'm1', desc: 'd1'}); - const i18n_3 = i18nMsg( - '{$interpolation} and {$interpolation_1} and again {$interpolation_2}', - [ - ['interpolation', String.raw`\uFFFD0\uFFFD`], - ['interpolation_1', String.raw`\uFFFD1\uFFFD`], - ['interpolation_2', String.raw`\uFFFD2\uFFFD`] - ], - {meaning: 'm2', desc: 'd2'}); - const i18n_4 = i18nMsg('{$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]]); - - const output = String.raw` - decls: 5, - vars: 8, - consts: function() { - ${i18n_0} - ${i18n_1} - ${i18n_2} - ${i18n_3} - ${i18n_4} - return [ - ["id", "dynamic-1", ${AttributeMarker.I18n}, "aria-roledescription", - "title", "aria-label"], - ["aria-roledescription", $i18n_0$, "title", $i18n_1$, "aria-label", $i18n_2$], - ["id", "dynamic-2", ${AttributeMarker.I18n}, "title", "aria-roledescription"], - ["title", $i18n_3$, "aria-roledescription", $i18n_4$] - ]; - }, - template: function MyComponent_Template(rf, ctx) { - if (rf & 1) { - $r3$.ɵɵelementStart(0, "div", 0); - $r3$.ɵɵpipe(1, "uppercase"); - $r3$.ɵɵi18nAttributes(2, 1); - $r3$.ɵɵelementEnd(); - $r3$.ɵɵelementStart(3, "div", 2); - $r3$.ɵɵi18nAttributes(4, 3); - $r3$.ɵɵelementEnd(); - } - if (rf & 2) { - $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 6, ctx.valueA))(ctx.valueB); - $r3$.ɵɵi18nApply(2); - $r3$.ɵɵadvance(3); - $r3$.ɵɵi18nExp(ctx.valueA)(ctx.valueB)(ctx.valueA + ctx.valueB)(ctx.valueC); - $r3$.ɵɵi18nApply(4); - } - } - `; - - verify(input, output); - }); - it('should correctly bind to context in nested template', () => { const input = `
@@ -925,7 +848,6 @@ describe('i18n support in the template compiler', () => { ${i18n_0} ${i18n_1} return [ - [${AttributeMarker.I18n}, "title"], ["title", $i18n_0$], $i18n_1$ ]; @@ -933,8 +855,7 @@ describe('i18n support in the template compiler', () => { template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelementStart(0, "div", 0); - $r3$.ɵɵi18nAttributes(1, 1); - $r3$.ɵɵi18n(2, 2); + $r3$.ɵɵi18n(1, 1); $r3$.ɵɵelementEnd(); } } diff --git a/packages/compiler/src/render3/view/i18n/util.ts b/packages/compiler/src/render3/view/i18n/util.ts index 3c0360cffe..b4fe3e9586 100644 --- a/packages/compiler/src/render3/view/i18n/util.ts +++ b/packages/compiler/src/render3/view/i18n/util.ts @@ -50,6 +50,11 @@ export function hasI18nMeta(node: t.Node&{i18n?: i18n.I18nMeta}): boolean { return !!node.i18n; } +export function isBoundI18nAttribute(node: t.TextAttribute| + t.BoundAttribute): node is t.BoundAttribute { + return node.i18n !== undefined && node instanceof t.BoundAttribute; +} + 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 e71b84c05d..f7b0ebebaf 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -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, hasI18nMeta, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_VAR_PREFIX, wrapI18nPlaceholder} from './i18n/util'; +import {assembleBoundTextPlaceholders, assembleI18nBoundString, declareI18nVariable, getTranslationConstPrefix, hasI18nMeta, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isBoundI18nAttribute, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_VAR_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'; @@ -492,30 +492,25 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } private i18nAttributesInstruction( - nodeIndex: number, attrs: (t.TextAttribute|t.BoundAttribute)[], - sourceSpan: ParseSourceSpan): void { + nodeIndex: number, attrs: t.BoundAttribute[], sourceSpan: ParseSourceSpan): void { let hasBindings: boolean = false; const i18nAttrArgs: o.Expression[] = []; const bindings: ChainableBindingInstruction[] = []; attrs.forEach(attr => { const message = attr.i18n! as i18n.Message; - if (attr instanceof t.TextAttribute) { - i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message)); - } else { - const converted = attr.value.visit(this._valueConverter); - this.allocateBindingSlots(converted); - if (converted instanceof Interpolation) { - const placeholders = assembleBoundTextPlaceholders(message); - const params = placeholdersToParams(placeholders); - i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message, params)); - converted.expressions.forEach(expression => { - hasBindings = true; - bindings.push({ - sourceSpan, - value: () => this.convertPropertyBinding(expression), - }); + const converted = attr.value.visit(this._valueConverter); + this.allocateBindingSlots(converted); + if (converted instanceof Interpolation) { + const placeholders = assembleBoundTextPlaceholders(message); + const params = placeholdersToParams(placeholders); + i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message, params)); + converted.expressions.forEach(expression => { + hasBindings = true; + bindings.push({ + sourceSpan, + value: () => this.convertPropertyBinding(expression), }); - } + }); } }); if (bindings.length > 0) { @@ -591,7 +586,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const isI18nRootElement: boolean = isI18nRootNode(element.i18n) && !isSingleI18nIcu(element.i18n); - const i18nAttrs: (t.TextAttribute|t.BoundAttribute)[] = []; + const boundI18nAttrs: t.BoundAttribute[] = []; const outputAttrs: t.TextAttribute[] = []; const [namespaceKey, elementName] = splitNsName(element.name); @@ -606,8 +601,12 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver stylingBuilder.registerStyleAttr(value); } else if (name === 'class') { stylingBuilder.registerClassAttr(value); + } else if (isBoundI18nAttribute(attr)) { + // Note that we don't collect static i18n attributes here, because + // they can be treated in the same way as regular attributes. + boundI18nAttrs.push(attr); } else { - (attr.i18n ? i18nAttrs : outputAttrs).push(attr); + outputAttrs.push(attr); } } @@ -627,7 +626,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const stylingInputWasSet = stylingBuilder.registerBoundInput(input); if (!stylingInputWasSet) { if (input.type === BindingType.Property && input.i18n) { - i18nAttrs.push(input); + boundI18nAttrs.push(input); } else { allOtherInputs.push(input); } @@ -636,7 +635,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // add attributes for directive and projection matching purposes const attributes: o.Expression[] = this.getAttributeExpressions( - element.name, outputAttrs, allOtherInputs, element.outputs, stylingBuilder, [], i18nAttrs); + element.name, outputAttrs, allOtherInputs, element.outputs, stylingBuilder, [], + boundI18nAttrs); parameters.push(this.addAttrsToConsts(attributes)); // local refs (ex.:
) @@ -662,7 +662,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver element.children.length > 0; const createSelfClosingInstruction = !stylingBuilder.hasBindingsWithPipes && - element.outputs.length === 0 && i18nAttrs.length === 0 && !hasChildren; + element.outputs.length === 0 && boundI18nAttrs.length === 0 && !hasChildren; const createSelfClosingI18nInstruction = !createSelfClosingInstruction && hasTextChildrenOnly(element.children); @@ -679,9 +679,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.creationInstruction(element.startSourceSpan, R3.disableBindings); } - if (i18nAttrs.length > 0) { + if (boundI18nAttrs.length > 0) { this.i18nAttributesInstruction( - elementIndex, i18nAttrs, element.startSourceSpan ?? element.sourceSpan); + elementIndex, boundI18nAttrs, element.startSourceSpan ?? element.sourceSpan); } // Generate Listeners (outputs) @@ -866,10 +866,11 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.matchDirectives(NG_TEMPLATE_TAG_NAME, template); // prepare attributes parameter (including attributes used for directive matching) - const [i18nStaticAttrs, staticAttrs] = partitionArray(template.attributes, hasI18nMeta); + const [boundI18nAttrs, attrs] = partitionArray( + template.attributes, isBoundI18nAttribute); const attrsExprs: o.Expression[] = this.getAttributeExpressions( - NG_TEMPLATE_TAG_NAME, staticAttrs, template.inputs, template.outputs, - undefined /* styles */, template.templateAttrs, i18nStaticAttrs); + NG_TEMPLATE_TAG_NAME, attrs, template.inputs, template.outputs, undefined /* styles */, + template.templateAttrs, boundI18nAttrs); parameters.push(this.addAttrsToConsts(attrsExprs)); // local refs (ex.: ) @@ -913,8 +914,9 @@ 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 [i18nInputs, inputs] = partitionArray(template.inputs, hasI18nMeta); - const i18nAttrs = [...i18nStaticAttrs, ...i18nInputs]; + const [i18nInputs, inputs] = + partitionArray(template.inputs, hasI18nMeta); + const i18nAttrs = [...boundI18nAttrs, ...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 @@ -1289,18 +1291,25 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver elementName: string, renderAttributes: t.TextAttribute[], inputs: t.BoundAttribute[], outputs: t.BoundEvent[], styles?: StylingBuilder, templateAttrs: (t.BoundAttribute|t.TextAttribute)[] = [], - i18nAttrs: (t.BoundAttribute|t.TextAttribute)[] = []): o.Expression[] { + boundI18nAttrs: t.BoundAttribute[] = []): o.Expression[] { const alreadySeen = new Set(); const attrExprs: o.Expression[] = []; let ngProjectAsAttr: t.TextAttribute|undefined; - renderAttributes.forEach((attr: t.TextAttribute) => { + for (const attr of renderAttributes) { if (attr.name === NG_PROJECT_AS_ATTR_NAME) { ngProjectAsAttr = attr; } - attrExprs.push( - ...getAttributeNameLiterals(attr.name), trustedConstAttribute(elementName, attr)); - }); + + // Note that static i18n attributes aren't in the i18n array, + // because they're treated in the same way as regular attributes. + if (attr.i18n) { + attrExprs.push(o.literal(attr.name), this.i18nTranslate(attr.i18n as i18n.Message)); + } else { + attrExprs.push( + ...getAttributeNameLiterals(attr.name), trustedConstAttribute(elementName, attr)); + } + } // Keep ngProjectAs next to the other name, value pairs so we can verify that we match // ngProjectAs marker in the attribute name slot. @@ -1360,9 +1369,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver templateAttrs.forEach(attr => addAttrExpr(attr.name)); } - if (i18nAttrs.length) { + if (boundI18nAttrs.length) { attrExprs.push(o.literal(core.AttributeMarker.I18n)); - i18nAttrs.forEach(attr => addAttrExpr(attr.name)); + boundI18nAttrs.forEach(attr => addAttrExpr(attr.name)); } return attrExprs; diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 0bedc5751e..0d58b083e1 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -280,12 +280,12 @@ export function newArray(size: number, value?: T): T[] { * @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[]] { +export function partitionArray( + arr: (T|F)[], conditionFn: (value: T|F) => boolean): [T[], F[]] { const truthy: T[] = []; - const falsy: T[] = []; - arr.forEach(item => { - (conditionFn(item) ? truthy : falsy).push(item); - }); + const falsy: F[] = []; + for (const item of arr) { + (conditionFn(item) ? truthy : falsy).push(item as any); + } return [truthy, falsy]; } diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 60584ad733..6ed2836bf7 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -14,16 +14,14 @@ import {_sanitizeUrl, sanitizeSrcset} from '../../sanitization/url_sanitizer'; import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertOneOf, assertString} from '../../util/assert'; import {CharCode} from '../../util/char_code'; import {loadIcuContainerVisitor} from '../instructions/i18n_icu_container_visitor'; -import {allocExpando, createTNodeAtIndex, elementAttributeInternal, setInputsForProperty, setNgReflectProperties} from '../instructions/shared'; +import {allocExpando, createTNodeAtIndex} from '../instructions/shared'; import {getDocument} from '../interfaces/document'; import {ELEMENT_MARKER, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes, IcuExpression, IcuType, TI18n, TIcu} from '../interfaces/i18n'; import {TNode, TNodeType} from '../interfaces/node'; -import {RComment, RElement} from '../interfaces/renderer'; import {SanitizerFn} from '../interfaces/sanitization'; import {HEADER_OFFSET, LView, TView} from '../interfaces/view'; import {getCurrentParentTNode, getCurrentTNode, setCurrentTNode} from '../state'; import {attachDebugGetter} from '../util/debug_utils'; -import {getNativeByIndex, getTNode} from '../util/view_utils'; import {i18nCreateOpCodesToString, i18nRemoveOpCodesToString, i18nUpdateOpCodesToString, icuCreateOpCodesToString} from './i18n_debug'; import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index'; @@ -102,7 +100,10 @@ export function i18nStartFirstCreatePass( // Verify that ICU expression has the right shape. Translations might contain invalid // constructions (while original messages were correct), so ICU parsing at runtime may // not succeed (thus `icuExpression` remains a string). - if (ngDevMode && typeof icuExpression !== 'object') { + // Note: we intentionally retain the error here by not using `ngDevMode`, because + // the value can change based on the locale and users aren't guaranteed to hit + // an invalid string while they're developing. + if (typeof icuExpression !== 'object') { throw new Error(`Unable to parse ICU expression in "${message}" message.`); } const icuContainerTNode = createTNodeAndAddOpCode( @@ -225,54 +226,34 @@ function i18nStartFirstCreatePassProcessTextNode( /** * See `i18nAttributes` above. */ -export function i18nAttributesFirstPass( - lView: LView, tView: TView, index: number, values: string[]) { +export function i18nAttributesFirstPass(tView: TView, index: number, values: string[]) { const previousElement = getCurrentTNode()!; const previousElementIndex = previousElement.index; const updateOpCodes: I18nUpdateOpCodes = [] as any; if (ngDevMode) { attachDebugGetter(updateOpCodes, i18nUpdateOpCodesToString); } - for (let i = 0; i < values.length; i += 2) { - const attrName = values[i]; - const message = values[i + 1]; - const parts = message.split(ICU_REGEXP); - for (let j = 0; j < parts.length; j++) { - const value = parts[j]; + if (tView.firstCreatePass && tView.data[index] === null) { + for (let i = 0; i < values.length; i += 2) { + const attrName = values[i]; + const message = values[i + 1]; - if (j & 1) { - // Odd indexes are ICU expressions - // TODO(ocombe): support ICU expressions in attributes - throw new Error('ICU expressions are not yet supported in attributes'); - } else if (value !== '') { - // Even indexes are text (including bindings) - const hasBinding = !!value.match(BINDING_REGEXP); - if (hasBinding) { - if (tView.firstCreatePass && tView.data[index] === null) { - generateBindingUpdateOpCodes(updateOpCodes, value, previousElementIndex, attrName); - } - } else { - const tNode = getTNode(tView, previousElementIndex); - // Set attributes for Elements only, for other types (like ElementContainer), - // only set inputs below - if (tNode.type & TNodeType.AnyRNode) { - elementAttributeInternal(tNode, lView, attrName, value, null, null); - } - // Check if that attribute is a directive input - const dataValue = tNode.inputs !== null && tNode.inputs[attrName]; - if (dataValue) { - setInputsForProperty(tView, lView, dataValue, attrName, value); - if (ngDevMode) { - const element = getNativeByIndex(previousElementIndex, lView) as RElement | RComment; - setNgReflectProperties(lView, element, tNode.type, dataValue, value); - } - } + if (message !== '') { + // Check if attribute value contains an ICU and throw an error if that's the case. + // ICUs in element attributes are not supported. + // Note: we intentionally retain the error here by not using `ngDevMode`, because + // the `value` can change based on the locale and users aren't guaranteed to hit + // an invalid string while they're developing. + if (ICU_REGEXP.test(message)) { + throw new Error( + `ICU expressions are not supported in attributes. Message: "${message}".`); } + + // i18n attributes that hit this code path are guaranteed to have bindings, because + // the compiler treats static i18n attributes as regular attribute bindings. + generateBindingUpdateOpCodes(updateOpCodes, message, previousElementIndex, attrName); } } - } - - if (tView.firstCreatePass && tView.data[index] === null) { tView.data[index] = updateOpCodes; } } @@ -622,10 +603,10 @@ function walkIcuTree( generateBindingUpdateOpCodes(update, attr.value, newIndex, attr.name); } } else { - ngDevMode && console.warn(` WARNING: - ignoring unsafe attribute value ${lowerAttrName} on element $ { - tagName - } (see http://g.co/ng/security#xss)`); + ngDevMode && + console.warn( + `WARNING: ignoring unsafe attribute value ` + + `${lowerAttrName} on element ${tagName} (see http://g.co/ng/security#xss)`); } } else { addCreateAttribute(create, newIndex, attr); @@ -705,4 +686,4 @@ function addCreateNodeAndAppend( function addCreateAttribute(create: IcuCreateOpCodes, newIndex: number, attr: Attr) { create.push(newIndex << IcuCreateOpCode.SHIFT_REF | IcuCreateOpCode.Attr, attr.name, attr.value); -} \ No newline at end of file +} diff --git a/packages/core/src/render3/instructions/i18n.ts b/packages/core/src/render3/instructions/i18n.ts index 8557470809..cfeda67690 100644 --- a/packages/core/src/render3/instructions/i18n.ts +++ b/packages/core/src/render3/instructions/i18n.ts @@ -122,11 +122,10 @@ export function ɵɵi18n(index: number, messageIndex: number, subTemplateIndex?: * @codeGenApi */ export function ɵɵi18nAttributes(index: number, attrsIndex: number): void { - const lView = getLView(); const tView = getTView(); ngDevMode && assertDefined(tView, `tView should be defined`); const attrs = getConstant(tView.consts, attrsIndex)!; - i18nAttributesFirstPass(lView, tView, index + HEADER_OFFSET, attrs); + i18nAttributesFirstPass(tView, index + HEADER_OFFSET, attrs); } @@ -181,4 +180,4 @@ export function ɵɵi18nApply(index: number) { export function ɵɵi18nPostprocess( message: string, replacements: {[key: string]: (string|string[])} = {}): string { return i18nPostprocess(message, replacements); -} \ No newline at end of file +} diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts index fc652dfcbd..8bc0206a56 100644 --- a/packages/core/test/acceptance/di_spec.ts +++ b/packages/core/test/acceptance/di_spec.ts @@ -2054,6 +2054,26 @@ describe('di', () => { expect(directive.outputAttr).toBeNull(); expect(directive.other).toBe('otherValue'); }); + + it('should inject `null` for attributes with data bindings', () => { + @Directive({selector: '[dir]'}) + class MyDir { + constructor(@Attribute('title') public attrValue: string) {} + } + + @Component({template: '
'}) + class MyComp { + @ViewChild(MyDir) directiveInstance!: MyDir; + value = 'value'; + } + + TestBed.configureTestingModule({declarations: [MyDir, MyComp]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + + expect(fixture.componentInstance.directiveInstance.attrValue).toBeNull(); + expect(fixture.nativeElement.querySelector('div').getAttribute('title')).toBe('title value'); + }); }); it('should support dependencies in Pipes used inside ICUs', () => { diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index bcbce5a482..315e92d448 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -13,7 +13,7 @@ import {CommonModule, DOCUMENT, registerLocaleData} from '@angular/common'; import localeEs from '@angular/common/locales/es'; import localeRo from '@angular/common/locales/ro'; import {computeMsgId} from '@angular/compiler'; -import {Component, ContentChild, ContentChildren, Directive, ElementRef, HostBinding, Input, LOCALE_ID, NO_ERRORS_SCHEMA, Pipe, PipeTransform, QueryList, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef, ɵsetDocument} from '@angular/core'; +import {Attribute, Component, ContentChild, ContentChildren, Directive, ElementRef, HostBinding, Input, LOCALE_ID, NO_ERRORS_SCHEMA, Pipe, PipeTransform, QueryList, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef, ɵsetDocument} from '@angular/core'; import {DebugNode, HEADER_OFFSET, TVIEW} from '@angular/core/src/render3/interfaces/view'; import {getComponentLView} from '@angular/core/src/render3/util/discovery_utils'; import {TestBed} from '@angular/core/testing'; @@ -642,9 +642,9 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { // such a case a single `TIcuContainerNode` should be generated only. it('should create a single dynamic TNode for ICU', () => { const fixture = initWithTemplate(AppComp, ` - {count, plural, - =0 {just now} - =1 {one minute ago} + {count, plural, + =0 {just now} + =1 {one minute ago} other {{{count}} minutes ago} } `.trim()); @@ -662,13 +662,13 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { it('should support multiple ICUs', () => { const fixture = initWithTemplate(AppComp, ` - {count, plural, - =0 {just now} - =1 {one minute ago} + {count, plural, + =0 {just now} + =1 {one minute ago} other {{{count}} minutes ago} } - {name, select, - Angular {Mr. Angular} + {name, select, + Angular {Mr. Angular} other {Sir} } `); @@ -1843,40 +1843,6 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { expect(fixture.nativeElement.firstChild.title).toEqual(`ANGULAR - value 1 - value 2 (fr)`); }); - it('should create corresponding ng-reflect properties', () => { - @Component({ - selector: 'welcome', - template: '{{ messageText }}', - }) - class WelcomeComp { - @Input() messageText!: string; - } - - @Component({ - template: ` - - - ` - }) - class App { - } - - TestBed.configureTestingModule({ - declarations: [App, WelcomeComp], - }); - loadTranslations({ - [computeMsgId('Hello')]: 'Bonjour', - }); - const fixture = TestBed.createComponent(App); - fixture.detectChanges(); - - const comp = fixture.debugElement.query(By.css('welcome')); - expect(comp.attributes['messagetext']).toBe('Bonjour'); - expect(comp.attributes['ng-reflect-message-text']).toBe('Bonjour'); - }); - it('should support i18n attributes on elements', () => { loadTranslations({[computeMsgId('Hello', 'meaning')]: 'Bonjour'}); @@ -3024,6 +2990,50 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent).toEqual(`two,one,`); }); + + it('should be able to inject a static i18n attribute', () => { + loadTranslations({[computeMsgId('text')]: 'translatedText'}); + + @Directive({selector: '[injectTitle]'}) + class InjectTitleDir { + constructor(@Attribute('title') public title: string) {} + } + + @Component({template: `
`}) + class App { + @ViewChild(InjectTitleDir) dir!: InjectTitleDir; + } + + TestBed.configureTestingModule({declarations: [App, InjectTitleDir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.dir.title).toBe('translatedText'); + expect(fixture.nativeElement.querySelector('div').getAttribute('title')).toBe('translatedText'); + }); + + it('should inject `null` for an i18n attribute with an interpolation', () => { + loadTranslations({[computeMsgId('text {$INTERPOLATION}')]: 'translatedText {$INTERPOLATION}'}); + + @Directive({selector: '[injectTitle]'}) + class InjectTitleDir { + constructor(@Attribute('title') public title: string) {} + } + + @Component({template: `
`}) + class App { + @ViewChild(InjectTitleDir) dir!: InjectTitleDir; + value = 'value'; + } + + TestBed.configureTestingModule({declarations: [App, InjectTitleDir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.dir.title).toBeNull(); + expect(fixture.nativeElement.querySelector('div').getAttribute('title')) + .toBe('translatedText value'); + }); }); function initWithTemplate(compType: Type, template: string) { diff --git a/packages/core/test/render3/i18n/i18n_spec.ts b/packages/core/test/render3/i18n/i18n_spec.ts index cd72a4dec2..26c2be6c2a 100644 --- a/packages/core/test/render3/i18n/i18n_spec.ts +++ b/packages/core/test/render3/i18n/i18n_spec.ts @@ -416,30 +416,6 @@ describe('Runtime i18n', () => { }); describe(`i18nAttribute`, () => { - it('for text', () => { - const message = `Hello world!`; - const attrs = ['title', message]; - const nbDecls = 2; - const index = 1; - const fixture = new TemplateFixture({ - create: () => { - ɵɵelementStart(0, 'div'); - ɵɵi18nAttributes(index, 0); - ɵɵelementEnd(); - }, - decls: nbDecls, - vars: index, - consts: [attrs], - }); - const tView = fixture.hostView[TVIEW]; - const opCodes = tView.data[HEADER_OFFSET + index] as I18nUpdateOpCodes; - - expect(opCodes).toEqual([]); - expect((getNativeByIndex(HEADER_OFFSET, fixture.hostView as LView) as any as Element) - .getAttribute('title')) - .toEqual(message); - }); - it('for simple bindings', () => { const message = `Hello �0�!`; const attrs = ['title', message];