[no-unnecessary-condition] False positive unnecessary optional chain on nested void values
<
8000
span class="f1-light color-fg-muted">#9754
Comments
undefined valuesvoid values
|
TIL that TypeScript allows optional chaining access on Not sure this code is good to be allowed though, since |
|
It doesn't look like this reproduces in our playground |
I originally wasn't able to reproduce it in the playground either, which is why I created the gist. But weirdly enough the link you just posted does reproduce for me now. 
|
|
Uh oh! @lukekarrys, the image you shared is missing helpful alt text. Check #9754 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
Something weird.... it wasn't working and then I open my link and it repros 😅 |
I agree that the code is unsound and also quite convoluted. Here is the slightly more real world example that led me to create this reproduction (playground link): const maybeGenerator = () => {
if (Math.random()) {
return
}
return {
*g() {
yield { i: 1 }
},
}
}
maybeGenerator()?.g().next().value?.i
// ~~
// 12:35 error Unnecessary optional chain on a non-nullish value @typescript-eslint/no-unnecessary-condition |
I'd definitely agree with Josh here this code is very unsound! Really we should treat it like |
That sounds good to me. My only annoyance with the current behavior is that it will fix by removing the optional chain, but then TypeScript will error instead. So treating it the same as |
|
I did find this test case which asserts After finding that I experimented a little more and found that it only happens when the root object is nullish and the nested property could be For consistency (if changing the current test cases would be considered a regression) could nested properties be treated the same way as the test case? |
|
@bradzacher Based on my last comment, do you have an opinion on if the fix should be:
I’d like to attempt a PR for this. |
|
+1 to make it behave as |
|
Ideally - both fixes! 😁 Void should generally be treated as any because it's unsafe to assume anything but that. But also as you pointed out it looks like void is not correctly handled in this case when the chaining is off a nullish object. |
Uh oh!
There was an error while loading. Please reload this page.
Before You File a Bug Report Please Confirm You Have Done The Following...
Issue Description
Given the following code, I would expect it to lint successfully:
In my reproduction the second optional chain is removed if I run with
--fixbut then TypeScript errors with the following:I also want to note that this behavior does not occur if the value could be
nullonlyvoid. It successfully lints if I change the return ofmaybetoT | null.In my reproduction I also tried to narrow it down as far as I could and found that these examples lint successfully:
Reproduction Repository Link
https://gist.github.com/lukekarrys/979ea0dd68b52937e9a257b4d2c9c293
Repro Steps
Versions
@typescript-eslint/eslint-plugin8.0.1TypeScript5.5.4ESLint9.8.0node22.4.0The text was updated successfully, but these errors were encountered: