Skip to content

feat(compiler-cli): add diagnostic for uninvoked functions in text interpolation #59191

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat(compiler-cli): add diagnostic for uninvoked functions in text interpolation #59191

wants to merge 1 commit into from

Conversation

RafaelJCamara
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

We don't perform this check whenever we have an uninvoked function in a text interpolation.

Issue Number: #59157

What is the new behavior?

Whenever we have an uninvoked function in a text interpolation, a warning is triggered (NG8114).

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from alxhub December 14, 2024 10:32
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Dec 14, 2024
JeanMeche
JeanMeche reviewed Dec 14, 2024
View reviewed changes
@JeanMeche
Copy link
Member

Could you also make sure to check the linter failures, thank you.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 14, 2024 •
edited
Loading

Deployed adev-preview for b3607ab to: https://ng-dev-previews-fw--pr-angular-angular-59191-adev-prev-ehyguwsu.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@JeanMeche
Copy link
Member

Also, we need to add the entry to adev/src/content/reference/extended-diagnostics/overview.md.

@angular-robot angular-robot bot added the area: docs Related to the documentation label Dec 18, 2024
@ngbot ngbot bot added this to the Backlog milestone Dec 18, 2024
@RafaelJCamara
Copy link
Contributor Author

@JeanMeche I think I've addressed all the comments!

JeanMeche
JeanMeche reviewed Dec 18, 2024
View reviewed changes
@RafaelJCamara
Copy link
Contributor Author

Whenever you have time, please look at the new changes @JeanMeche!

JoostK
JoostK reviewed Dec 19, 2024
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

I suspect these points may have originated in the signal check, but mentioning them here anyway.

@RafaelJCamara RafaelJCamara requested a review from JoostK December 19, 2024 19:21
@RafaelJCamara RafaelJCamara requested a review from JoostK December 19, 2024 21:54
@RafaelJCamara
Copy link
Contributor Author

@JeanMeche @JoostK Changes have been applied and are ready to be blasted with another review!

@RafaelJCamara
Copy link
Contributor Author

@alxhub @JeanMeche @JoostK Do you think this is ready for a review?

@jelbourn jelbourn mentioned this pull request Jan 29, 2025
Closed
JoostK
JoostK reviewed Feb 6, 2025
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

This looks correct for me

@JeanMeche
Copy link
Member

Case you rebase your PR and take the previous comments into account ? Thank you.

@JeanMeche JeanMeche force-pushed the RafaelJCamara/59157-func-text-interpolation branch from 8230854 to 192b848 Compare March 4, 2025 21:20
@angular-robot angular-robot bot added the area: compiler Issues related to `ngc`, Angular's template compiler label Mar 4, 2025
@JeanMeche JeanMeche force-pushed the RafaelJCamara/59157-func-text-interpolation branch 2 times, most recently from b648bc4 to 04791c9 Compare March 4, 2025 21:36
@angular-robot angular-robot bot removed the area: docs Related to the documentation label Mar 4, 2025
@JeanMeche JeanMeche requested a review from crisbeto June 27, 2025 11:12
@JeanMeche
Copy link
Member

The last fix CL from the previous TGP was submitted.

This should be good to go.

@JeanMeche
Copy link
Member

TESTED=TGP

JeanMeche
JeanMeche approved these changes Jul 15, 2025
View reviewed changes
Copy link
Member

@JeanMeche JeanMeche left a 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

thePunderWoman
thePunderWoman approved these changes Jul 15, 2025
View reviewed changes
Copy link
Contributor

@thePunderWoman thePunderWoman left a 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

@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Jul 15, 2025
@AndrewKushnir
Copy link
Contributor

@JeanMeche @thePunderWoman I'd like to propose running a new TGP to make sure there are no regressions. Could you please start a new TGP when you get a chance?

@AndrewKushnir AndrewKushnir added requires: TGP This PR requires a passing TGP before merging is allowed action: global presubmit The PR is in need of a google3 global presubmit labels Jul 15, 2025
@pullapprove pullapprove bot removed action: global presubmit The PR is in need of a google3 global presubmit requires: TGP This PR requires a passing TGP before merging is allowed labels Jul 15, 2025
@JeanMeche
Copy link
Member

I'll take care of it tomorrow morning !

@AndrewKushnir AndrewKushnir added requires: TGP This PR requires a passing TGP before merging is allowed action: global presubmit The PR is in need of a google3 global presubmit and removed action: merge The PR is ready for merge by the caretaker labels Jul 15, 2025
@pullapprove pullapprove bot removed action: global presubmit The PR is in need of a google3 global presubmit requires: TGP This PR requires a passing TGP before merging is allowed labels Jul 15, 2025
@JeanMeche
Copy link
Member

TESTED=TGP

@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label Jul 16, 2025
@kirjs
Copy link
Contributor

kirjs commented Jul 16, 2025

This PR was merged into the repository by commit 5abfe4a.

The changes were merged into the following branches: main

@kirjs kirjs closed this in 5abfe4a Jul 16, 2025
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoostK JoostK JoostK left review comments

@JeanMeche JeanMeche JeanMeche approved these changes

@thePunderWoman thePunderWoman thePunderWoman approved these changes

@crisbeto crisbeto crisbeto approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker adev: preview 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.
7 participants
@RafaelJCamara @JeanMeche @AndrewKushnir @kirjs @JoostK @thePunderWoman @crisbeto