GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat(compiler): report unknown property bindings on explicit ng-templa… by aparzi · Pull Request #69640 · angular/angular

Conversation

@aparzi

@aparzi aparzi commented Jul 3, 2026 •
edited by JeanMeche
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Previously, property bindings on <ng-template> that were not claimed
by a matched directive were silently ignored by the type checker. This
was inconsistent with how bindings on regular elements are handled.

This check is added behind a temporary compiler flag checkTypeOfNgTemplateBindings which will be removed
in the next breaking change window.

When the flag is enabled, the type checker will report an error for any property binding on an <ng-template> that is not matched by a directive.

Fixes #69322

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erkamyaman, JonataB, and mauriziocescon reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
@pullapprove
pullapprove Bot requested a review from kirjs July 3, 2026 21:54
@angular-robot angular-robot Bot added the area: compiler Issues related to `ngc`, Angular's template compiler label Jul 3, 2026
@ngbot ngbot Bot added this to the Backlog milestone Jul 3, 2026
@aparzi
aparzi force-pushed the feat-compiler-missing-import-in-template branch from 4b5c505 to f5a5d21 Compare July 3, 2026 21:56
@aparzi

aparzi commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche

JeanMeche commented Jul 4, 2026

Copy link
Copy Markdown
Member

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.

aparzi reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aparzi

aparzi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche

JeanMeche commented Jul 5, 2026

Copy link
Copy Markdown
Member

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.

aparzi reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche JeanMeche added the target: minor This PR is targeted for the next minor release label Jul 5, 2026
@aparzi

aparzi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche

JeanMeche commented Jul 6, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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>
aparzi reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aparzi
aparzi force-pushed the feat-compiler-missing-import-in-template branch from f5a5d21 to 1d4a41f Compare July 6, 2026 15:54
@JeanMeche

JeanMeche commented Jul 8, 2026

Copy link
Copy Markdown
Member

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.

aparzi reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aparzi
aparzi force-pushed the feat-compiler-missing-import-in-template branch from 1d4a41f to 0ae017a Compare July 8, 2026 20:33
@aparzi

aparzi commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aparzi aparzi mentioned this pull request Jul 11, 2026
feat(migrations): add migration to remove unclaimed bindings on ng-template #69718
Open
4 of 14 tasks
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 0ae017a to 20b6d22 Compare July 14, 2026 09:31
@JeanMeche
JeanMeche requested review from JeanMeche and JoostK and removed request for kirjs July 14, 2026 09:31
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 20b6d22 to 89fad3d Compare July 14, 2026 09:33
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch 4 times, most recently from 5f8cf15 to 4f6ab48 Compare July 31, 2026 12:29
JoostK
JoostK reviewed Aug 2, 2026
View reviewed changes
Comment thread goldens/public-api/compiler-cli/compiler_options.api.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts Outdated
expect(diags.length).toBe(0);
});

it('should not report unclaimed properties on an ng-template that matches a directive', () => {

@JoostK JoostK Aug 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Ah, it's in the comments in TCB generation:

If any directive matched, the node is not checked, since a binding may exist solely to trigger directive matching via its selector.

That's really unfortunate. Perhaps we should be cross-checking against the selector, or simpler: treat the presence of a binding value/expression as requiring a corresponding input. A missing input means the bound value is always disregarded, so may be reported as error. That leaves only bare attributes without value as unreported even if it doesn't correspond with a selector (if we don't cross-check against the selector).

I don't know if we should go as far as checking against the selector, but I'd prefer we do if we can come up with an appropriate check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JoostK JoostK Sep 13, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The force push including a rebase makes GH's "Compare" mode useless so it's now very hard to spot what has changed to this extent. What's the current state of this? I think I may have seen updated tests and parsing a selector, but I also still see the "a binding may exist solely to trigger directive matching" comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JeanMeche JeanMeche Sep 14, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont remember all the details either. The check is now should report unclaimed properties on an ng-template that matches a directive.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
expect(diags.map((d) => d.messageText)).toEqual([]);
});

it('should not report properties on an ng-template inside an svg element', () => {

@JoostK JoostK Aug 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the ng-template treated as svg:ng-template here (inheriting the implicit SVG namespace) for this to not act like ng-template? Shouldn't that report a diagnostic of its own (when custom element schema is not present).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JoostK JoostK Sep 30, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still pending

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JeanMeche JeanMeche Oct 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean here. Would you expect a separate diagnostic if we hit a svg:ng-template ? What would the diagnostic expose ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JoostK JoostK Oct 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test captures behavior where the newly introduced diagnostic doesn't fire for svg:ng-template, because supposedly that isn't an actual ng-template. But then I'd indeed expect an unknown element diagnostic for this scenario.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
);
});

