TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Allow non returning functions to have contextually required `undefined` return type

Open falsandtru opened this issue 6 years ago • 12 comments

Search Terms

#36239

Suggestion

Allow non returning functions to have contextually required undefined return type.

Use Cases

  • Callbacks

Examples

declare function f(a: () => undefined): void;
f(() => { }); // error -> ok
f((): undefined => { }); // error -> ok
const g1: () => undefined = () => { }; // error -> ok
const g2 = (): undefined => { }; // error -> ok if possible
function h1() {
}
f(h1); // error -> error
function h2(): undefined { // error -> ok if possible
}

Checklist

My suggestion meets these guidelines:

  • [x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [x] This wouldn't change the runtime behavior of existing JavaScript code
  • [x] This could be implemented without emitting different JS based on the types of the expressions
  • [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • [x] This feature would agree with the rest of TypeScript's Design Goals.

falsandtru avatar Jan 18 '20 17:01 falsandtru

I like this idea 👍

RyanCavanaugh avatar Jan 23 '20 17:01 RyanCavanaugh

This would be great.

I'm working with an API that's typed in closure compiler's ES4 type system as () => boolean|undefined and the TypeScript functions passed in are required to explicitly return undefined. If I omit the return, they're typed as void and generate the error Type 'void' is not assignable to type 'boolean | undefined'.(2322)

e.g.

type listener = () => boolean | undefined;
function listen(listener: listener) { }

listen(() => true);  // ok
listen(() => undefined); // ok
listen(() => { });  // error!

What needs to happen to allow the listener callback type to omit return statements? Any chance of this landing in a future release?

wffurr avatar Jul 06 '20 13:07 wffurr

I have a lot of code that has functions with return types of undefined or T|undefined.

I'd be happy to try to take this on myself, if a project member could give some pointers as to how such a thing could be implemented.

The idea is that any argument accepting a function parameter whose return type includes undefined could accept a non-returning function instead of requiring an explicit return undefined. Explicitly returning undefined is pretty weird when that's the default behavior of a non-returning function in JavaScript.

wffurr avatar Jul 28 '20 23:07 wffurr

Do note that an explicit return statement without any value causes TypeScript to allow the undefined return type annotation.

const ok = (): undefined => {
	return; // This explicit return statement is currently required
};

ExE-Boss avatar Sep 06 '20 08:09 ExE-Boss

Consider ReactJS's useEffect, the first argument must be a function that returns void | (() => void | undefined). The problem is the following is valid:

const f: () => void = () => 42;
useEffect(f);

IMO, an API which expects a function that returns void means it will not look at the return value, which isn't the case. In a sense, void as a return value is more like unknown than undefined.

But if the API had been correctly defined as declare function useEffect(f: () => undefined | (() => void)): void;, the following wouldn't typecheck with noImplicitReturns:

useEffect(() => {});

I believe accepting this suggestion would help defining better APIs.

ilogico avatar Sep 08 '20 16:09 ilogico

I'd like to vote against this -- or at least provide it as a tsconfig option (so that we're not forced to use it).

In fact, I would like to propose making returns more explicit than they are today. Currently the compile error goes away providing your method has at least one terminal branch with an explicit return... it would seem safer IMO if the error only goes away once all terminal branches have an explicit return:

I have many T | undefined in my code: if I forget to explicitly return a value from a terminal branch, there's only a 50% chance that the implicitly returned undefined is the correct value to return (meaning a 50% chance of a bug).

Consider bugs like this, which would be prevented if every terminal branch required a return:

function foo(): number | undefined {
  if (someFunction()) {
    return undefined
  }

  const value: number = someOtherFunction()
  anotherFunction(value)
  // return value <- The code I forgot to write! Today this compiles fine... but it shouldn't IMO.
}

So tbh rather than making the returns more implicit, it would be safer to make them more explicit.

(@falsandtru @RyanCavanaugh sorry for the "thumbs downs": your comments are great, I just wanted the maintainers to see at a glance that someone has an issue with the proposed change, as currently it looks unanimously in favour 😄)

ljwagerfield avatar May 26 '21 14:05 ljwagerfield

@ljwagerfield it sounds like you should turn on the noImplicitReturns flag, which does exactly that?

RyanCavanaugh avatar May 26 '21 22:05 RyanCavanaugh

@ljwagerfield it sounds like you should turn on the noImplicitReturns flag, which does exactly that?

Excellent, so it does!

In that case, assuming noImplicitReturns: true works the same after the proposed changes, then I'm happy to withdraw my reservation.

Thanks again @RyanCavanaugh !

ljwagerfield avatar May 27 '21 11:05 ljwagerfield

Very cool I like it this would have saved me two hours of bugfixing earlier 👍

borgesius avatar Feb 18 '22 18:02 borgesius

This would be a great feature to incorporate into TS!

bombillazo avatar May 19 '22 20:05 bombillazo

It sounds great. In this example, e and h are already typed when noImplicitReturns: false.

function a(): void { return }
function b(): void { if(0) return }
function c(): void {}

function d(): undefined { return }
function e(): undefined { if(0) return } // error when noImplicitReturns: true
function f(): undefined {}               // always error

function g(): number | undefined { return }
function h(): number | undefined { if(0) return } // error when noImplicitReturns: true
function i(): number | undefined {}               // always error

It would be more consistent to allow f and i under noImplicitReturns: false.

This change seems to have no drawbacks. What blocks approval of it?

akouryy avatar Jun 16 '22 09:06 akouryy

I'm all for this! This would allow typing event handlers as (ev: Event) => undefined | boolean instead of the weird (ev: Event) => any :D

Edit: The current implementation still doesn't allow this as it blocks unions with undefined :(

stevenwdv avatar Oct 18 '22 21:10 stevenwdv

Thank you @saschanaz!

DanielRosenwasser avatar Mar 17 '23 19:03 DanielRosenwasser