Skip to content

Add a regression test for discriminating property access in parenthesized expression #51778

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

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Dec 6, 2022

No description provided.

Comment on lines 19 to 22
// extra parens on purpose
if (typeof (a.error) === 'undefined') {
a.result.prop; // number
}
Copy link
Contributor Author

@Andarist Andarist Dec 6, 2022

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yup, that was an intended fix. I'm not sure we need this extra test since #51270 already includes a regression test from #51700 with two typeof checks that differ only in parentheses. It was in fact that difference that caused me to fix the issue.

Copy link
Contributor Author

@Andarist Andarist Dec 6, 2022

Choose a reason for hiding this comment

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

#51720 has a test with optional chaining and parenthesis - that has worked before your PR despite the fact that the version without parenthesis reported an error. This test is kinda about the opposite situation (no optional chaining and parenthesis) - the version without parenthesis worked alright while the version with them reported an error.

I've concluded that they are slightly different and thus I've created a PR (I already had it locally as I was looking into fixing #51700 when you picked it up). I understand that they can be seen as similar enough though. Feel free to merge or close - whatever you think is best here.

@sandersn
Copy link
Member

@ahejlsberg It sounded like you were leaning against adding this test case. Does @Andarist 's explanation change that? If not, let's close it.

@sandersn sandersn added the Housekeeping Housekeeping PRs label Dec 22, 2022
…-parent-discriminate-union-narrowing

# Conflicts:
#	tests/baselines/reference/narrowingTypeofUndefined.symbols
#	tests/baselines/reference/narrowingTypeofUndefined1.js
@sandersn sandersn closed this Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Housekeeping PRs
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants