fix(compiler): do not remove whitespace wrapping i18n expansions (#31962)
Similar to interpolation, we do not want to completely remove whitespace nodes that are siblings of an expansion. For example, the following template ```html <div> <strong>items left<strong> {count, plural, =1 {item} other {items}} </div> ``` was being collapsed to ```html <div><strong>items left<strong>{count, plural, =1 {item} other {items}}</div> ``` which results in the text looking like ``` items left4 ``` instead it should be collapsed to ```html <div><strong>items left<strong> {count, plural, =1 {item} other {items}}</div> ``` which results in the text looking like ``` items left 4 ``` --- **Analysis of the code and manual testing has shown that this does not cause the generated ids to change, so there is no breaking change here.** PR Close #31962
This commit is contained in:

committed by
Kara Erickson

parent
fd6ed1713d
commit
0ddf0c4895
@ -60,18 +60,20 @@ export class WhitespaceVisitor implements html.Visitor {
|
||||
}
|
||||
|
||||
return new html.Element(
|
||||
element.name, element.attrs, html.visitAll(this, element.children), element.sourceSpan,
|
||||
element.startSourceSpan, element.endSourceSpan, element.i18n);
|
||||
element.name, element.attrs, visitAllWithSiblings(this, element.children),
|
||||
element.sourceSpan, element.startSourceSpan, element.endSourceSpan, element.i18n);
|
||||
}
|
||||
|
||||
visitAttribute(attribute: html.Attribute, context: any): any {
|
||||
return attribute.name !== PRESERVE_WS_ATTR_NAME ? attribute : null;
|
||||
}
|
||||
|
||||
visitText(text: html.Text, context: any): any {
|
||||
visitText(text: html.Text, context: SiblingVisitorContext|null): any {
|
||||
const isNotBlank = text.value.match(NO_WS_REGEXP);
|
||||
const hasExpansionSibling = context &&
|
||||
(context.prev instanceof html.Expansion || context.next instanceof html.Expansion);
|
||||
|
||||
if (isNotBlank) {
|
||||
if (isNotBlank || hasExpansionSibling) {
|
||||
return new html.Text(
|
||||
replaceNgsp(text.value).replace(WS_REPLACE_REGEXP, ' '), text.sourceSpan, text.i18n);
|
||||
}
|
||||
@ -91,3 +93,21 @@ export function removeWhitespaces(htmlAstWithErrors: ParseTreeResult): ParseTree
|
||||
html.visitAll(new WhitespaceVisitor(), htmlAstWithErrors.rootNodes),
|
||||
htmlAstWithErrors.errors);
|
||||
}
|
||||
|
||||
interface SiblingVisitorContext {
|
||||
prev: html.Node|undefined;
|
||||
next: html.Node|undefined;
|
||||
}
|
||||
|
||||
function visitAllWithSiblings(visitor: WhitespaceVisitor, nodes: html.Node[]): any[] {
|
||||
const result: any[] = [];
|
||||
|
||||
nodes.forEach((ast, i) => {
|
||||
const context: SiblingVisitorContext = {prev: nodes[i - 1], next: nodes[i + 1]};
|
||||
const astResult = ast.visit(visitor, context);
|
||||
if (astResult) {
|
||||
result.push(astResult);
|
||||
}
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
@ -40,7 +40,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
|
||||
it('should not create a message for plain elements',
|
||||
() => { expect(_humanizeMessages('<div></div>')).toEqual([]); });
|
||||
|
||||
it('should suppoprt void elements', () => {
|
||||
it('should support void elements', () => {
|
||||
expect(_humanizeMessages('<div i18n="m|d"><p><br></p></div>')).toEqual([
|
||||
[
|
||||
[
|
||||
@ -173,6 +173,13 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
|
||||
]);
|
||||
});
|
||||
|
||||
it('should extract as ICU + ph when wrapped in whitespace in an element', () => {
|
||||
expect(_humanizeMessages('<div i18n="m|d"> {count, plural, =0 {zero}} </div>')).toEqual([
|
||||
[[' ', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', ' '], 'm', 'd'],
|
||||
[['{count, plural, =0 {[zero]}}'], '', ''],
|
||||
]);
|
||||
});
|
||||
|
||||
it('should extract as ICU when single child of a block', () => {
|
||||
expect(_humanizeMessages('<!-- i18n:m|d -->{count, plural, =0 {zero}}<!-- /i18n -->'))
|
||||
.toEqual([
|
||||
|
@ -9,14 +9,15 @@
|
||||
import * as html from '../../src/ml_parser/ast';
|
||||
import {HtmlParser} from '../../src/ml_parser/html_parser';
|
||||
import {PRESERVE_WS_ATTR_NAME, removeWhitespaces} from '../../src/ml_parser/html_whitespaces';
|
||||
import {TokenizeOptions} from '../../src/ml_parser/lexer';
|
||||
|
||||
import {humanizeDom} from './ast_spec_utils';
|
||||
|
||||
{
|
||||
describe('removeWhitespaces', () => {
|
||||
|
||||
function parseAndRemoveWS(template: string): any[] {
|
||||
return humanizeDom(removeWhitespaces(new HtmlParser().parse(template, 'TestComp')));
|
||||
function parseAndRemoveWS(template: string, options?: TokenizeOptions): any[] {
|
||||
return humanizeDom(removeWhitespaces(new HtmlParser().parse(template, 'TestComp', options)));
|
||||
}
|
||||
|
||||
it('should remove blank text nodes', () => {
|
||||
@ -97,6 +98,17 @@ import {humanizeDom} from './ast_spec_utils';
|
||||
]);
|
||||
});
|
||||
|
||||
it('should preserve whitespaces around ICU expansions', () => {
|
||||
expect(parseAndRemoveWS(`<span> {a, b, =4 {c}} </span>`, {tokenizeExpansionForms: true}))
|
||||
.toEqual([
|
||||
[html.Element, 'span', 0],
|
||||
[html.Text, ' ', 1],
|
||||
[html.Expansion, 'a', 'b', 1],
|
||||
[html.ExpansionCase, '=4', 2],
|
||||
[html.Text, ' ', 1],
|
||||
]);
|
||||
});
|
||||
|
||||
it('should preserve whitespaces inside <pre> elements', () => {
|
||||
expect(parseAndRemoveWS(`<pre><strong>foo</strong>\n<strong>bar</strong></pre>`)).toEqual([
|
||||
[html.Element, 'pre', 0],
|
||||
|
@ -798,6 +798,30 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
|
||||
]);
|
||||
});
|
||||
|
||||
it('should parse an expansion form with whitespace surrounding it', () => {
|
||||
expect(tokenizeAndHumanizeParts(
|
||||
'<div><span> {a, b, =4 {c}} </span></div>', {tokenizeExpansionForms: true}))
|
||||
.toEqual([
|
||||
[lex.TokenType.TAG_OPEN_START, '', 'div'],
|
||||
[lex.TokenType.TAG_OPEN_END],
|
||||
[lex.TokenType.TAG_OPEN_START, '', 'span'],
|
||||
[lex.TokenType.TAG_OPEN_END],
|
||||
[lex.TokenType.TEXT, ' '],
|
||||
[lex.TokenType.EXPANSION_FORM_START],
|
||||
[lex.TokenType.RAW_TEXT, 'a'],
|
||||
[lex.TokenType.RAW_TEXT, 'b'],
|
||||
[lex.TokenType.EXPANSION_CASE_VALUE, '=4'],
|
||||
[lex.TokenType.EXPANSION_CASE_EXP_START],
|
||||
[lex.TokenType.TEXT, 'c'],
|
||||
[lex.TokenType.EXPANSION_CASE_EXP_END],
|
||||
[lex.TokenType.EXPANSION_FORM_END],
|
||||
[lex.TokenType.TEXT, ' '],
|
||||
[lex.TokenType.TAG_CLOSE, '', 'span'],
|
||||
[lex.TokenType.TAG_CLOSE, '', 'div'],
|
||||
[lex.TokenType.EOF],
|
||||
]);
|
||||
});
|
||||
|
||||
it('should parse an expansion forms with elements in it', () => {
|
||||
expect(tokenizeAndHumanizeParts(
|
||||
'{one.two, three, =4 {four <b>a</b>}}', {tokenizeExpansionForms: true}))
|
||||
|
Reference in New Issue
Block a user