Control flow schematic: @for (document of <collection>; track document) results in template not compiling

[hamfastgamgee]

Which @angular/* package(s) are the source of the bug?

compiler-cli, core

Is this a regression?

No

Description

We had a component processed by the migration that resulted in our iteration variable and our track variable both being named "document", but the template parser choked on this and said that all the HTML inside the @for did not have a "createElement" method. It seems clear that the parser got confused with the global "document" object.

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

No response

Anything else?

No response

[AndrewKushnir]

@hamfastgamgee could you please provide an example of a part of a template that is not migrated correctly and an error message that you see? Please also make sure you are using the most recent version of Angular packages (17.0.5), since there were a number of improvements merged recently. Thank you.

[hamfastgamgee]

Honestly, I just renamed "document" to "historicalDocument" in my case and moved on to the next dozen files that needed TLC. The migration did exactly like I expected, changing *ngFor="let document of history" to @for (document of history; track document), but it seemed like the template parser choked on the variable being named "document" in both my IDE and a build, complaining that the div elements inside the @for lacked a .create Element() method or something to that effect. From the behavior, I suspect that my local "document" variable was scoped such that the global document object needed for DOM construction was being overridden. I imagine it would be replicable in any template where the variable inside @for is named "document". (I have no idea if any other global properties would be similarly overridden.)

[crisbeto]

I'm guessing the document property is assigned to the actual document, because this test passes for me both in our testing environment and in Material's development app:

fit('', () => {
  @Component({
    template: '@for (document of document; track document) {{{document}}|}',
  })
  class TestComponent {
    document = [1, 2, 3];
  }

  const fixture = TestBed.createComponent(TestComponent);
  fixture.detectChanges();
  expect(fixture.nativeElement.textContent).toBe('1|2|3|');

  fixture.componentInstance.document.pop();
  fixture.detectChanges();
  expect(fixture.nativeElement.textContent).toBe('1|2|');

  fixture.componentInstance.document.push(3);
  fixture.detectChanges();
  expect(fixture.nativeElement.textContent).toBe('1|2|3|');

  fixture.componentInstance.document[0] = 3;
  fixture.componentInstance.document[2] = 1;
  fixture.detectChanges();
  expect(fixture.nativeElement.textContent).toBe('3|2|1|');
});

[hamfastgamgee]

Does it do the same if the contents are HTML elements? My case had at least one div element and maybe a Material component in there. The only thing that was named "document" was the variable used for each item in the collection, not the collection itself, FWIW. Would love to find out this was just a "me" problem, especially since my workaround of renaming the variable was foolproof. And to answer an earlier question I missed: I was definitely running on 7.0.5 when this happened.

[crisbeto]

How did you get a hold of the elements? Was it an array you constructed or through something like querySelectorAll?

[hamfastgamgee]

Looking back at the component in question, it was an array of strongly-typed objects brought in via @input. However, I can reproduce it without that, at least locally on 7.0.5.

This is the most minimal template I have tried (trying to rip out everything specific to our codebase):

import { Component } from '@angular/core';

@Component({
    selector: 'upload-history',
    standalone: true,
    templateUrl: './upload-history.component.html',
})
export class UploadHistoryComponent {
    history: { fileSize: number }[];
}

@for (document of history; track document) {
    <div>
    </div>
}

In VSCode for me, this lights up the inner div element as an error with the following message:

Property 'createElement' does not exist on type '{ fileSize: number; }'.ngtsc(2339)
upload-history.component.ts(4, 4): Error occurs in the template of component UploadHistoryComponent.

[JoostK]

We do use document.createElement for DOM-based type-checking (e.g. for native event listeners), it appears that template variables may be emitted as-is in the TCB such that the ambient document global becomes aliased. This would be a bug in the TCB generation, not the control flow migration.

[crisbeto]

@JoostK we do emit the variables as is, but I would expect the local variables to take precedence over the global document. For reference, we emit something like this:

for (const document of this.document) {
  var _t1 = document;
  // do things with _t1...
}

[crisbeto]

Ah, I see now. If we have other document.createElement calls further down inside the loop, they'll refer to the local variable as well.