function that returns union with void can be incorrectly narrowed

[DetachHead]

Bug Report

🔎 Search Terms

void narrow

🕗 Version & Regression Information

4.6.0-20211117

⏯ Playground Link

Playground link with relevant code

💻 Code

const a: (value: number) => void = (i) => i
const b: (value: number) => void | string = a

const v = b(1)
if (v) {
    v.toUpperCase() //runtime error: v.toUpperCase is not a function 
}

🙁 Actual behavior

no error

🙂 Expected behavior

value doesn't get narrowed. perhaps void should not be falsy because you can't be sure its value will be falsy at runtime

[MartinJohns]

Related: #42709

[RyanCavanaugh]

I think we should do some patchwork and just disallow writing void | T in --strict. It's never what you want.

[KotlinIsland]

@RyanCavanaugh based

[mdbetancourt]

I think we should do some patchwork and just disallow writing void | T in --strict. It's never what you want.

i don't think so void !== undefined

function convert<T>(t: T): void | T { // ok
  if(typeof t === 'string') {
    return t
  }
}
function convert2<T>(t: T): undefined | T { // error here "A function whose declared type is neither 'void' nor 'any' must return a value"
  if(typeof t === 'string') {
    return t
  }
}

[RyanCavanaugh]

@mdbetancourt noImplicitReturns means no implicit returns. If you don't like the error in the second case, you should turn that option off.

[mdbetancourt]

what about this case extending his case

const a = (i: number[]) => i.forEach(console.log)

const b: (value?: number) => void | false = (v) => v ? a([v]) : false
//                             ^ undefined or anything else is not gonna work

[RyanCavanaugh]

unknown is equally correct there. We could change forEach to return undefined and then it can be undefined | false.

[mdbetancourt]

unknown is equally correct there.

@RyanCavanaugh unknown dont feel right, unknown means you don't know the value but you do also it absorbs the false type it's like an any

const a = (i: number[]) => i.forEach(console.log)

const b: (value?: number) => unknown | false = (v) => v ? a([v]) : false
const value = b() // unknown
if(typeof value === 'string') {
  value // type is "string" you think this is ok?
}

const b: (value?: number) => void | false = (v) => v ? a([v]) : false
const value = b() // void | false
if(typeof value === 'string') {
  value // type is "never" much better isn't?
}

We could change forEach to return undefined and then it can be undefined | false

but that could break some codes isn't? e.g:

function voidFn() {}

const log = [].forEach as () => undefined
const h: typeof log = voidFn
//    ^ Type 'void' is not assignable to type 'undefined'

a real case could be

function altLogger(text: string) {}

// if you change forEach signature you have to change every void signature as well, then
const log = console.log as (text: string) => undefined
const h: typeof log = altLogger
//    ^ Type 'void' is not assignable to type 'undefined'

[marcospgp]

what about this case extending his case

const a = (i: number[]) => i.forEach(console.log)

const b: (value?: number) => void | false = (v) => v ? a([v]) : false
//                             ^ undefined or anything else is not gonna work

I think this should error, and a function should not be able to return a union that includes void. Perhaps only in strict mode.

There already is "Not all code paths return a value. ts(7030)"

[robbiespeed]

Here's another example of the issue, this time without direct use of void | T:

function run (fn: (() => void) | (() => number)): number {
  const v = fn(); // results in void | number
  // void behaving like undefined
  if (typeof v === "object") {
    v; // never
    return -1;
  }
  if (v !== undefined) {
    return v; // TS thinks it's a number (but could really be anything)
  }

  return -1;
}

const foo = () => "hello";

const num: number = run(foo); // allowed because void allows any return value but ends up returning a string that thinks it's a number

typeof num === "string"; 

playground

I think we should do some patchwork and just disallow writing void | T in --strict. It's never what you want.

@RyanCavanaugh I don't think that will cut it. In the example above you don't need to write it, TS will infer it. I suppose you could disallow any union containing void somewhere no matter how deep, that seems complicated though and might be overly limiting. What about any time TS encounters void | T either explicitly or inferred it converts it to unknown?

[RyanCavanaugh]

I agree with @robbiespeed 's analysis. Really the only "good" fix here is #42709, so let's just track this there.