fix(compiler): throw error for duplicate template references

[crisbeto]

Adds an error if a reference is used more than once on the same element (e.g. <div #a #a>). We used to have this error in ViewEngine, but it wasn't ported over to Ivy.

Fixes #40536.

[Airblader]

@crisbeto What about the other case mentioned in the ticket where the reference is not on the same element, but on two?

If I understand correctly this also fixes that one, but no test for it has been added.

[crisbeto]

It looks like I missed the other case and this code only checked within the same element. I've updated the code to check across elements as well.

[JoostK]

Reference names are scoped to their containing template; whereas it looks like this tracks a global set. The VE template parser appears to be buggy here as well, in that it only checks for duplicated variable declarations in the root, not in nested templates. E.g. this correctly succeeds in VE:

<div *ngIf="true" #divRef></div>
<div #divRef></div>

but this should be rejected, but is incorrectly accepted:

<div *ngIf="true" #divRef #divRef></div>

Also note that the similar operation of reporting duplicate template variables (which is different to reference declarations, to be clear) is currently checked during type-checking:

angular/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines 1142 to 1152 in d466db8

	// The template's variable declarations need to be added as `TcbVariableOp`s.
	const varMap = new Map<string, TmplAstVariable>();
	for (const v of templateOrNodes.variables) {
	// Validate that variables on the `TmplAstTemplate` are only declared once.
	if (!varMap.has(v.name)) {
	varMap.set(v.name, v);
	} else {
	const firstDecl = varMap.get(v.name)!;
	tcb.oobRecorder.duplicateTemplateVar(tcb.id, v, firstDecl);
	}

That's just for reference, though; I think it's fine to report this error from within the template transform.

[crisbeto]

My understanding was that we create one HtmlToIvyAst per parsed file so the registry shouldn't be global, or am I mistaken? For reference, here's where we create it: https://github.com/angular/angular/blob/master/packages/compiler/src/render3/r3_template_transform.ts#L58

[JoostK]

My usage of global was a bit poor; I meant to say "for a single component template". The single set is a problem for e.g:

<div *ngIf="true" #divRef></div>
<div #divRef></div>

as the first divRef occurs within an ng-template which creates a new scope, so the two declarations are actually not conflicting.

[crisbeto]

I see what you mean now. I suppose that we can make it work, but the question is whether it's worth the trouble. In my opinion, even though the names won't conflict at runtime, it's cleaner if they're all unique anyway.

[crisbeto]

Now that I'm looking at it again, it seems like Ivy explicitly supports multiple local references within the same template. See https://github.com/angular/angular/blob/master/packages/core/test/acceptance/query_spec.ts#L64. I'll roll back the changes so they only verify that the references are unique within a particular element.

[AndrewKushnir]

Presubmit.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}