Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

refactor(compiler): Fix self-closing element source spans in template pipeline (#53574)

Browse source 
When an element is self-closing, it will cause an `element` instruction to be emitted (instead of `elementStart`/`elementEnd`). In that case, we should use map whole source span for the instruction, not just the starting span.

PR Close #53574
This commit is contained in:
Dylan Hunn 2023-12-14 10:56:35 -08:00 committed by Jessica Janiuk
parent a5565d176a
commit ea71ff8ef7
4 changed files with 64 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

63
packages/compiler-cli/test/ngtsc/template_mapping_spec.ts
View file

@ -426,36 +426,39 @@ runInEachFileSystem((os) => {
});
});
it('should correctly handle collapsed whitespace in interpolation placeholder source-mappings',
async () => {
const mappings = await compileAndMap(
`<div i18n title=" pre-title {{name}} post-title" i18n-title> pre-body {{greeting}} post-body</div>`);
expectMapping(mappings, {
source: '<div i18n title=" pre-title {{name}} 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: '{{greeting}}',
generated: '"\\uFFFD0\\uFFFD"',
sourceUrl: '../test.ts',
});
expectMapping(mappings, {
source: ' post-body',
generated: '}:INTERPOLATION: post-body`',
sourceUrl: '../test.ts',
});
});
// TODO: Temporarily disabled because Template Pipeline produces different const indices
// than TemplateDefinitionBuilder. Re-enable after Template Pipeline is the default, and
// update the test with the new const index.
xit('should correctly handle collapsed whitespace in interpolation placeholder source-mappings',
async () => {
const mappings = await compileAndMap(
`<div i18n title=" pre-title {{name}} post-title" i18n-title> pre-body {{greeting}} post-body</div>`);
expectMapping(mappings, {
source: '<div i18n title=" pre-title {{name}} 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: '{{greeting}}',
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',
async () => {

29
packages/compiler/src/template/pipeline/ir/src/ops/create.ts
View file

@ -101,7 +101,15 @@ export interface ElementOrContainerOpBase extends Op<CreateOp>, ConsumesSlotOpTr
*/
nonBindable: boolean;
sourceSpan: ParseSourceSpan;
/**
* The span of the element's start tag.
*/
startSourceSpan: ParseSourceSpan;
/**
* The whole source span of the element, including children.
*/
wholeSourceSpan: ParseSourceSpan;
}
export interface ElementOpBase extends ElementOrContainerOpBase {
@ -135,7 +143,7 @@ export interface ElementStartOp extends ElementOpBase {
*/
export function createElementStartOp(
tag: string, xref: XrefId, namespace: Namespace, i18nPlaceholder: i18n.TagPlaceholder|undefined,
sourceSpan: ParseSourceSpan): ElementStartOp {
startSourceSpan: ParseSourceSpan, wholeSourceSpan: ParseSourceSpan): ElementStartOp {
return {
kind: OpKind.ElementStart,
xref,
@ -146,7 +154,8 @@ export function createElementStartOp(
nonBindable: false,
namespace,
i18nPlaceholder,
sourceSpan,
startSourceSpan,
wholeSourceSpan,
...TRAIT_CONSUMES_SLOT,
...NEW_OP,
};
@ -201,7 +210,7 @@ export interface TemplateOp extends ElementOpBase {
export function createTemplateOp(
xref: XrefId, templateKind: TemplateKind, tag: string|null, functionNameSuffix: string,
namespace: Namespace, i18nPlaceholder: i18n.TagPlaceholder|i18n.BlockPlaceholder|undefined,
sourceSpan: ParseSourceSpan): TemplateOp {
startSourceSpan: ParseSourceSpan, wholeSourceSpan: ParseSourceSpan): TemplateOp {
return {
kind: OpKind.Template,
xref,
@ -216,7 +225,8 @@ export function createTemplateOp(
nonBindable: false,
namespace,
i18nPlaceholder,
sourceSpan,
startSourceSpan,
wholeSourceSpan,
...TRAIT_CONSUMES_SLOT,
...NEW_OP,
};
@ -280,8 +290,6 @@ export interface RepeaterCreateOp extends ElementOpBase {
* The i18n placeholder for the empty template.
*/
emptyI18nPlaceholder: i18n.BlockPlaceholder|undefined;
sourceSpan: ParseSourceSpan;
}
// TODO: add source spans?
@ -298,8 +306,8 @@ export interface RepeaterVarNames {
export function createRepeaterCreateOp(
primaryView: XrefId, emptyView: XrefId|null, tag: string|null, track: o.Expression,
varNames: RepeaterVarNames, i18nPlaceholder: i18n.BlockPlaceholder|undefined,
emptyI18nPlaceholder: i18n.BlockPlaceholder|undefined,
sourceSpan: ParseSourceSpan): RepeaterCreateOp {
emptyI18nPlaceholder: i18n.BlockPlaceholder|undefined, startSourceSpan: ParseSourceSpan,
wholeSourceSpan: ParseSourceSpan): RepeaterCreateOp {
return {
kind: OpKind.RepeaterCreate,
attributes: null,
@ -319,7 +327,8 @@ export function createRepeaterCreateOp(
usesComponentInstance: false,
i18nPlaceholder,
emptyI18nPlaceholder,
sourceSpan,
startSourceSpan,
wholeSourceSpan,
...TRAIT_CONSUMES_SLOT,
...NEW_OP,
numSlotsUsed: emptyView === null ? 2 : 3,

12
packages/compiler/src/template/pipeline/src/ingest.ts
View file

@ -181,7 +181,7 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
const startOp = ir.createElementStartOp(
elementName, id, namespaceForKey(namespaceKey),
element.i18n instanceof i18n.TagPlaceholder ? element.i18n : undefined,
element.startSourceSpan);
element.startSourceSpan, element.sourceSpan);
unit.create.push(startOp);
ingestElementBindings(unit, startOp, element);
@ -236,7 +236,7 @@ function ingestTemplate(unit: ViewCompilationUnit, tmpl: t.Template): void {
isPlainTemplate(tmpl) ? ir.TemplateKind.NgTemplate : ir.TemplateKind.Structural;
const templateOp = ir.createTemplateOp(
childView.xref, templateKind, tagNameWithoutNamespace, functionNameSuffix, namespace,
i18nPlaceholder, tmpl.startSourceSpan);
i18nPlaceholder, tmpl.startSourceSpan, tmpl.sourceSpan);
unit.create.push(templateOp);
ingestTemplateBindings(unit, templateOp, tmpl, templateKind);
@ -359,7 +359,7 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
const templateOp = ir.createTemplateOp(
cView.xref, ir.TemplateKind.Block, tagName, 'Conditional', ir.Namespace.HTML,
ifCaseI18nMeta, ifCase.sourceSpan);
ifCaseI18nMeta, ifCase.startSourceSpan, ifCase.sourceSpan);
unit.create.push(templateOp);
if (firstXref === null) {
@ -397,7 +397,7 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
}
const templateOp = ir.createTemplateOp(
cView.xref, ir.TemplateKind.Block, null, 'Case', ir.Namespace.HTML, switchCaseI18nMeta,
switchCase.sourceSpan);
switchCase.startSourceSpan, switchCase.sourceSpan);
unit.create.push(templateOp);
if (firstXref === null) {
firstXref = cView.xref;
@ -430,7 +430,7 @@ function ingestDeferView(
ingestNodes(secondaryView, children);
const templateOp = ir.createTemplateOp(
secondaryView.xref, ir.TemplateKind.Block, null, `Defer${suffix}`, ir.Namespace.HTML,
i18nMeta, sourceSpan!);
i18nMeta, sourceSpan!, sourceSpan!);
unit.create.push(templateOp);
return templateOp;
}
@ -627,7 +627,7 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo
const tagName = ingestControlFlowInsertionPoint(unit, repeaterView.xref, forBlock);
const repeaterCreate = ir.createRepeaterCreateOp(
repeaterView.xref, emptyView?.xref ?? null, tagName, track, varNames, i18nPlaceholder,
emptyI18nPlaceholder, forBlock.sourceSpan);
emptyI18nPlaceholder, forBlock.startSourceSpan, forBlock.sourceSpan);
unit.create.push(repeaterCreate);
const expression = convertAst(

12
packages/compiler/src/template/pipeline/src/phases/reify.ts
View file

@ -49,14 +49,14 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
op,
ng.elementStart(
op.handle.slot!, op.tag!, op.attributes as number | null,
op.localRefs as number | null, op.sourceSpan));
op.localRefs as number | null, op.startSourceSpan));
break;
case ir.OpKind.Element:
ir.OpList.replace(
op,
ng.element(
op.handle.slot!, op.tag!, op.attributes as number | null,
op.localRefs as number | null, op.sourceSpan));
op.localRefs as number | null, op.wholeSourceSpan));
break;
case ir.OpKind.ElementEnd:
ir.OpList.replace(op, ng.elementEnd(op.sourceSpan));
@ -66,14 +66,14 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
op,
ng.elementContainerStart(
op.handle.slot!, op.attributes as number | null, op.localRefs as number | null,
op.sourceSpan));
op.startSourceSpan));
break;
case ir.OpKind.Container:
ir.OpList.replace(
op,
ng.elementContainer(
op.handle.slot!, op.attributes as number | null, op.localRefs as number | null,
op.sourceSpan));
op.wholeSourceSpan));
break;
case ir.OpKind.ContainerEnd:
ir.OpList.replace(op, ng.elementContainerEnd());
@ -107,7 +107,7 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
op,
ng.template(
op.handle.slot!, o.variable(childView.fnName!), childView.decls!, childView.vars!,
op.tag, op.attributes, op.localRefs, op.sourceSpan),
op.tag, op.attributes, op.localRefs, op.startSourceSpan),
);
break;
case ir.OpKind.DisableBindings:
@ -239,7 +239,7 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
ng.repeaterCreate(
op.handle.slot, repeaterView.fnName, op.decls!, op.vars!, op.tag, op.attributes,
op.trackByFn!, op.usesComponentInstance, emptyViewFnName, emptyDecls, emptyVars,
op.sourceSpan));
op.wholeSourceSpan));
break;
case ir.OpKind.Statement:
// Pass statement operations directly through.