fix(compiler): ensure that i18n message-parts have the correct source-span (#39589)

petebacondarwin · mhevery · commit e67a331b12a5 · 2020-11-06T09:09:48.000-08:00
In an i18n message, two placeholders next to each other must have an "empty" message-part to separate them. Previously, the source-span for this message-part was pointing to the wrong original location. This caused problems in the generated source-maps and lead to extracted i18n messages from being rendered incorrectly. PR Close #39589
diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts
@@ -120,7 +120,7 @@ function processMessagePieces(pieces: o.MessagePiece[]):
       placeHolders.push(part);
       if (pieces[i - 1] instanceof o.PlaceholderPiece) {
         // There were two placeholders in a row, so we need to add an empty message part.
-        messageParts.push(createEmptyMessagePart(part.sourceSpan.end));
+        messageParts.push(createEmptyMessagePart(pieces[i - 1].sourceSpan.end));
       }
     }
   }
diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts
@@ -458,14 +458,33 @@ describe('serializeI18nMessageForLocalize', () => {
        expect(placeHolders[3].sourceSpan.toString()).toEqual('</span>');
      });
 
+  it('should create the correct source-spans when there are two placeholders next to each other',
+     () => {
+       const {messageParts, placeHolders} = serialize('<b>{{value}}</b>');
+       expect(messageParts[0].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)');
+       expect(messageParts[1].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('"" (13-13)');
+       expect(messageParts[2].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (22-22)');
+       expect(messageParts[3].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (26-26)');
+
+       expect(placeHolders[0].text).toEqual('START_BOLD_TEXT');
+       expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"<b>" (10-13)');
+       expect(placeHolders[1].text).toEqual('INTERPOLATION');
+       expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (13-22)');
+       expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT');
+       expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (22-26)');
+     });
+
   it('should serialize simple ICU for `$localize()`', () => {
     expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({
       messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')],
       placeHolders: []
     });
   });
 
-
   it('should serialize nested ICUs for `$localize()`', () => {
     expect(serialize(
                '{age, plural, 10 {ten {size, select, 1 {one} 2 {two} other {2+}}} other {other}}'))
@@ -478,7 +497,6 @@ describe('serializeI18nMessageForLocalize', () => {
         });
   });
 
-
   it('should serialize ICU with embedded HTML for `$localize()`', () => {
     expect(serialize('{age, plural, 10 {<b>ten</b>} other {<div class="A">other</div>}}')).toEqual({
       messageParts: [
@@ -564,3 +582,7 @@ function literal(text: string, span: any = jasmine.any(ParseSourceSpan)): o.Lite
 function placeholder(name: string, span: any = jasmine.any(ParseSourceSpan)): o.PlaceholderPiece {
   return new o.PlaceholderPiece(name, span);
 }
+
+function humanizeSourceSpan(span: ParseSourceSpan): string {
+  return `"${span.toString()}" (${span.start.offset}-${span.end.offset})`;
+}

Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ function processMessagePieces(pieces: o.MessagePiece[]):`
`120`	`120`	`placeHolders.push(part);`
`121`	`121`	`if (pieces[i - 1] instanceof o.PlaceholderPiece) {`
`122`	`122`	`// There were two placeholders in a row, so we need to add an empty message part.`
`123`		`- messageParts.push(createEmptyMessagePart(part.sourceSpan.end));`
	`123`	`+ messageParts.push(createEmptyMessagePart(pieces[i - 1].sourceSpan.end));`
`124`	`124`	`}`
`125`	`125`	`}`
`126`	`126`	`}`