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): enable narrowing of using type guard methods #44447

Closed
wants to merge 1 commit into from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Dec 11, 2021

The changes in 2028c39 caused method
calls to be emitted using additional parenthesis into the TCB, which in
turn prevented proper type narrowing when the method acts as a type
guard. This commit special-cases method calls from property reads to
avoid the additional parenthesis.

Fixes #44353

@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: template type-checking labels Dec 11, 2021
@ngbot ngbot bot added this to the Backlog milestone Dec 11, 2021
@JoostK JoostK added the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 11, 2021
@JoostK JoostK marked this pull request as ready for review December 11, 2021 21:00
@JoostK JoostK requested a review from crisbeto January 6, 2022 20:20
// For calls that have a property read as receiver, we have to special-case their emit to avoid
// inserting superfluous parenthesis as they prevent TypeScript from applying a narrowing effect
// if the method acts as a type guard.
if (receiver instanceof PropertyRead) {
Copy link
Member

Choose a reason for hiding this comment

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

Would we have to do the same for KeyedRead?

Copy link
Member Author

Choose a reason for hiding this comment

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

The changes in 2028c39 caused method
calls to be emitted using additional parenthesis into the TCB, which in
turn prevented proper type narrowing when the method acts as a type
guard. This commit special-cases method calls from property reads to
avoid the additional parenthesis.

Fixes angular#44353
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@JoostK JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 10, 2022
@atscott
Copy link
Contributor

atscott commented Jan 10, 2022

This PR was merged into the repository by commit 1a91218.

@atscott atscott closed this in 1a91218 Jan 10, 2022
atscott pushed a commit that referenced this pull request Jan 10, 2022
The changes in 2028c39 caused method
calls to be emitted using additional parenthesis into the TCB, which in
turn prevented proper type narrowing when the method acts as a type
guard. This commit special-cases method calls from property reads to
avoid the additional parenthesis.

Fixes #44353

PR Close #44447
@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 Feb 10, 2022
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 action: presubmit The PR is in need of a google3 presubmit area: compiler Issues related to `ngc`, Angular's template compiler compiler: template type-checking target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type narrowing doesn't work as expected in templates in Angular 13
3 participants