Skip to content

Return type incorrectly inferred as void #57840

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

Open
fabiospampinato opened this issue Mar 19, 2024 · 5 comments Β· May be fixed by #57912
Open

Return type incorrectly inferred as void #57840

fabiospampinato opened this issue Mar 19, 2024 · 5 comments Β· May be fixed by #57912
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@fabiospampinato
Copy link

fabiospampinato commented Mar 19, 2024

πŸ”Ž Search Terms

  • return type, void, infer, undefined

πŸ•— Version & Regression Information

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

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.5.0-dev.20240318#code/FAFwngDgpgBAYgORgXhgCgJQoHwwAoBOA9gLYCWAzlADwCuAdgCZQBmZ9UjuAPjA82w6MA3MGABjIvQogYLegEYAXPCSpMOGAG9gMPTAJQQtAvVEBfURKky59AEwrEKdFmS4d+g0ZP0+TVnZOC1EgA

πŸ’» Code

type FN = () => Promise<undefined> | undefined;

const fn1: FN = () => {
    return;
};

const fn2: FN = () => {
    return undefined;
};

πŸ™ Actual behavior

fn1 is detected as returning void, while fn2 is detected as returning undefined.

πŸ™‚ Expected behavior

I'm not sure it makes sense to me that the inferred type for these kinds of function is void rather than undefined, I would expect the more precise type to be inferred in this case.

Mainly there's no difference in JS between doing a return; and a return undefined;, unless .toString() is called on the function containing that, so TS differentiating between return; and return undefined; seems a bit weird to me.

Presumably this is something that has been considered and returning void is done on purpose and it's actually better on average, and requiring an explicit return undefined; makes sense, but I'd like to point out that there are some detectable cases where one is not return-ing to skip the following code, but one is explicitly returning undefined for no other reason than to be explicit about it, for example in this case, where the return statement is the last statement of the function, and it's not even inside an else branch. Like there is no other purpose to that return than to mean, in every way, what the longer return undefined; expresses, so perhaps the logic that decides whether void or undefined should be the returned type could be refined to account for this specific scenario.

Also if I'm passing a function like that as an argument that accepts () => undefined I feel like that should be taken into consideration, and the function in that case should "obviously" be considered as returning undefined rather than void.

Additional information about the issue

I stumbled on this issue because at one point I'm accepting a function that has the following interface: () => (Promise<undefined> | undefined), the idea is that if the function returns a promise I'll call its then function, and if it doesn't I'll call the then function on a fallback value, so I'll just end up calling something called then either way. Example code:

const FAKE_PROMISE = {
  then: fn => fn ()
};

const return = myFn() || FAKE_PROMISE;
 
return.then( () => {/* ... */} );
@fabiospampinato
Copy link
Author

Interestingly if the function is async now Promise<undefined> is always the return type, even if I omit the return statement entirely, which seems pretty inconsistent.

https://www.typescriptlang.org/play?ts=5.5.0-dev.20240318#code/FAFwngDgpgBAYgORgXhgCgJQoHwwAoBOA9gLYCWAzlADwCuAdgCZQBmZ9UjuAPjA82w6MA3MGABjIvQogYLegEYAXPCSoAhhTD1x6LMlwBvYDFMwCUELQL1RAX1ESpMufQBMKxChibtuzDgwxmbmltb0fEys7Jz2okA

@Andarist
Copy link
Contributor

It's related to this 5.1 change. The linked PR mentions that:

Functions with an explicitly specified return type undefined aren't required to have return statements (similar to functions with an explicitly specified return type of void or any). Note that this does not apply to functions with union return types that include undefined.

I'm not sure what's the exact reason behind this but I find it confusing and inconsistent too. I get it that it might be easy to forget a return value in such cases. It feels like a potential lint rule for people who prefer explicit return undefined. It's surprising that the typechecker complains here. At the very least the error message could mention how to fix this problem because at the moment it's not obvious what to do about it.

This is a reason why people write weird things when they actually want to allow those implicit undefined when the conceptual return type is meant to be a union that includes undefined (taken from @types/react):

declare const UNDEFINED_VOID_ONLY: unique symbol;
type Destructor = () => void | { [UNDEFINED_VOID_ONLY]: never };
type EffectCallback = () => void | Destructor;

Whereas if undefined could be implied when dealing with unions this would be much simpler:

type Destructor = () => void;

function useEffect(cb: () => undefined | Destructor) {}

useEffect(() => {
  // it would be perfect if this could be implied
  return undefined;
});

// errors as expected
useEffect(() => {
  return 10;
});

useEffect(() => {
  // it works just fine
  return () => {};
});

useEffect(() => {
  return () => {
    // we don't care about the return type here so it's allowed
    return "foo";
  };
});

@RyanCavanaugh
Copy link
Member

This is just a bug. We have special logic in contextual typing to ensure that a function that would normally be inferred as void return undefined instead if needed:

type FN = () => undefined;
// Not an error
const fn1: FN = () => {
    
};

Something with the union with Promise is messing it up, probably because we also have logic to make the Promise case in specific also work:

type FN = () => Promise<undefined>;
// Also not an error
const fn1: FN = async () => {
    
};

Unions with other values don't seem to have this problem

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Mar 19, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 19, 2024
@Andarist
Copy link
Contributor

Andarist commented Mar 20, 2024

Unions with other values don't seem to have this problem

Maybe we are talking about some other behaviors. I think the OP is reporting the same problem as the one showcased here:

type FN = () => string | undefined;

// Type '() => void' is not assignable to type 'FN'.
//   Type 'void' is not assignable to type 'string | undefined'.(2322)
const fn1: FN = () => {
  return;
};

// ok
const fn2: FN = () => {
  return undefined;
};

// Not all code paths return a value.(7030)
const fn3: FN = () => {
  if (Math.random()) {
    return "";
  }
};

The code that "converts" the inferred void to undefined is here and it doesn't use someType (that would handle unions) to check if contextualReturnType contains an undefined type.

Unless you are saying that both of those should be errors:

type FN = () => Promise<undefined> | undefined;

// Type '() => void' is not assignable to type 'FN'.
//   Type 'void' is not assignable to type 'Promise<undefined> | undefined'.(2322)
const fn1: FN = () => {
  return;
};

// ok
const fn2: FN = async () => {};

IIUC, this is not what OP's post is about though so the added "Bug"/"Help Wanted" labels make it confusing to me.

@RyanCavanaugh
Copy link
Member

What I mean is that if the contextual type itself i a union with other types, that doesn't spoil the pot:

type FN = 42 | (() => undefined);
// Not an error
const fn1: FN = () => {
    
};

IIUC, this is not what OP's post is about though so the added "Bug"/"Help Wanted" labels make it confusing to me.

To clarify, this code should not have an error:

type FN = () => Promise<undefined> | undefined;

const fn1: FN = () => {
    return;
};

Seems like using someType in that linked code might be the right fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants