Skip to content

Commit fe3e4d6

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
fix(compiler-cli): Handle ng-template with structural directive in indexer (#44788)
An `ng-template` with an inline template (i.e. has a structural directive) would previously not get an `undefined` `tagName` because the logic assumed the element would be `t.Element` or `t.Content` and read the tag name from the `name` property. For a `t.Template`, this exists instead on the `t.tagName`. The final result would be an `tagName` of `undefined` for the parent `t.Template`, causing failures in the indexer downstream. This `undefined` value is actually expected in the renderer code, even though the type does not specify this possibility. This change updates the type of `tagName` to be `string|null` and explicitly handles the case where there is a structural directive on an `ng-template`. You can see how the two are differentiated in the compliance code that was modified in this commit. PR Close #44788
1 parent 6cb7c3e commit fe3e4d6

File tree

10 files changed

+55
-17
lines changed

10 files changed

+55
-17
lines changed

‎packages/compiler-cli/src/ngtsc/indexer/src/template.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class TemplateVisitor extends TmplAstRecursiveVisitor {
229229
let name: string;
230230
let kind: IdentifierKind.Element|IdentifierKind.Template;
231231
if (node instanceof TmplAstTemplate) {
232-
name = node.tagName;
232+
name = node.tagName ?? 'ng-template';
233233
kind = IdentifierKind.Template;
234234
} else {
235235
name = node.name;

‎packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,20 @@ runInEachFileSystem(() => {
7575
});
7676
});
7777

78+
it('works when structural directives are on templates', () => {
79+
const template = '<ng-template *ngIf="true">';
80+
const refs = getTemplateIdentifiers(bind(template));
81+
82+
const [ref] = Array.from(refs);
83+
expect(ref).toEqual({
84+
kind: IdentifierKind.Template,
85+
name: 'ng-template',
86+
span: new AbsoluteSourceSpan(1, 12),
87+
usedDirectives: new Set(),
88+
attributes: new Set(),
89+
});
90+
});
91+
7892
it('should generate nothing in empty template', () => {
7993
const template = '';
8094
const refs = getTemplateIdentifiers(bind(template));

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/ng-template_interpolation_structural.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ consts: function() {
2020
},
2121
template: function MyComponent_Template(rf, ctx) {
2222
if (rf & 1) {
23-
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 1, undefined, 0);
23+
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 1, null, 0);
2424
}
2525
if (rf & 2) {
2626
$r3$.ɵɵproperty("ngIf", true);

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/ng-template_structural.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ consts: function() {
1919
},
2020
template: function MyComponent_Template(rf, ctx) {
2121
if (rf & 1) {
22-
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, undefined, 0);
22+
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, null, 0);
2323
}
2424
if (rf & 2) {
2525
$r3$.ɵɵproperty("ngIf", ctx.visible);

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/ng-container_ng-template/structural_directives.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ consts: function() {
3131
},
3232
template: function MyComponent_Template(rf, ctx) {
3333
if (rf & 1) {
34-
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, undefined, 0);
34+
$r3$.ɵɵtemplate(0, MyComponent_0_Template, 1, 0, null, 0);
3535
$r3$.ɵɵtemplate(1, MyComponent_ng_container_1_Template, 2, 0, "ng-container", 0);
3636
}
3737
if (rf & 2) {

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_interpolated_prop_with_structural_directive_outer_template.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
consts: [[4, "ngIf"], [3, "dir"]],
22
template: function TestComp_Template(rf, ctx) {
33
if (rf & 1) {
4-
$i0$.ɵɵtemplate(0, $TestComp_0_Template$, 1, 1, undefined, 0);
4+
$i0$.ɵɵtemplate(0, $TestComp_0_Template$, 1, 1, null, 0);
55
}
66
if (rf & 2) {
77
$i0$.ɵɵproperty("ngIf", true);

‎packages/compiler/src/render3/r3_ast.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,23 @@ export class Element implements Node {
119119

120120
export class Template implements Node {
121121
constructor(
122-
public tagName: string, public attributes: TextAttribute[], public inputs: BoundAttribute[],
123-
public outputs: BoundEvent[], public templateAttrs: (BoundAttribute|TextAttribute)[],
124-
public children: Node[], public references: Reference[], public variables: Variable[],
125-
public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan,
126-
public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nMeta) {}
122+
// tagName is the name of the container element, if applicable.
123+
// `null` is a special case for when there is a structural directive on an `ng-template` so
124+
// the renderer can differentiate between the synthetic template and the one written in the
125+
// file.
126+
public tagName: string|null,
127+
public attributes: TextAttribute[],
128+
public inputs: BoundAttribute[],
129+
public outputs: BoundEvent[],
130+
public templateAttrs: (BoundAttribute|TextAttribute)[],
131+
public children: Node[],
132+
public references: Reference[],
133+
public variables: Variable[],
134+
public sourceSpan: ParseSourceSpan,
135+
public startSourceSpan: ParseSourceSpan,
136+
public endSourceSpan: ParseSourceSpan|null,
137+
public i18n?: I18nMeta,
138+
) {}
127139
visit<Result>(visitor: Visitor<Result>): Result {
128140
return visitor.visitTemplate(this);
129141
}

‎packages/compiler/src/render3/r3_template_transform.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class HtmlAstToIvyAst implements html.Visitor {
186186
const children: t.Node[] =
187187
html.visitAll(preparsedElement.nonBindable ? NON_BINDABLE_VISITOR : this, element.children);
188188

189-
let parsedElement: t.Node|undefined;
189+
let parsedElement: t.Content|t.Template|t.Element|undefined;
190190
if (preparsedElement.type === PreparsedElementType.NG_CONTENT) {
191191
// `<ng-content>`
192192
if (element.children &&
@@ -235,13 +235,12 @@ class HtmlAstToIvyAst implements html.Visitor {
235235
// the wrapping template to prevent unnecessary i18n instructions from being generated. The
236236
// necessary i18n meta information will be extracted from child elements.
237237
const i18n = isTemplateElement && isI18nRootElement ? undefined : element.i18n;
238+
const name = parsedElement instanceof t.Template ? null : parsedElement.name;
238239

239-
// TODO(pk): test for this case
240240
parsedElement = new t.Template(
241-
(parsedElement as t.Element | t.Content).name, hoistedAttrs.attributes,
242-
hoistedAttrs.inputs, hoistedAttrs.outputs, templateAttrs, [parsedElement],
243-
[/* no references */], templateVariables, element.sourceSpan, element.startSourceSpan,
244-
element.endSourceSpan, i18n);
241+
name, hoistedAttrs.attributes, hoistedAttrs.inputs, hoistedAttrs.outputs, templateAttrs,
242+
[parsedElement], [/* no references */], templateVariables, element.sourceSpan,
243+
element.startSourceSpan, element.endSourceSpan, i18n);
245244
}
246245
if (isI18nRootElement) {
247246
this.inI18nBlock = false;

‎packages/compiler/test/render3/r3_template_transform_spec.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {BindingType} from '../../src/expression_parser/ast';
1010
import * as t from '../../src/render3/r3_ast';
1111
import {unparse} from '../expression_parser/utils/unparser';
12+
1213
import {parseR3 as parse} from './view/util';
1314

1415

@@ -272,6 +273,18 @@ describe('R3 template transform', () => {
272273
]);
273274
});
274275

276+
it('should support <ng-template> with structural directive', () => {
277+
expectFromHtml('<ng-template *ngIf="true"></ng-template>').toEqual([
278+
['Template'],
279+
['BoundAttribute', 0, 'ngIf', 'true'],
280+
['Template'],
281+
]);
282+
const res = parse('<ng-template *ngIf="true"></ng-template>', {ignoreError: false});
283+
expect((res.nodes[0] as t.Template).tagName).toEqual(null);
284+
expect(((res.nodes[0] as t.Template).children[0] as t.Template).tagName)
285+
.toEqual('ng-template');
286+
});
287+
275288
it('should support reference via #...', () => {
276289
expectFromHtml('<ng-template #a></ng-template>').toEqual([
277290
['Template'],

‎packages/language-service/src/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ function toAttributeCssSelector(attribute: t.TextAttribute|t.BoundAttribute|t.Bo
183183
}
184184

185185
function getNodeName(node: t.Template|t.Element): string {
186-
return node instanceof t.Template ? node.tagName : node.name;
186+
return node instanceof t.Template ? (node.tagName ?? 'ng-template') : node.name;
187187
}
188188

189189
/**

0 commit comments

Comments
 (0)