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

refactor(compiler): `element.sourceSpan` should span the `outerHTML` (#38581) 

Previously, the `sourceSpan` and `startSourceSpan` were the same
object, which meant that you had the following situation:

```
element = <div>some content</div>
sourceSpan = <div>
startSourceSpan = <div>
endSourceSpan = </div>
```

This made `sourceSpan` redundant and meant that if you
wanted a span for the whole element including its content
and closing tag, it had to be computed.

Now `sourceSpan` is separated from `startSourceSpan`
resulting in:

```
element = <div>some content</div>
sourceSpan = <div>some content</div>
startSourceSpan = <div>
endSourceSpan = </div>
```

PR Close #38581
This commit is contained in:
Pete Bacon Darwin 2020-08-26 11:56:38 +01:00 committed by Andrew Scott
parent a68f1a78a7
commit 1d8c5d88cd
11 changed files with 59 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/compiler-cli/src/ngtsc/indexer/src/template.ts
View File

@ -246,7 +246,7 @@ class TemplateVisitor extends TmplAstRecursiveVisitor {
name = node.name;
kind = IdentifierKind.Element;
}
const {sourceSpan} = node;
const sourceSpan = node.startSourceSpan;
// An element's or template's source span can be of the form `<element>`, `<element />`, or
// `<element></element>`. Only the selector is interesting to the indexer, so the source is
// searched for the first occurrence of the element (selector) name.

2
packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts
View File

@ -93,7 +93,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker {
}
const diag = makeTemplateDiagnostic(
id, mapping, element.sourceSpan, ts.DiagnosticCategory.Error,
id, mapping, element.startSourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT), errorMsg);
this._diagnostics.push(diag);
}

4
packages/compiler-cli/test/extract_i18n_spec.ts
View File

@ -42,7 +42,7 @@ const EXPECTED_XMB = `<?xml version="1.0" encoding="UTF-8" ?>
<msg id="5811701742971715242" desc="with ICU and other things"><source>src/icu.html:4,6</source>
foo <ph name="ICU"><ex>{ count, plural, =1 {...} other {...}}</ex>{ count, plural, =1 {...} other {...}}</ph>
</msg>
<msg id="7254052530614200029" desc="with placeholders"><source>src/placeholders.html:1</source>Name: <ph name="START_BOLD_TEXT"><ex>&lt;b&gt;</ex>&lt;b&gt;</ph><ph name="NAME"><ex>{{
<msg id="7254052530614200029" desc="with placeholders"><source>src/placeholders.html:1,3</source>Name: <ph name="START_BOLD_TEXT"><ex>&lt;b&gt;</ex>&lt;b&gt;</ph><ph name="NAME"><ex>{{
name // i18n(ph=&quot;name&quot;)
}}</ex>{{
name // i18n(ph=&quot;name&quot;)
@ -182,7 +182,7 @@ const EXPECTED_XLIFF2 = `<?xml version="1.0" encoding="UTF-8" ?>
<unit id="7254052530614200029">
<notes>
<note category="description">with placeholders</note>
<note category="location">src/placeholders.html:1</note>
<note category="location">src/placeholders.html:1,3</note>
</notes>
<segment>
<source>Name: <pc id="0" equivStart="START_BOLD_TEXT" equivEnd="CLOSE_BOLD_TEXT" type="fmt" dispStart="&lt;b&gt;" dispEnd="&lt;/b&gt;"><ph id="1" equiv="NAME" disp="{{

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

@ -342,7 +342,7 @@ runInEachFileSystem((os) => {
expect(mappings).toContain(
{source: '<h3>', generated: 'i0.ɵɵelementStart(0, "h3")', sourceUrl: '../test.ts'});
expect(mappings).toContain({
source: '<ng-content select="title">',
source: '<ng-content select="title"></ng-content>',
generated: 'i0.ɵɵprojection(1)',
sourceUrl: '../test.ts'
});
@ -351,7 +351,7 @@ runInEachFileSystem((os) => {
expect(mappings).toContain(
{source: '<div>', generated: 'i0.ɵɵelementStart(2, "div")', sourceUrl: '../test.ts'});
expect(mappings).toContain({
source: '<ng-content>',
source: '<ng-content></ng-content>',
generated: 'i0.ɵɵprojection(3, 1)',
sourceUrl: '../test.ts'
});

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

@ -83,7 +83,7 @@ class _I18nVisitor implements html.Visitor {
const isVoid: boolean = getHtmlTagDefinition(el.name).isVoid;
const startPhName =
context.placeholderRegistry.getStartTagPlaceholderName(el.name, attrs, isVoid);
context.placeholderToContent[startPhName] = el.sourceSpan.toString();
context.placeholderToContent[startPhName] = el.startSourceSpan.toString();
let closePhName = '';

5
packages/compiler/src/ml_parser/parser.ts
View File

@ -258,7 +258,9 @@ class _TreeBuilder {
}
const end = this._peek.sourceSpan.start;
const span = new ParseSourceSpan(startTagToken.sourceSpan.start, end);
const el = new html.Element(fullName, attrs, [], span, span, undefined);
// Create a separate `startSpan` because `span` will be modified when there is an `end` span.
const startSpan = new ParseSourceSpan(startTagToken.sourceSpan.start, end);
const el = new html.Element(fullName, attrs, [], span, startSpan, undefined);
this._pushElement(el);
if (selfClosing) {
// Elements that are self-closed have their `endSourceSpan` set to the full span, as the
@ -301,6 +303,7 @@ class _TreeBuilder {
// removed from the element stack at this point are closed implicitly, so they won't get
// an end source span (as there is no explicit closing element).
el.endSourceSpan = endSourceSpan;
el.sourceSpan.end = endSourceSpan.end || el.sourceSpan.end;
this._elementStack.splice(stackIndex, this._elementStack.length - stackIndex);
return true;

16
packages/compiler/src/render3/view/template.ts
View File

@ -544,7 +544,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
private addNamespaceInstruction(nsInstruction: o.ExternalReference, element: t.Element) {
this._namespace = nsInstruction;
this.creationInstruction(element.sourceSpan, nsInstruction);
this.creationInstruction(element.startSourceSpan, nsInstruction);
}
/**
@ -671,15 +671,16 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
trimTrailingNulls(parameters));
} else {
this.creationInstruction(
element.sourceSpan, isNgContainer ? R3.elementContainerStart : R3.elementStart,
element.startSourceSpan, isNgContainer ? R3.elementContainerStart : R3.elementStart,
trimTrailingNulls(parameters));
if (isNonBindableMode) {
this.creationInstruction(element.sourceSpan, R3.disableBindings);
this.creationInstruction(element.startSourceSpan, R3.disableBindings);
}
if (i18nAttrs.length > 0) {
this.i18nAttributesInstruction(elementIndex, i18nAttrs, element.sourceSpan);
this.i18nAttributesInstruction(
elementIndex, i18nAttrs, element.startSourceSpan ?? element.sourceSpan);
}
// Generate Listeners (outputs)
@ -695,7 +696,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// Note: it's important to keep i18n/i18nStart instructions after i18nAttributes and
// listeners, to make sure i18nAttributes instruction targets current element at runtime.
if (isI18nRootElement) {
this.i18nStart(element.sourceSpan, element.i18n!, createSelfClosingI18nInstruction);
this.i18nStart(element.startSourceSpan, element.i18n!, createSelfClosingI18nInstruction);
}
}
@ -827,7 +828,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
if (!createSelfClosingInstruction) {
// Finish element construction mode.
const span = element.endSourceSpan || element.sourceSpan;
const span = element.endSourceSpan ?? element.sourceSpan;
if (isI18nRootElement) {
this.i18nEnd(span, createSelfClosingI18nInstruction);
}
@ -919,7 +920,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// elements, in case of inline templates, corresponding instructions will be generated in the
// nested template function.
if (i18nAttrs.length > 0) {
this.i18nAttributesInstruction(templateIndex, i18nAttrs, template.sourceSpan);
this.i18nAttributesInstruction(
templateIndex, i18nAttrs, template.startSourceSpan ?? template.sourceSpan);
}
// Add the input bindings

17
packages/compiler/test/i18n/extractor_merger_spec.ts
View File

@ -322,19 +322,28 @@ import {serializeNodes as serializeHtmlNodes} from '../ml_parser/util/util';
describe('elements', () => {
it('should report nested translatable elements', () => {
expect(extractErrors(`<p i18n><b i18n></b></p>`)).toEqual([
['Could not mark an element as translatable inside a translatable section', '<b i18n>'],
[
'Could not mark an element as translatable inside a translatable section',
'<b i18n></b>'
],
]);
});
it('should report translatable elements in implicit elements', () => {
expect(extractErrors(`<p><b i18n></b></p>`, ['p'])).toEqual([
['Could not mark an element as translatable inside a translatable section', '<b i18n>'],
[
'Could not mark an element as translatable inside a translatable section',
'<b i18n></b>'
],
]);
});
it('should report translatable elements in translatable blocks', () => {
expect(extractErrors(`<!-- i18n --><b i18n></b><!-- /i18n -->`)).toEqual([
['Could not mark an element as translatable inside a translatable section', '<b i18n>'],
[
'Could not mark an element as translatable inside a translatable section',
'<b i18n></b>'
],
]);
});
});
@ -370,7 +379,7 @@ import {serializeNodes as serializeHtmlNodes} from '../ml_parser/util/util';
it('should report when start and end of a block are not at the same level', () => {
expect(extractErrors(`<!-- i18n --><p><!-- /i18n --></p>`)).toEqual([
['I18N blocks should not cross element boundaries', '<!--'],
['Unclosed block', '<p>'],
['Unclosed block', '<p><!-- /i18n --></p>'],
]);
expect(extractErrors(`<p><!-- i18n --></p><!-- /i18n -->`)).toEqual([

15
packages/compiler/test/ml_parser/html_parser_spec.ts
View File

@ -653,7 +653,8 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe
'<div [prop]="v1" (e)="do()" attr="v2" noValue>\na\n</div>', 'TestComp')))
.toEqual([
[
html.Element, 'div', 0, '<div [prop]="v1" (e)="do()" attr="v2" noValue>',
html.Element, 'div', 0,
'<div [prop]="v1" (e)="do()" attr="v2" noValue>\na\n</div>',
'<div [prop]="v1" (e)="do()" attr="v2" noValue>', '</div>'
],
[html.Attribute, '[prop]', 'v1', '[prop]="v1"'],
@ -676,14 +677,14 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe
it('should not set the end source span for void elements', () => {
expect(humanizeDomSourceSpans(parser.parse('<div><br></div>', 'TestComp'))).toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div><br></div>', '<div>', '</div>'],
[html.Element, 'br', 1, '<br>', '<br>', null],
]);
});
it('should not set the end source span for multiple void elements', () => {
expect(humanizeDomSourceSpans(parser.parse('<div><br><hr></div>', 'TestComp'))).toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div><br><hr></div>', '<div>', '</div>'],
[html.Element, 'br', 1, '<br>', '<br>', null],
[html.Element, 'hr', 1, '<hr>', '<hr>', null],
]);
@ -703,19 +704,19 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe
it('should set the end source span for self-closing elements', () => {
expect(humanizeDomSourceSpans(parser.parse('<div><br/></div>', 'TestComp'))).toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div><br/></div>', '<div>', '</div>'],
[html.Element, 'br', 1, '<br/>', '<br/>', '<br/>'],
]);
});
it('should not set the end source span for elements that are implicitly closed', () => {
expect(humanizeDomSourceSpans(parser.parse('<div><p></div>', 'TestComp'))).toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div><p></div>', '<div>', '</div>'],
[html.Element, 'p', 1, '<p>', '<p>', null],
]);
expect(humanizeDomSourceSpans(parser.parse('<div><li>A<li>B</div>', 'TestComp')))
.toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div><li>A<li>B</div>', '<div>', '</div>'],
[html.Element, 'li', 1, '<li>', '<li>', null],
[html.Text, 'A', 2, 'A'],
[html.Element, 'li', 1, '<li>', '<li>', null],
@ -728,7 +729,7 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe
'<div>{count, plural, =0 {msg}}</div>', 'TestComp',
{tokenizeExpansionForms: true})))
.toEqual([
[html.Element, 'div', 0, '<div>', '<div>', '</div>'],
[html.Element, 'div', 0, '<div>{count, plural, =0 {msg}}</div>', '<div>', '</div>'],
[html.Expansion, 'count', 'plural', 1, '{count, plural, =0 {msg}}'],
[html.ExpansionCase, '=0', 2, '=0 {msg}'],
]);

24
packages/compiler/test/render3/r3_ast_spans_spec.ts
View File

@ -173,7 +173,7 @@ describe('R3 AST source spans', () => {
describe('templates', () => {
it('is correct for * directives', () => {
expectFromHtml('<div *ngIf></div>').toEqual([
['Template', '0:11', '0:11', '11:17'],
['Template', '0:17', '0:11', '11:17'],
['TextAttribute', '5:10', '<empty>'],
['Element', '0:17', '0:11', '11:17'],
]);
@ -181,48 +181,48 @@ describe('R3 AST source spans', () => {
it('is correct for <ng-template>', () => {
expectFromHtml('<ng-template></ng-template>').toEqual([
['Template', '0:13', '0:13', '13:27'],
['Template', '0:27', '0:13', '13:27'],
]);
});
it('is correct for reference via #...', () => {
expectFromHtml('<ng-template #a></ng-template>').toEqual([
['Template', '0:16', '0:16', '16:30'],
['Template', '0:30', '0:16', '16:30'],
['Reference', '13:15', '<empty>'],
]);
});
it('is correct for reference with name', () => {
expectFromHtml('<ng-template #a="b"></ng-template>').toEqual([
['Template', '0:20', '0:20', '20:34'],
['Template', '0:34', '0:20', '20:34'],
['Reference', '13:19', '17:18'],
]);
});
it('is correct for reference via ref-...', () => {
expectFromHtml('<ng-template ref-a></ng-template>').toEqual([
['Template', '0:19', '0:19', '19:33'],
['Template', '0:33', '0:19', '19:33'],
['Reference', '13:18', '<empty>'],
]);
});
it('is correct for variables via let-...', () => {
expectFromHtml('<ng-template let-a="b"></ng-template>').toEqual([
['Template', '0:23', '0:23', '23:37'],
['Template', '0:37', '0:23', '23:37'],
['Variable', '13:22', '20:21'],
]);
});
it('is correct for attributes', () => {
expectFromHtml('<ng-template k1="v1"></ng-template>').toEqual([
['Template', '0:21', '0:21', '21:35'],
['Template', '0:35', '0:21', '21:35'],
['TextAttribute', '13:20', '17:19'],
]);
});
it('is correct for bound attributes', () => {
expectFromHtml('<ng-template [k1]="v1"></ng-template>').toEqual([
['Template', '0:23', '0:23', '23:37'],
['Template', '0:37', '0:23', '23:37'],
['BoundAttribute', '13:22', '19:21'],
]);
});
@ -236,7 +236,7 @@ describe('R3 AST source spans', () => {
// <div></div>
// </ng-template>
expectFromHtml('<div *ngFor="let item of items"></div>').toEqual([
['Template', '0:32', '0:32', '32:38'],
['Template', '0:38', '0:32', '32:38'],
['TextAttribute', '5:31', '<empty>'],
['BoundAttribute', '5:31', '25:30'], // *ngFor="let item of items" -> items
['Variable', '13:22', '<empty>'], // let item
@ -250,7 +250,7 @@ describe('R3 AST source spans', () => {
// <div></div>
// </ng-template>
expectFromHtml('<div *ngFor="item of items"></div>').toEqual([
['Template', '0:28', '0:28', '28:34'],
['Template', '0:34', '0:28', '28:34'],
['BoundAttribute', '5:27', '13:17'], // ngFor="item of items" -> item
['BoundAttribute', '5:27', '21:26'], // ngFor="item of items" -> items
['Element', '0:34', '0:28', '28:34'],
@ -259,7 +259,7 @@ describe('R3 AST source spans', () => {
it('is correct for variables via let ...', () => {
expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([
['Template', '0:21', '0:21', '21:27'],
['Template', '0:27', '0:21', '21:27'],
['TextAttribute', '5:20', '<empty>'],
['Variable', '12:19', '18:19'], // let a=b -> b
['Element', '0:27', '0:21', '21:27'],
@ -268,7 +268,7 @@ describe('R3 AST source spans', () => {
it('is correct for variables via as ...', () => {
expectFromHtml('<div *ngIf="expr as local"></div>').toEqual([
['Template', '0:27', '0:27', '27:33'],
['Template', '0:33', '0:27', '27:33'],
['BoundAttribute', '5:26', '12:16'], // ngIf="expr as local" -> expr
['Variable', '6:25', '6:10'], // ngIf="expr as local -> ngIf
['Element', '0:33', '0:27', '27:33'],

11
packages/compiler/test/template_parser/template_parser_spec.ts
View File

@ -2046,7 +2046,7 @@ Property binding a not used by any directive on an embedded template. Make sure
it('should support embedded template', () => {
expect(humanizeTplAstSourceSpans(parse('<ng-template></ng-template>', []))).toEqual([
[EmbeddedTemplateAst, '<ng-template>']
[EmbeddedTemplateAst, '<ng-template></ng-template>']
]);
});
@ -2058,14 +2058,14 @@ Property binding a not used by any directive on an embedded template. Make sure
it('should support references', () => {
expect(humanizeTplAstSourceSpans(parse('<div #a></div>', []))).toEqual([
[ElementAst, 'div', '<div #a>'], [ReferenceAst, 'a', null, '#a']
[ElementAst, 'div', '<div #a></div>'], [ReferenceAst, 'a', null, '#a']
]);
});
it('should support variables', () => {
expect(humanizeTplAstSourceSpans(parse('<ng-template let-a="b"></ng-template>', [])))
.toEqual([
[EmbeddedTemplateAst, '<ng-template let-a="b">'],
[EmbeddedTemplateAst, '<ng-template let-a="b"></ng-template>'],
[VariableAst, 'a', 'b', 'let-a="b"'],
]);
});
@ -2128,7 +2128,7 @@ Property binding a not used by any directive on an embedded template. Make sure
expect(humanizeTplAstSourceSpans(
parse('<svg><circle /><use xlink:href="Port" /></svg>', [tagSel, attrSel])))
.toEqual([
[ElementAst, ':svg:svg', '<svg>'],
[ElementAst, ':svg:svg', '<svg><circle /><use xlink:href="Port" /></svg>'],
[ElementAst, ':svg:circle', '<circle />'],
[DirectiveAst, tagSel, '<circle />'],
[ElementAst, ':svg:use', '<use xlink:href="Port" />'],
@ -2144,7 +2144,8 @@ Property binding a not used by any directive on an embedded template. Make sure
inputs: ['aProp']
}).toSummary();
expect(humanizeTplAstSourceSpans(parse('<div [aProp]="foo"></div>', [dirA]))).toEqual([
[ElementAst, 'div', '<div [aProp]="foo">'], [DirectiveAst, dirA, '<div [aProp]="foo">'],
[ElementAst, 'div', '<div [aProp]="foo"></div>'],
[DirectiveAst, dirA, '<div [aProp]="foo"></div>'],
[BoundDirectivePropertyAst, 'aProp', 'foo', '[aProp]="foo"']
]);
});