refactor(compiler): align &ngsp; handling with Angular Dart implementation (#18774)

PR Close #18774
This commit is contained in:
Pawel Kozlowski 2017-08-18 15:00:12 +02:00 committed by Miško Hevery
parent 7f4c964eef
commit 7d72d0eb9b
3 changed files with 35 additions and 13 deletions

View File

@ -18,6 +18,17 @@ function hasPreserveWhitespacesAttr(attrs: html.Attribute[]): boolean {
return attrs.some((attr: html.Attribute) => attr.name === PRESERVE_WS_ATTR_NAME);
}
/**
* Angular Dart introduced &ngsp; as a placeholder for non-removable space, see:
* https://github.com/dart-lang/angular/blob/0bb611387d29d65b5af7f9d2515ab571fd3fbee4/_tests/test/compiler/preserve_whitespace_test.dart#L25-L32
* In Angular Dart &ngsp; is converted to the 0xE500 PUA (Private Use Areas) unicode character
* and later on replaced by a space. We are re-implementing the same idea here.
*/
export function replaceNgsp(value: string): string {
// lexer is replacing the &ngsp; pseudo-entity with NGSP_UNICODE
return value.replace(new RegExp(NGSP_UNICODE, 'g'), ' ');
}
/**
* This visitor can walk HTML parse tree and remove / trim text nodes using the following rules:
* - consider spaces, tabs and new lines as whitespace characters;
@ -25,15 +36,9 @@ function hasPreserveWhitespacesAttr(attrs: html.Attribute[]): boolean {
* - for all other text nodes replace consecutive whitespace characters with one space;
* - convert &ngsp; pseudo-entity to a single space;
*
* The idea of using &ngsp; as a placeholder for non-removable space was originally introduced in
* Angular Dart, see:
* https://github.com/dart-lang/angular/blob/0bb611387d29d65b5af7f9d2515ab571fd3fbee4/_tests/test/compiler/preserve_whitespace_test.dart#L25-L32
* In Angular Dart &ngsp; is converted to the 0xE500 PUA (Private Use Areas) unicode character
* and later on replaced by a space. We are re-implementing the same idea here.
*
* Removal and trimming of whitespaces have positive performance impact (less code to generate
* while compiling templates, faster view creation). At the same time it can be "destructive"
* in some cases (whitespaces can influence layout). Becouse of the potential of breaking layout
* in some cases (whitespaces can influence layout). Because of the potential of breaking layout
* this visitor is not activated by default in Angular 5 and people need to explicitly opt-in for
* whitespace removal. The default option for whitespace removal will be revisited in Angular 6
* and might be changed to "on" by default.
@ -61,9 +66,7 @@ class WhitespaceVisitor implements html.Visitor {
const isBlank = text.value.trim().length === 0;
if (!isBlank) {
// lexer is replacing the &ngsp; pseudo-entity with NGSP_UNICODE
return new html.Text(
text.value.replace(NGSP_UNICODE, ' ').replace(/\s\s+/g, ' '), text.sourceSpan);
return new html.Text(replaceNgsp(text.value).replace(/\s\s+/g, ' '), text.sourceSpan);
}
return null;

View File

@ -16,7 +16,7 @@ import {I18NHtmlParser} from '../i18n/i18n_html_parser';
import {Identifiers, createTokenForExternalReference, createTokenForReference} from '../identifiers';
import * as html from '../ml_parser/ast';
import {ParseTreeResult} from '../ml_parser/html_parser';
import {removeWhitespaces} from '../ml_parser/html_whitespaces';
import {removeWhitespaces, replaceNgsp} from '../ml_parser/html_whitespaces';
import {expandNodes} from '../ml_parser/icu_ast_expander';
import {InterpolationConfig} from '../ml_parser/interpolation_config';
import {isNgTemplate, splitNsName} from '../ml_parser/tags';
@ -248,9 +248,10 @@ class TemplateParseVisitor implements html.Visitor {
visitText(text: html.Text, parent: ElementContext): any {
const ngContentIndex = parent.findNgContentIndex(TEXT_CSS_SELECTOR) !;
const expr = this._bindingParser.parseInterpolation(text.value, text.sourceSpan !);
const valueNoNgsp = replaceNgsp(text.value);
const expr = this._bindingParser.parseInterpolation(valueNoNgsp, text.sourceSpan !);
return expr ? new BoundTextAst(expr, ngContentIndex, text.sourceSpan !) :
new TextAst(text.value, ngContentIndex, text.sourceSpan !);
new TextAst(valueNoNgsp, ngContentIndex, text.sourceSpan !);
}
visitAttribute(attribute: html.Attribute, context: any): any {

View File

@ -2071,6 +2071,12 @@ The pipe 'test' could not be found ("{{[ERROR ->]a | test}}"): TestComp@0:2`);
describe('whitespaces removal', () => {
beforeEach(() => {
TestBed.configureCompiler({providers: [TEST_COMPILER_PROVIDERS, MOCK_SCHEMA_REGISTRY]});
});
commonBeforeEach();
it('should not remove whitespaces by default', () => {
expect(humanizeTplAst(parse(' <br> <br>\t<br>\n<br> ', []))).toEqual([
[TextAst, ' '],
@ -2085,6 +2091,18 @@ The pipe 'test' could not be found ("{{[ERROR ->]a | test}}"): TestComp@0:2`);
]);
});
it('should replace each &ngsp; with a space when preserveWhitespaces is true', () => {
expect(humanizeTplAst(parse('foo&ngsp;&ngsp;&ngsp;bar', [], [], [], true))).toEqual([
[TextAst, 'foo bar'],
]);
});
it('should replace every &ngsp; with a single space when preserveWhitespaces is false', () => {
expect(humanizeTplAst(parse('foo&ngsp;&ngsp;&ngsp;bar', [], [], [], false))).toEqual([
[TextAst, 'foo bar'],
]);
});
it('should remove whitespaces when explicitly requested', () => {
expect(humanizeTplAst(parse(' <br> <br>\t<br>\n<br> ', [], [], [], false))).toEqual([
[ElementAst, 'br'],