describe('strictNgTemplateTypes', () => {

@JoostK JoostK Aug 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't checked, but does removing the isExplicitNgTemplate condition cause any test failures? I suppose it will, but if not we'd need a test for that (i.e. structural directive syntax)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JeanMeche JeanMeche Aug 17, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not report properties on an ng-template inside an svg element is failing when isExplicitNgTemplate only returns true.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JoostK JoostK Aug 17, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question was whether there's an explicit structural directive test as well, outside of the SVG namespace.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JoostK JoostK Sep 30, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still an open question

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/compiler/src/typecheck/ops/scope.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@SkyZeroZx SkyZeroZx mentioned this pull request Aug 23, 2026
fix(compiler): report unclaimed property bindings on ng-template #70345
Closed
4 of 6 tasks
@JeanMeche JeanMeche mentioned this pull request Aug 29, 2026
Warning for directive not used within the template, when it actually is #70471
Closed
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch 2 times, most recently from bcbdae2 to baeffad Compare August 29, 2026 20:33
@pullapprove
pullapprove Bot requested a review from kirjs November 11, 2026 08:01
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 11, 2026
@JeanMeche
JeanMeche requested review from thePunderWoman and removed request for crisbeto and kirjs November 11, 2026 10:04
@JeanMeche JeanMeche removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 11, 2026
@pullapprove
pullapprove Bot requested a review from kirjs November 11, 2026 10:04
@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label Nov 11, 2026
@ngbot

ngbot Bot commented Nov 11, 2026

Copy link
Copy Markdown

I see that you just added the action: merge label, but the following checks are still failing:
    failure conflicts with base branch "main"
    pending status "google-internal-tests" is pending
    pending status "pullapprove" is pending
    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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thePunderWoman

thePunderWoman commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

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

aparzi reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

thePunderWoman
thePunderWoman approved these changes Nov 11, 2026
View reviewed changes

@thePunderWoman thePunderWoman left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@aparzi

aparzi commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

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

yes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 738837a to 55860b3 Compare November 11, 2026 16:09
@aparzi

aparzi commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@thePunderWoman I think @JeanMeche did a rebase

JeanMeche reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 55860b3 to 49205c8 Compare November 11, 2026 16:42
@thePunderWoman
thePunderWoman removed the request for review from kirjs November 11, 2026 16:54
thePunderWoman
thePunderWoman approved these changes Nov 11, 2026
View reviewed changes

@thePunderWoman thePunderWoman left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@thePunderWoman

thePunderWoman commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

@aparzi @JeanMeche Looks like this is conflicted again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thePunderWoman thePunderWoman added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Nov 11, 2026
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 49205c8 to 7dc15b4 Compare November 11, 2026 17:58
@aparzi @JeanMeche
feat(compiler): report unknown property bindings on explicit ng-templ…
39a5e0d
…ate elements

Previously, property bindings on `<ng-template>` that were not claimed
by a matched directive were silently ignored by the type checker. This
was inconsistent with how bindings on regular elements are handled.

This check is added behind a temporary compiler flag `checkTypeOfNgTemplateBindings` which will be removed
in the next breaking change window.

When the flag is enabled, the type checker will report an error for any property binding on an `<ng-template>` that is not matched by a directive.

Fixes angular#69322

Co-authored-by: Matthieu Riegler <[email protected]>
@JeanMeche
JeanMeche force-pushed the feat-compiler-missing-import-in-template branch from 7dc15b4 to 39a5e0d Compare November 11, 2026 18:42
@JeanMeche
JeanMeche requested review from JeanMeche and removed request for JeanMeche November 11, 2026 21:38
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Nov 11, 2026
thePunderWoman
thePunderWoman approved these changes Nov 11, 2026
View reviewed changes
@thePunderWoman
thePunderWoman merged commit e4f6744 into angular:main Nov 11, 2026
29 of 32 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@thePunderWoman

thePunderWoman commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

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

  • main: e4f6744
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoostK JoostK JoostK left review comments
@JeanMeche JeanMeche JeanMeche approved these changes
@thePunderWoman thePunderWoman thePunderWoman approved these changes
@josephperrott josephperrott josephperrott approved these changes
@alan-agius4 alan-agius4 alan-agius4 approved these changes
@mattrbeck mattrbeck mattrbeck approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Compiler doesn't report missing import for a directive applied on ng-template

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@aparzi @JeanMeche @Flarette @thePunderWoman @JoostK @josephperrott @alan-agius4 @mattrbeck
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.