Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): reduce false negatives of interpolatedSignalNotInvoked extended diagnostic #57337

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Aug 11, 2024

A prior fix in #57291 has introduced the potential for false negatives, as the used directives
that were checked can be present anywhere in the template, not necessarily on the current element
that is being checked. Fixing this requires reshuffling the check's logic a bit, as the extended
diagnostics infrastructure does not provide access to ancestor nodes, nor do the AST nodes
themselves.

The performance regression that was fixed in #57291 doesn't reappear with this change, although
it may affect performance slightly as potentially more bindings may end up being checked.

JoostK added 2 commits August 11, 2024 22:30
…voked` extended diagnostic

A prior fix in angular#57291 has introduced the potential for false negatives, as the used directives
that were checked can be present anywhere in the template, not necessarily on the current element
that is being checked. Fixing this requires reshuffling the check's logic a bit, as the extended
diagnostics infrastructure does not provide access to ancestor nodes, nor do the AST nodes
themselves.

The performance regression that was fixed in angular#57291 doesn't reappear with this change, although
it may affect performance slightly as potentially more bindings may end up being checked.
…levant

Symbol lookups are somewhat expensive and this instance is easily avoided, as it is expected that
many property reads will not be one of the relevant properties.
@JoostK JoostK added target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler compiler: extended diagnostics labels Aug 11, 2024
@ngbot ngbot bot modified the milestone: Backlog Aug 11, 2024
@JoostK JoostK added the action: review The PR is still awaiting reviews from at least one requested reviewer label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler compiler: extended diagnostics target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant