Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

fix(compiler-cli): add compiler option to disable control flow content projection diagnostic#53311

Closed
crisbeto wants to merge 1 commit intoangular:mainangular/angular:mainfrom
crisbeto:disable-control-flow-diagcrisbeto/angular:disable-control-flow-diagCopy head branch name to clipboard
Closed

fix(compiler-cli): add compiler option to disable control flow content projection diagnostic#53311
crisbeto wants to merge 1 commit intoangular:mainangular/angular:mainfrom
crisbeto:disable-control-flow-diagcrisbeto/angular:disable-control-flow-diagCopy head branch name to clipboard

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Dec 1, 2023

These changes add an option to the extendedDiagnostics field that allows the check from #53190 to be disabled. This is a follow-up based on a recent discussion.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release labels Dec 1, 2023
@ngbot ngbot bot modified the milestone: Backlog Dec 1, 2023
Copy link
Member Author

@crisbeto crisbeto Dec 1, 2023

Choose a reason for hiding this comment

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

Note: it feels a bit weird to be special-casing this diagnostic as an extended diagnostic that's built into the compiler, but at the moment it seem possible to implement this as an extended diagnostic.

I have a branch where the check is implemented as an actual diagnostic and it passes our own tests, however when I tried installing it in two separate apps (Material dev app and an ng new app), it didn't work. It seems like calling TemplateTypeChecker.getSymbolOfNode from an extended diagnostic doesn't work.

Since the option do disable it is in the extendedDiagnostics field, we can easily swap out the implementation later without changing the functionality.

…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
@crisbeto crisbeto force-pushed the disable-control-flow-diag branch from 3f0a768 to 86b5618 Compare December 1, 2023 12:54
@crisbeto crisbeto marked this pull request as ready for review December 1, 2023 13:01
Copy link
Contributor

@jessicajaniuk jessicajaniuk 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

@pullapprove pullapprove bot requested a review from atscott December 1, 2023 14:47
Copy link
Contributor

@AndrewKushnir AndrewKushnir 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

@crisbeto crisbeto removed the request for review from atscott December 5, 2023 16:22
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 5, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Dec 6, 2023

This PR was merged into the repository by commit e620b3a.

@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 Jan 6, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…t projection diagnostic (angular#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close angular#53311
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…t projection diagnostic (angular#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close angular#53311
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…t projection diagnostic (angular#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close angular#53311
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.