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

(nullable?.methodOrProperty ?? defaultValue) === defaultValue should be a type guard #58196

Closed
tats-u opened this issue Apr 15, 2024 · 11 comments
Labels
Duplicate An existing issue was already created

Comments

@tats-u
Copy link

tats-u commented Apr 15, 2024

πŸ”Ž Search Terms

  • type guard
  • nullish coalescing (??)
  • optional chaining
  • default value

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about Type Guards

4.9 & Playground

⏯ Playground Link

https://www.typescriptlang.org/play?#code/GYVwdgxgLglg9mABMOcAUAjAhgJwFyIDOUOMYA5gNoC6iAPomCADbP2LgAmApsGd5wCUiAN4AoRJMQ5uUEDiSZcAfgB0zbhSgALRMuWIADMICEAXjNHEAMmuIAslh3q45JTnWbyOwQG4xAL5AA

πŸ’» Code

function foo(bar: string[] | null | undefined) {
    return (bar?.length ?? 0) !== 0 && Math.log(bar.length);
}

πŸ™ Actual behavior

'bar' is possibly 'null' or 'undefined'.

πŸ™‚ Expected behavior

No errors

Additional information about the issue

Math.log(bar.length) can be a React component in real applications.

I have to write the following instead as of now:

function foo(bar: string[] | null | undefined) {
    return bar != null && bar.length !== 0 && Math.log(bar.length);
}
@MartinJohns
Copy link
Contributor

Duplicate of #54825.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Apr 15, 2024
@uhyo
Copy link
Contributor

uhyo commented Apr 16, 2024

I doubt this is a duplicate. The linked issue is all about whether TS should know that 0 < 1, while this issue only requires the knowledge that 0 === 0, which TS already uses in many places.

Also the linked issue mentions another issue ( #48536 (comment) ) but that's a different topic too. This issue does not include any request to reason about β€œor” logics.

@MartinJohns
Copy link
Contributor

The underlying issue is the same.

@tats-u
Copy link
Author

tats-u commented Apr 16, 2024

@MartinJohns You have said that that issue is difficult to be fixed for a "design limitation", but this is a more limited form and easier to be fixed. This issue must not be closed just because of the combination of the "duplication" and that "design limitation".

@tats-u
Copy link
Author

tats-u commented Apr 16, 2024

I have added an additional limitation that the fallback value is the same as the value of the other operand of the equality or non-or-equal comparison (< or >) operator.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2024
@tats-u
Copy link
Author

tats-u commented Apr 19, 2024

Why no advanced notice?

@MartinJohns
Copy link
Contributor

Reopen and remove Duplilate

Ryan considers it a duplicate, that's why he marked the issue as such. He surely read your comment, but it doesn't change anything. The underlaying issue is still the same. Type narrowing is done on variables, not expressions. By adding the ?? 0 to bar?.length you're not checking anything bar related for truthiness anymore, but the entire expression, so bar is not narrowed. Changing that would require a lot of changes in the code base.

Why no advanced notice?

It's been marked as Duplicate for 4 days. That's the advance notice.

@tats-u
Copy link
Author

tats-u commented Apr 19, 2024

@MartinJohns Do you mean #48536 (comment) ?
a ?? "" === "" is as simple as typeof a === "b". The only differences are:

  • the fallback value propagated from the left operand expression when a is nullish is vary from a single value ("undefined")
  • ?? is a binary instead of unary

FYI C# 8.0+'s null flow analysis can recognize this pattern. You will be able to get its specification from https://ecma-international.org/publications-and-standards/standards/ecma-334/ next year. This patern might as well be considered to be added to the backlog.

TS may not be able to handle the type number (not nullish) | 0 (nullish) unlike ("string" | "nunber" | ...) | "undefined" in a expression.

It's been marked as Duplicate for 4 days. That's the advance notice.

FYI, https://github.com/actions/stale, which has been famous, politely writes a post before closing, but your bot does not. However I do not require the bot to be improved because this improvement is just a bullshit job for your team and even hell can be home.

@MartinJohns
Copy link
Contributor

Please note that I am not a member of the TypeScript team. It's not my bot.

Besides that, the bot did write a polite comment before closing the issue.

@tats-u
Copy link
Author

tats-u commented Apr 19, 2024

Oh no, you are not Owner or Member. I have misunderstood and just noticed it. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants