feat(compiler): report unknown property bindings on explicit ng-templa… - #69640

This PR introduce a little Breaking Change. Existing code that has unused properties in directives will receive a compiler error.

Presubmit came back green. I'll have to run a TGP to see how many breakages it triggers. This will help us determine how we can land this change.

Presubmit came back green. I'll have to run a TGP to see how many breakages it triggers. This will help us determine how we can land this change.

Let me know how the TGP looks. I'm ready to help with any adjustments or schematics if it triggers some internal breakages. Thanks!

Yeah having a schematic to help with the cleanup would be great.
The breakages I've seen are all missing imports or inputs that probably don't exist anymore after directives evolved.

@JeanMeche
Makes sense! I'll open a separate PR with the cleanup schematic to handle those template fixes automatically. Thanks for the heads up!

The TGP raised (at least) a false positive:

<ng-template>
  <xhtml:div xmlns:xhtml="http://www.w3.org/1999/xhtml">
    <ng-template [ngIf]="true"> </ng-template>
  </xhtml:div>
</ng-template>

Throws with your changes while NgIf being correctly imported.

Another simpler one that reports a false positive:

<ng-template #foo>foo</ng-template>
<svg>
  <ng-template [ngTemplateOutlet]="foo"></ng-template>
</svg>

There is another false positive that gets flaged:

import {NgTemplateOutlet} from '@angular/common';
import {Component, Directive} from '@angular/core';
@Directive({
  selector: '[foobar]',
})
export class FoobarDirective {
  constructor() {
    console.log('foobar directive');
  }
}

@Component({
  selector: 'app-root',
  imports: [FoobarDirective, NgTemplateOutlet],
  template: `
    <ng-template #foo [foobar]>foo</ng-template>

    <ng-container [ngTemplateOutlet]="foo"></ng-container>
  `,
})
export class App {}

[foobar] is currently being accepted correctly at runtime as matching the selector.

There is another false positive that gets flaged:

import {NgTemplateOutlet} from '@angular/common';
import {Component, Directive} from '@angular/core';
@Directive({
  selector: '[foobar]',
})
export class FoobarDirective {
  constructor() {
    console.log('foobar directive');
  }
}

@Component({
  selector: 'app-root',
  imports: [FoobarDirective, NgTemplateOutlet],
  template: `
    <ng-template #foo [foobar]>foo</ng-template>

    <ng-container [ngTemplateOutlet]="foo"></ng-container>
  `,
})
export class App {}

[foobar] is currently being accepted correctly at runtime as matching the selector.

Good catch, thanks! [foobar] matches the directive's selector even without a corresponding input, so it's valid usage that shouldn't be flagged. I've updated the check to only report bindings on an when no directive matched the node at all, this still covers the missing-import scenario from #69322, while tolerating selector-only bindings like this one.

I see that you just added the action: merge label, but the following checks are still failing:
     conflicts with base branch "main"
     status "google-internal-tests" is pending
     status "pullapprove" is pending
     1 pending code review

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@aparzi This needs a rebase and has conflicts that need resolving. Can you take a look?

@aparzi This needs a rebase and has conflicts that need resolving. Can you take a look?

yes

@thePunderWoman I think @JeanMeche did a rebase

@aparzi @JeanMeche Looks like this is conflicted again.

This PR was merged into the repository. The changes were merged into the following branches:

• main: e4f6744