honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler): ensure that placeholders have the correct sourceSpan (#39717) 

When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.

Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.

This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.

Fixes #39671

PR Close #39717
This commit is contained in:
Pete Bacon Darwin 2020-11-16 15:02:38 +00:00 committed by Andrew Kushnir
parent 6b38c44a22
commit 0462a616c3
2 changed files with 132 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
packages/compiler-cli/test/ngtsc/template_mapping_spec.ts
View File

@ -427,6 +427,95 @@ runInEachFileSystem((os) => {
});
});
it('should correctly handle collapsed whitespace in interpolation placeholder source-mappings',
() => {
const mappings = compileAndMap(
`<div i18n title=" pre-title {{title_value}} post-title" i18n-title> pre-body {{body_value}} post-body</div>`);
expectMapping(mappings, {
source: '<div i18n title=" pre-title {{title_value}} post-title" i18n-title> ',
generated: 'i0.ɵɵelementStart(0, "div", 0)',
sourceUrl: '../test.ts',
});
expectMapping(mappings, {
source: '</div>',
generated: 'i0.ɵɵelementEnd()',
sourceUrl: '../test.ts',
});
expectMapping(mappings, {
source: ' pre-body ',
generated: '` pre-body ${',
sourceUrl: '../test.ts',
});
expectMapping(mappings, {
source: '{{body_value}}',
generated: '"\\uFFFD0\\uFFFD"',
sourceUrl: '../test.ts',
});
expectMapping(mappings, {
source: ' post-body',
generated: '}:INTERPOLATION: post-body`',
sourceUrl: '../test.ts',
});
});
it('should correctly handle collapsed whitespace in element placeholder source-mappings',
() => {
const mappings =
compileAndMap(`<div i18n>\n pre-p\n <p>\n in-p\n </p>\n post-p\n</div>`);
// $localize expressions
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: 'pre-p\n ',
generated: '` pre-p ${',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '<p>\n ',
generated: '"\\uFFFD#2\\uFFFD"',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: 'in-p\n ',
generated: '}:START_PARAGRAPH: in-p ${',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '</p>\n ',
generated: '"\\uFFFD/#2\\uFFFD"',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: 'post-p\n',
generated: '}:CLOSE_PARAGRAPH: post-p\n`',
});
// ivy instructions
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '<div i18n>\n ',
generated: 'i0.ɵɵelementStart(0, "div")',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '<div i18n>\n ',
generated: 'i0.ɵɵi18nStart(1, 0)',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '<p>\n in-p\n </p>',
generated: 'i0.ɵɵelement(2, "p")',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '</div>',
generated: 'i0.ɵɵi18nEnd()',
});
expectMapping(mappings, {
sourceUrl: '../test.ts',
source: '</div>',
generated: 'i0.ɵɵelementEnd()',
});
});
it('should create tag (container) placeholder source-mappings', () => {
const mappings = compileAndMap(`<div i18n>Hello, <b>World</b>!</div>`);
expectMapping(mappings, {

46
packages/compiler/src/i18n/i18n_parser.ts
View File

@ -105,12 +105,13 @@ class _I18nVisitor implements html.Visitor {
}
visitAttribute(attribute: html.Attribute, context: I18nMessageVisitorContext): i18n.Node {
const node = this._visitTextWithInterpolation(attribute.value, attribute.sourceSpan, context);
const node = this._visitTextWithInterpolation(
attribute.value, attribute.valueSpan || attribute.sourceSpan, context, attribute.i18n);
return context.visitNodeFn(attribute, node);
}
visitText(text: html.Text, context: I18nMessageVisitorContext): i18n.Node {
const node = this._visitTextWithInterpolation(text.value, text.sourceSpan, context);
const node = this._visitTextWithInterpolation(text.value, text.sourceSpan, context, text.i18n);
return context.visitNodeFn(text, node);
}
@ -156,7 +157,8 @@ class _I18nVisitor implements html.Visitor {
}
private _visitTextWithInterpolation(
text: string, sourceSpan: ParseSourceSpan, context: I18nMessageVisitorContext): i18n.Node {
text: string, sourceSpan: ParseSourceSpan, context: I18nMessageVisitorContext,
previousI18n: i18n.I18nMeta|undefined): i18n.Node {
const splitInterpolation = this._expressionParser.splitInterpolation(
text, sourceSpan.start.toString(), this._interpolationConfig);
@ -197,10 +199,48 @@ class _I18nVisitor implements html.Visitor {
getOffsetSourceSpan(sourceSpan, splitInterpolation.stringSpans[lastStringIdx]);
nodes.push(new i18n.Text(splitInterpolation.strings[lastStringIdx], stringSpan));
}
if (previousI18n instanceof i18n.Message) {
// The `previousI18n` is an i18n `Message`, so we are processing an `Attribute` with i18n
// metadata. The `Message` should consist only of a single `Container` that contains the
// parts (`Text` and `Placeholder`) to process.
assertSingleContainerMessage(previousI18n);
previousI18n = previousI18n.nodes[0];
}
if (previousI18n instanceof i18n.Container) {
// The `previousI18n` is a `Container`, which means that this is a second i18n extraction pass
// after whitespace has been removed from the AST ndoes.
assertEquivalentNodes(previousI18n.children, nodes);
// Reuse the source-spans from the first pass.
for (let i = 0; i < nodes.length; i++) {
nodes[i].sourceSpan = previousI18n.children[i].sourceSpan;
}
}
return container;
}
}
function assertSingleContainerMessage(message: i18n.Message): void {
const nodes = message.nodes;
if (nodes.length !== 1 || !(nodes[0] instanceof i18n.Container)) {
throw new Error(
'Unexpected previous i18n message - expected it to consist of only a single `Container` node.');
}
}
function assertEquivalentNodes(previousNodes: i18n.Node[], nodes: i18n.Node[]): void {
if (previousNodes.length !== nodes.length) {
throw new Error('The number of i18n message children changed between first and second pass.');
}
if (previousNodes.some((node, i) => nodes[i].constructor !== node.constructor)) {
throw new Error(
'The types of the i18n message children changed between first and second pass.');
}
}
function getOffsetSourceSpan(
sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan {
return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end));