Skip to content

Commit 2b684b7

Browse files
petebacondarwinmhevery
petebacondarwin
authored and
mhevery
committed
fix(compiler): skipping leading whitespace should not break placeholder source-spans (#39589)
Tokenized text node may have leading whitespace skipped from their source-span. But the source-span is used to compute where there are interpolated blocks, resulting in placeholder nodes whose source-spans are offset by the amount of skipped characters. This fix uses the `fullStart` location of text source-spans for computing the source-span of placeholders, so that they are accurate. Fixes #39195 PR Close #39589
1 parent ff31b43 commit 2b684b7

4 files changed

Lines changed: 30 additions & 7 deletions

File tree

‎packages/compiler/src/i18n/i18n_parser.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ class _I18nVisitor implements html.Visitor {
191191

192192
function getOffsetSourceSpan(
193193
sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan {
194-
return new ParseSourceSpan(sourceSpan.start.moveBy(start), sourceSpan.start.moveBy(end));
194+
return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end));
195195
}
196196

197197
const _CUSTOM_PH_EXP =

‎packages/compiler/src/render3/view/template.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const NG_PROJECT_AS_ATTR_NAME = 'ngProjectAs';
5252
const GLOBAL_TARGET_RESOLVERS = new Map<string, o.ExternalReference>(
5353
[['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]);
5454

55-
const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t'];
55+
export const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t'];
5656

5757
// if (rf & flags) { .. }
5858
export function renderFlagCheckIfStmt(

‎packages/compiler/test/render3/view/i18n_spec.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer';
1818
import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils';
1919
import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta';
2020
import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util';
21+
import {LEADING_TRIVIA_CHARS} from '../../../src/render3/view/template';
2122

2223
import {parseR3 as parse} from './util';
2324

@@ -355,7 +356,7 @@ describe('serializeI18nMessageForGetMsg', () => {
355356

356357
describe('serializeI18nMessageForLocalize', () => {
357358
const serialize = (input: string) => {
358-
const tree = parse(`<div i18n>${input}</div>`);
359+
const tree = parse(`<div i18n>${input}</div>`, {leadingTriviaChars: LEADING_TRIVIA_CHARS});
359360
const root = tree.nodes[0] as t.Element;
360361
return serializeI18nMessageForLocalize(root.i18n as i18n.Message);
361362
};
@@ -446,7 +447,7 @@ describe('serializeI18nMessageForLocalize', () => {
446447
expect(messageParts[3].text).toEqual('');
447448
expect(messageParts[3].sourceSpan.toString()).toEqual('');
448449
expect(messageParts[4].text).toEqual(' D');
449-
expect(messageParts[4].sourceSpan.toString()).toEqual(' D');
450+
expect(messageParts[4].sourceSpan.toString()).toEqual('D');
450451

451452
expect(placeHolders[0].text).toEqual('START_TAG_SPAN');
452453
expect(placeHolders[0].sourceSpan.toString()).toEqual('<span>');
@@ -478,6 +479,26 @@ describe('serializeI18nMessageForLocalize', () => {
478479
expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (22-26)');
479480
});
480481

482+
it('should create the correct placeholder source-spans when there is skipped leading whitespace',
483+
() => {
484+
const {messageParts, placeHolders} = serialize('<b> {{value}}</b>');
485+
expect(messageParts[0].text).toEqual('');
486+
expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)');
487+
expect(messageParts[1].text).toEqual(' ');
488+
expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('" " (13-16)');
489+
expect(messageParts[2].text).toEqual('');
490+
expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (25-25)');
491+
expect(messageParts[3].text).toEqual('');
492+
expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (29-29)');
493+
494+
expect(placeHolders[0].text).toEqual('START_BOLD_TEXT');
495+
expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"<b> " (10-16)');
496+
expect(placeHolders[1].text).toEqual('INTERPOLATION');
497+
expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (16-25)');
498+
expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT');
499+
expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (25-29)');
500+
});
501+
481502
it('should serialize simple ICU for `$localize()`', () => {
482503
expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({
483504
messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')],

‎packages/compiler/test/render3/view/util.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,13 @@ export function toStringExpression(expr: e.AST): string {
7878

7979
// Parse an html string to IVY specific info
8080
export function parseR3(
81-
input: string, options: {preserveWhitespaces?: boolean} = {}): Render3ParseResult {
81+
input: string, options: {preserveWhitespaces?: boolean, leadingTriviaChars?: string[]} = {}):
82+
Render3ParseResult {
8283
const htmlParser = new HtmlParser();
8384

84-
const parseResult =
85-
htmlParser.parse(input, 'path:://to/template', {tokenizeExpansionForms: true});
85+
const parseResult = htmlParser.parse(
86+
input, 'path:://to/template',
87+
{tokenizeExpansionForms: true, leadingTriviaChars: options.leadingTriviaChars});
8688

8789
if (parseResult.errors.length > 0) {
8890
const msg = parseResult.errors.map(e => e.toString()).join('\n');

0 commit comments

Comments
 (0)