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

Optional chaining type narrowing of void #35236

Closed
sharno opened this issue Nov 20, 2019 · 12 comments
Closed

Optional chaining type narrowing of void #35236

sharno opened this issue Nov 20, 2019 · 12 comments

Comments

@sharno
Copy link

sharno commented Nov 20, 2019

TypeScript Version: 3.7.2

Search Terms:
Type narrowing void
Optional Chaining void

Code

type A = { b: string };

// example of how I try to use it:
async (p: Promise<A>) => {
  const a = await p.catch(console.error);
  a?.b; // error Property 'b' does not exist on type 'void | A'.
};

// simplified examples:
(a: A | void) => {
  a?.b; // error Property 'b' does not exist on type 'void | A'.
  a ? a.b : undefined // works
};

(a: A | undefined) => {
  a?.b; // works
};

(a: A | null) => {
  a?.b; // works
};

Expected behavior:
No errors when using optional chaining with a T | void if T contains the fields specified. I guess void could be represented as a value as undefined considering that a function that returns undefined returns type void.

Not sure if this is feasible

Actual behavior:
compilation error

Playground Link: https://www.typescriptlang.org/play/index.html?ssl=10&ssc=22&pln=10&pc=13#code/C4TwDgpgBAglC8UDeUBGAuKBnYAnAlgHYDmUAvgNwBQVAhliIQMZQAUYmACrgPYC2+LBAA8MAHwBKBGORUoUJj0I4otBKoDutfMChgAdE1rAmAC1aLlPADYR9EXL1wTq82gH59qapRqtamHAAPlAAbjz4ACZS8DJIcqqe3glq7qpeUJgAroSREABmRBCRVL5U-oFQITl5hYTFMXEpST7U5QGwVVCEWdbWjbJuLaXUQA

@nmain
Copy link

nmain commented Nov 20, 2019

undefined and void are not synonymous. void represents that the return of a function could be anything and should not be used for anything, and so has consequences like https://github.com/microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void

So your request can be broken with something like this:

let a: () => void;

let b = () => ({ a: 5 })

a = b;

let c = true;

let d = c ? a() : { a: "5" };

d; // void | { a: string }
console.log(typeof d?.a); // "number"

@sharno
Copy link
Author

sharno commented Nov 20, 2019

@nmain Makes sense, thanks for the clarification

@sharno sharno closed this as completed Nov 20, 2019
@sharno
Copy link
Author

sharno commented Nov 20, 2019

Actually another question if according to #26262, void shouldn't allow checking for truthiness
Why is it allowed here when it's a discriminated union with another type?

(a: A | void) => {
  a ? a.b : undefined // works
};

Do you think I should file another bug for this?

@sharno sharno reopened this Nov 20, 2019
@nmain
Copy link

nmain commented Nov 20, 2019

Good question. I'm guessing that was intentional, but it does seem inconsistent; T | void can be tested for truthiness, but not safe-navigated. 🤷‍♂

@RyanCavanaugh
Copy link
Member

Testing void for truthiness is a one-off rule. There's no legitimate reason to write (a: A | void) => so whatever behavior that produces is immaterial

@sharno
Copy link
Author

sharno commented Nov 22, 2019

@RyanCavanaugh
There's a legitimate reason I have in my code to write something like this:

async (p: Promise<A>) => {
  const a = await p.catch(console.error);
  // do stuff with a
};

Apparently a will have type A | void. Passing any function that returns void to catch will yield the same type which might cause unwanted behaviors if checked for truthiness and the function returns something other than undefined.

I know this issue doesn't track this and it's good to close it. But what do you think about such a use case?

@Cellule
Copy link

Cellule commented Nov 27, 2019

There are also a lot of confusion between void and undefined in general.
For instance in react-apollo the result of a mutation was typed with void | {data: Data}.

const response = mutation();
if (response) {
  // This is fine because if(void|{data: Data}) removed void from union
  const data = response.data
}
// This is not fine because ?. won't remove void thus giving error that "data" doesn't exist on void
const data = response?.data;

While I agree that void and undefined are different, in practice there are used interchangeably.

If you stick to the argument that void | A is not legitimate to do, why are we allowed to write it?
Either void is special and can't be union'ed with another type or it behaves like undefined and should be removed from the union when using ?.

@RyanCavanaugh
Copy link
Member

The category of "things which you should not do because they are ultimately incoherent" is unbounded; it's similarly useless to write 0 * ((x + a + b) * 1 + 3) but not a categorical error. void has a meaning and | has a meaning; if you misapprehend those meanings and combine them you'll get weird results.

Banning of certain combinations of type operators and type operands has been discussed extensively before; in a language with structural generics it's ultimately a fool's errand because you can't prevent generic instantiations that end up creating "illegal" types.

@Cellule
Copy link

Cellule commented Nov 27, 2019

You make a very good point why void | A should not be blocked and I totally agree with you there.
My main question is why was it determined that if (void | A) will produce A in the if block and can we apply the same logic for ?.?

@RyanCavanaugh
Copy link
Member

We're actually thinking that the if block shouldn't narrow, since some other truthy value might inhabit response. If mutation is guaranteed to return undefined when it doesn't produce the prescribed value, then it should be written as such rather than using void

@RyanCavanaugh
Copy link
Member

Oh, and that console.error and its friends should have their return types changed from void to undefined since that behavior is specified

@Cellule
Copy link

Cellule commented Nov 27, 2019

The biggest problem with making such a change is all the libraries incorrectly using void instead of undefined would become broken. Upgrading is not always easy or possible in the short time for various reasons.
We'd be stuck having to do something like

type ReplaceVoid<T> = T extends void ? Exclude<T, void> | undefined : T;
function removeVoid<T>(a: T): ReplaceVoid<T> {
    return a as any;
}

type A = void | { data: any };
function foo(a: A) {
    // for if block
    const _a = removeVoid(a);
    if (_a) {
        _a.data
    }
    // using && operator
    _a && _a.data;
    // using optional chaining
    removeVoid(a)?.data;
}

Maybe there is an easier way to convert void to undefined, but we'd definitely need it if Typescript is going in that direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants