Skip to content

Commit

Permalink
fix(compiler): produce placeholder for blocks in i18n bundles (#52958)
Browse files Browse the repository at this point in the history
When blocks were initially implemented, they were represented as containers in the i18n AST. This is problematic, because block affect the structure of the message.

These changes introduce a new `BlockPlaceholder` AST node and integrate it into the i18n pipeline. With the new node blocks are represented with the `START_BLOCK_<name>` and `CLOSE_BLOCK_<name>` placeholders.

PR Close #52958
  • Loading branch information
crisbeto authored and AndrewKushnir committed Nov 20, 2023
1 parent ac9cd61 commit f01b718
Show file tree
Hide file tree
Showing 38 changed files with 333 additions and 78 deletions.
3 changes: 1 addition & 2 deletions packages/compiler/src/compiler.ts
Expand Up @@ -38,7 +38,7 @@ export * from './version';
export {CompilerConfig, preserveWhitespacesDefault} from './config';
export * from './resource_loader';
export {ConstantPool} from './constant_pool';
export {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/interpolation_config';
export {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/defaults';
export * from './schema/element_schema_registry';
export * from './i18n/index';
export * from './expression_parser/ast';
Expand All @@ -47,7 +47,6 @@ export * from './expression_parser/parser';
export * from './ml_parser/ast';
export * from './ml_parser/html_parser';
export * from './ml_parser/html_tags';
export * from './ml_parser/interpolation_config';
export * from './ml_parser/tags';
export {ParseTreeResult, TreeError} from './ml_parser/parser';
export {LexerRange} from './ml_parser/lexer';
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/expression_parser/parser.ts
Expand Up @@ -7,7 +7,7 @@
*/

import * as chars from '../chars';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/defaults';
import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType as MlParserTokenType} from '../ml_parser/tokens';

import {AbsoluteSourceSpan, AST, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, EmptyExpr, ExpressionBinding, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast';
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler/src/i18n/digest.ts
Expand Up @@ -81,6 +81,11 @@ class _SerializerVisitor implements i18n.Visitor {
visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any {
return `<ph icu name="${ph.name}">${ph.value.visit(this)}</ph>`;
}

visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context: any): any {
return `<ph block name="${ph.startName}">${
ph.children.map(child => child.visit(this)).join(', ')}</ph name="${ph.closeName}">`;
}
}

const serializerVisitor = new _SerializerVisitor();
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler/src/i18n/extractor_merger.ts
Expand Up @@ -7,7 +7,7 @@
*/

import * as html from '../ml_parser/ast';
import {InterpolationConfig} from '../ml_parser/interpolation_config';
import {DEFAULT_CONTAINER_BLOCKS, InterpolationConfig} from '../ml_parser/defaults';
import {ParseTreeResult} from '../ml_parser/parser';

import * as i18n from './i18n_ast';
Expand Down Expand Up @@ -308,7 +308,8 @@ class _Visitor implements html.Visitor {
this._errors = [];
this._messages = [];
this._inImplicitNode = false;
this._createI18nMessage = createI18nMessageFactory(interpolationConfig);
this._createI18nMessage =
createI18nMessageFactory(interpolationConfig, DEFAULT_CONTAINER_BLOCKS);
}

// looks for translatable attributes
Expand Down
28 changes: 28 additions & 0 deletions packages/compiler/src/i18n/i18n_ast.ts
Expand Up @@ -129,6 +129,17 @@ export class IcuPlaceholder implements Node {
}
}

export class BlockPlaceholder implements Node {
constructor(
public name: string, public parameters: string[], public startName: string,
public closeName: string, public children: Node[], public sourceSpan: ParseSourceSpan,
public startSourceSpan: ParseSourceSpan|null, public endSourceSpan: ParseSourceSpan|null) {}

visit(visitor: Visitor, context?: any): any {
return visitor.visitBlockPlaceholder(this, context);
}
}

/**
* Each HTML node that is affect by an i18n tag will also have an `i18n` property that is of type
* `I18nMeta`.
Expand All @@ -144,6 +155,7 @@ export interface Visitor {
visitTagPlaceholder(ph: TagPlaceholder, context?: any): any;
visitPlaceholder(ph: Placeholder, context?: any): any;
visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): any;
visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): any;
}

// Clone the AST
Expand Down Expand Up @@ -178,6 +190,13 @@ export class CloneVisitor implements Visitor {
visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): IcuPlaceholder {
return new IcuPlaceholder(ph.value, ph.name, ph.sourceSpan);
}

visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): BlockPlaceholder {
const children = ph.children.map(n => n.visit(this, context));
return new BlockPlaceholder(
ph.name, ph.parameters, ph.startName, ph.closeName, children, ph.sourceSpan,
ph.startSourceSpan, ph.endSourceSpan);
}
}

// Visit all the nodes recursively
Expand All @@ -201,6 +220,10 @@ export class RecurseVisitor implements Visitor {
visitPlaceholder(ph: Placeholder, context?: any): any {}

visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): any {}

visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): any {
ph.children.forEach(child => child.visit(this));
}
}


Expand Down Expand Up @@ -240,4 +263,9 @@ class LocalizeMessageStringVisitor implements Visitor {
visitIcuPlaceholder(ph: IcuPlaceholder): any {
return `{$${ph.name}}`;
}

visitBlockPlaceholder(ph: BlockPlaceholder): any {
const children = ph.children.map(child => child.visit(this)).join('');
return `{$${ph.startName}}${children}{$${ph.closeName}}`;
}
}
2 changes: 1 addition & 1 deletion packages/compiler/src/i18n/i18n_html_parser.ts
Expand Up @@ -7,8 +7,8 @@
*/

import {MissingTranslationStrategy} from '../core';
import {DEFAULT_INTERPOLATION_CONFIG} from '../ml_parser/defaults';
import {HtmlParser} from '../ml_parser/html_parser';
import {DEFAULT_INTERPOLATION_CONFIG} from '../ml_parser/interpolation_config';
import {TokenizeOptions} from '../ml_parser/lexer';
import {ParseTreeResult} from '../ml_parser/parser';
import {Console} from '../util';
Expand Down
40 changes: 33 additions & 7 deletions packages/compiler/src/i18n/i18n_parser.ts
Expand Up @@ -9,8 +9,8 @@
import {Lexer as ExpressionLexer} from '../expression_parser/lexer';
import {Parser as ExpressionParser} from '../expression_parser/parser';
import * as html from '../ml_parser/ast';
import {InterpolationConfig} from '../ml_parser/defaults';
import {getHtmlTagDefinition} from '../ml_parser/html_tags';
import {InterpolationConfig} from '../ml_parser/interpolation_config';
import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType} from '../ml_parser/tokens';
import {ParseSourceSpan} from '../parse_util';

Expand All @@ -29,9 +29,9 @@ export interface I18nMessageFactory {
/**
* Returns a function converting html nodes to an i18n Message given an interpolationConfig
*/
export function createI18nMessageFactory(interpolationConfig: InterpolationConfig):
I18nMessageFactory {
const visitor = new _I18nVisitor(_expParser, interpolationConfig);
export function createI18nMessageFactory(
interpolationConfig: InterpolationConfig, containerBlocks: Set<string>): I18nMessageFactory {
const visitor = new _I18nVisitor(_expParser, interpolationConfig, containerBlocks);
return (nodes, meaning, description, customId, visitNodeFn) =>
visitor.toI18nMessage(nodes, meaning, description, customId, visitNodeFn);
}
Expand All @@ -52,7 +52,9 @@ function noopVisitNodeFn(_html: html.Node, i18n: i18n.Node): i18n.Node {
class _I18nVisitor implements html.Visitor {
constructor(
private _expressionParser: ExpressionParser,
private _interpolationConfig: InterpolationConfig) {}
private _interpolationConfig: InterpolationConfig,
private _containerBlocks: Set<string>,
) {}

public toI18nMessage(
nodes: html.Node[], meaning = '', description = '', customId = '',
Expand Down Expand Up @@ -164,11 +166,35 @@ class _I18nVisitor implements html.Visitor {

visitBlock(block: html.Block, context: I18nMessageVisitorContext) {
const children = html.visitAll(this, block.children, context);
const node = new i18n.Container(children, block.sourceSpan);

if (this._containerBlocks.has(block.name)) {
return new i18n.Container(children, block.sourceSpan);
}

const parameters = block.parameters.map(param => param.expression);
const startPhName =
context.placeholderRegistry.getStartBlockPlaceholderName(block.name, parameters);
const closePhName = context.placeholderRegistry.getCloseBlockPlaceholderName(block.name);

context.placeholderToContent[startPhName] = {
text: block.startSourceSpan.toString(),
sourceSpan: block.startSourceSpan,
};

context.placeholderToContent[closePhName] = {
text: block.endSourceSpan ? block.endSourceSpan.toString() : '}',
sourceSpan: block.endSourceSpan ?? block.sourceSpan,
};

const node = new i18n.BlockPlaceholder(
block.name, parameters, startPhName, closePhName, children, block.sourceSpan,
block.startSourceSpan, block.endSourceSpan);
return context.visitNodeFn(block, node);
}

visitBlockParameter(_parameter: html.BlockParameter, _context: I18nMessageVisitorContext) {}
visitBlockParameter(_parameter: html.BlockParameter, _context: I18nMessageVisitorContext) {
throw new Error('Unreachable code');
}

/**
* Convert, text and interpolated tokens up into text and placeholder pieces.
Expand Down
12 changes: 11 additions & 1 deletion packages/compiler/src/i18n/message_bundle.ts
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {InterpolationConfig} from '../ml_parser/defaults';
import {HtmlParser} from '../ml_parser/html_parser';
import {InterpolationConfig} from '../ml_parser/interpolation_config';
import {ParseError} from '../parse_util';

import {extractMessages} from './extractor_merger';
Expand Down Expand Up @@ -99,6 +99,16 @@ class MapPlaceholderNames extends i18n.CloneVisitor {
ph.startSourceSpan, ph.endSourceSpan);
}

override visitBlockPlaceholder(ph: i18n.BlockPlaceholder, mapper: PlaceholderMapper):
i18n.BlockPlaceholder {
const startName = mapper.toPublicName(ph.startName)!;
const closeName = ph.closeName ? mapper.toPublicName(ph.closeName)! : ph.closeName;
const children = ph.children.map(n => n.visit(this, mapper));
return new i18n.BlockPlaceholder(
ph.name, ph.parameters, startName, closeName, children, ph.sourceSpan, ph.startSourceSpan,
ph.endSourceSpan);
}

override visitPlaceholder(ph: i18n.Placeholder, mapper: PlaceholderMapper): i18n.Placeholder {
return new i18n.Placeholder(ph.value, mapper.toPublicName(ph.name)!, ph.sourceSpan);
}
Expand Down
36 changes: 36 additions & 0 deletions packages/compiler/src/i18n/serializers/placeholder.ts
Expand Up @@ -97,6 +97,29 @@ export class PlaceholderRegistry {
return this._generateUniqueName(name.toUpperCase());
}

getStartBlockPlaceholderName(name: string, parameters: string[]): string {
const signature = this._hashBlock(name, parameters);
if (this._signatureToName[signature]) {
return this._signatureToName[signature];
}

const placeholder = this._generateUniqueName(`START_BLOCK_${this._toSnakeCase(name)}`);
this._signatureToName[signature] = placeholder;
return placeholder;
}

getCloseBlockPlaceholderName(name: string): string {
const signature = this._hashClosingBlock(name);
if (this._signatureToName[signature]) {
return this._signatureToName[signature];
}

const placeholder = this._generateUniqueName(`CLOSE_BLOCK_${this._toSnakeCase(name)}`);
this._signatureToName[signature] = placeholder;
return placeholder;
}


// Generate a hash for a tag - does not take attribute order into account
private _hashTag(tag: string, attrs: {[k: string]: string}, isVoid: boolean): string {
const start = `<${tag}`;
Expand All @@ -110,6 +133,19 @@ export class PlaceholderRegistry {
return this._hashTag(`/${tag}`, {}, false);
}

private _hashBlock(name: string, parameters: string[]): string {
const params = parameters.length === 0 ? '' : ` (${parameters.sort().join('; ')})`;
return `@${name}${params} {}`;
}

private _hashClosingBlock(name: string): string {
return this._hashBlock(`close_${name}`, []);
}

private _toSnakeCase(name: string) {
return name.toUpperCase().replace(/[^A-Z0-9]/g, '_');
}

private _generateUniqueName(base: string): string {
const seen = this._placeHolderNameCounts.hasOwnProperty(base);
if (!seen) {
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/src/i18n/serializers/serializer.ts
Expand Up @@ -77,6 +77,12 @@ export class SimplePlaceholderMapper extends i18n.RecurseVisitor implements Plac
this.visitPlaceholderName(ph.name);
}

override visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): any {
this.visitPlaceholderName(ph.startName);
super.visitBlockPlaceholder(ph, context);
this.visitPlaceholderName(ph.closeName);
}

override visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any {
this.visitPlaceholderName(ph.name);
}
Expand Down
9 changes: 9 additions & 0 deletions packages/compiler/src/i18n/serializers/xliff.ts
Expand Up @@ -164,6 +164,15 @@ class _WriteVisitor implements i18n.Visitor {
return [new xml.Tag(_PLACEHOLDER_TAG, {id: ph.name, 'equiv-text': `{{${ph.value}}}`})];
}

visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] {
const ctype = `x-${ph.name.toLowerCase().replace(/[^a-z0-9]/g, '-')}`;
const startTagPh =
new xml.Tag(_PLACEHOLDER_TAG, {id: ph.startName, ctype, 'equiv-text': `@${ph.name}`});
const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {id: ph.closeName, ctype, 'equiv-text': `}`});

return [startTagPh, ...this.serialize(ph.children), closeTagPh];
}

visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] {
const equivText = `{${ph.value.expression}, ${ph.value.type}, ${
Object.keys(ph.value.cases).map((value: string) => value + ' {...}').join(' ')}}`;
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler/src/i18n/serializers/xliff2.ts
Expand Up @@ -178,6 +178,25 @@ class _WriteVisitor implements i18n.Visitor {
})];
}

visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] {
const tagPc = new xml.Tag(_PLACEHOLDER_SPANNING_TAG, {
id: (this._nextPlaceholderId++).toString(),
equivStart: ph.startName,
equivEnd: ph.closeName,
type: 'other',
dispStart: `@${ph.name}`,
dispEnd: `}`,
});
const nodes: xml.Node[] = [].concat(...ph.children.map(node => node.visit(this)));
if (nodes.length) {
nodes.forEach((node: xml.Node) => tagPc.children.push(node));
} else {
tagPc.children.push(new xml.Text(''));
}

return [tagPc];
}

visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] {
const cases = Object.keys(ph.value.cases).map((value: string) => value + ' {...}').join(' ');
const idStr = (this._nextPlaceholderId++).toString();
Expand Down
14 changes: 14 additions & 0 deletions packages/compiler/src/i18n/serializers/xmb.ts
Expand Up @@ -148,6 +148,20 @@ class _Visitor implements i18n.Visitor {
];
}

visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] {
const startAsText = new xml.Text(`@${ph.name}`);
const startEx = new xml.Tag(_EXAMPLE_TAG, {}, [startAsText]);
// TC requires PH to have a non empty EX, and uses the text node to show the "original" value.
const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.startName}, [startEx, startAsText]);

const closeAsText = new xml.Text(`}`);
const closeEx = new xml.Tag(_EXAMPLE_TAG, {}, [closeAsText]);
// TC requires PH to have a non empty EX, and uses the text node to show the "original" value.
const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.closeName}, [closeEx, closeAsText]);

return [startTagPh, ...this.serialize(ph.children), closeTagPh];
}

visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] {
const icuExpression = ph.value.expression;
const icuType = ph.value.type;
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/src/i18n/translation_bundle.ts
Expand Up @@ -149,6 +149,12 @@ class I18nToHtmlVisitor implements i18n.Visitor {
return this._convertToText(this._srcMsg.placeholderToMessage[ph.name]);
}

visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): string {
const params = ph.parameters.length === 0 ? '' : ` (${ph.parameters.join('; ')})`;
const children = ph.children.map((c: i18n.Node) => c.visit(this)).join('');
return `@${ph.name}${params} {${children}}`;
}

/**
* Convert a source message to a translated text string:
* - text nodes are replaced with their translation,
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/jit_compiler_facade.ts
Expand Up @@ -10,7 +10,7 @@ import {CompilerFacade, CoreEnvironment, ExportedCompilerFacade, InputMap, Input
import {ConstantPool} from './constant_pool';
import {ChangeDetectionStrategy, HostBinding, HostListener, Input, Output, ViewEncapsulation} from './core';
import {compileInjectable} from './injectable_compiler_2';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/interpolation_config';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/defaults';
import {DeclareVarStmt, Expression, literal, LiteralExpr, Statement, StmtModifier, WrappedNodeExpr} from './output/output_ast';
import {JitEvaluator} from './output/output_jit';
import {ParseError, ParseSourceSpan, r3JitTypeSourceSpan} from './parse_util';
Expand Down
11 changes: 7 additions & 4 deletions packages/compiler/src/ml_parser/ast.ts
Expand Up @@ -86,13 +86,16 @@ export class Comment implements BaseNode {
}
}

export class Block implements BaseNode {
export class Block extends NodeWithI18n {
constructor(
public name: string, public parameters: BlockParameter[], public children: Node[],
public sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan,
public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null) {}
sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan,
public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null,
i18n?: I18nMeta) {
super(sourceSpan, i18n);
}

visit(visitor: Visitor, context: any) {
override visit(visitor: Visitor, context: any) {
return visitor.visitBlock(this, context);
}
}
Expand Down

0 comments on commit f01b718

Please sign in to comment.