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 not working with void type #35850

Open
overshom opened this issue Dec 25, 2019 · 22 comments
Open

Optional chaining not working with void type #35850

overshom opened this issue Dec 25, 2019 · 22 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@overshom
Copy link

overshom commented Dec 25, 2019

Demo

playground demo

TypeScript source with nightly version 3.8.0-dev.20191224

type Address = {
    city: string
}

type User = {
    home: Address | undefined
    work: Address | void
}

declare let u: User

u.home?.city
u.work?.city

JavaScript output the same for both "work" and "home" properties

"use strict";
var _a, _b;
(_a = u.work) === null || _a === void 0 ? void 0 : _a.city;
(_b = u.home) === null || _b === void 0 ? void 0 : _b.city;

Bug

If you run the code above, you will see that for u.home?.city there are no errors.
But for u.work?.city there is an error:

Error: Property 'city' does not exist on type 'void | Address'

Expectation

Optional chaining works for both 'undefined' and 'void' types the same way.

@MartinJohns
Copy link
Contributor

Please follow the issue template for bugs. Here you would have to fill out terms you searched for to find existing issues.

Duplicate of #35236.

@DanielRosenwasser
Copy link
Member

void is meant to be treated as an opaque return type for functions, since the type system allows you to assign () => number to a () => void.

In other words, it signals not just "this function doesn't return anything", but also "this functions values are not meant to be witnessed".

@overshom
Copy link
Author

I believe my issue not about void itself. We could leave void definition the same as now.
It's about optional chaining doesn't skip empty types in union type.

Not working, but expected to work:

type Address = {
    city: string
}

declare let value: Address | void | null | undefined | never

value?.city

Working good:

declare let value: Address | null | undefined | never

value?.city

Why void breaks optional chaining - not clear for me still.
What stops us from adding support for optional chaining with void in union type - not clear.

@MartinJohns
Copy link
Contributor

I believe my issue not about void itself.

Your issue is about your type being potentially void. Please check the other issue I linked you, the one you duplicated. Ryan provided reasoning there.

@overshom
Copy link
Author

ok, I will try to find out my root issue.

Originally, my type was Address | undefined.
For some unknown yet reason undefined type getting lost through layers of generics and type inference.
Therefore, in output I've got just Address instead of Address | undefined.

Once I replaced undefined with void my output got much stronger and void not lost.
So, output became expected Address | void.

And then, I found that I cannot use optional chaining with void as undefined.

@Raiondesu
Copy link

Raiondesu commented Dec 27, 2019

Actually, this is a big issue for functions with optional contexts.

Currently, the only way to make a function that accepts an optional context is to define it as
this: SomeType | void, since this?: SomeType doesn't work.

Due to this issue, it's currently impossible to use optional chaining inside the function to access properties of such context:

type Foo = { bar?: { baz: string; }; };

function example(this: Foo | void, value: string) {
  return this?.bar?.baz ?? value; // Currently an error here
}

example('default string');

One could argue that this is solvable by using undefined instead of void, but this forces all invocations to provide the context of undefined:

type Foo = { bar?: { baz: string; }; };

function example(this: Foo | undefined, value: string) {
  return this?.bar?.baz ?? value; // No error, cool
}

example('default string'); // "The 'this' context of type 'void' is not assignable to method's 'this' of type 'Foo | undefined'." 😕

So, currently, optional chaining is broken completely for functions with optional contexts. See more examples in this playground.


I, personally, have many uses for functions like this in my libraries and projects. And I believe other people have use-cases too.

@johan-gorter
Copy link

I think this is a simple example showing that unions with void are actually helping prevent runtime problems:

function simpleCallback() { }

async function advancedCallback() {
  await Promise.resolve();
}

// registerCallback can be called using simpleCallback or advancedCallback for example
function registerCallback(callback: () => Promise<any> | void) {
  // Does not compile unfortunately:
  // callback()?.catch(console.error);
  
  // workaround without optional chaining
  let result = callback();
  if (result) {
    result.catch(console.error)
  }
}

@MartinJohns
Copy link
Contributor

@johan-gorter I think you should rather show why void should be part of the union type, and not undefined.

void in TypeScript means the absence of a type, not the absence of a value. There might be a value, but it shouldn't be used. If a value is present your check might even produce false-positive.

http://www.typescriptlang.org/docs/handbook/basic-types.html#void

void is a little like the opposite of any: the absence of having any type at all.

https://github.com/microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void

Another way to think of this is that a void-returning callback type says "I'm not going to look at your return value, if one exists".

See also this comment by @RyanCavanaugh:

the problem with assuming a void-returning function is going to return a falsy value is that this code is legal:

var x: () => void = () => true;
if (x() || somethingImportant()) {
    // ...
}

@Raiondesu
Copy link

@MartinJohns, there's no arguing that void shouldn't be a part of a union type that defines a value.

But there is a case I mentioned earlier in which there's no way for void not to be a part of such a type, as it just breaks typescript altogether.

In @johan-gorter's example, the void type can be easily replaced with undefined - voilà, the problem is solved, optional chaining works:

function simpleCallback(): undefined { return; }

async function advancedCallback() {
  await Promise.resolve();
}

// registerCallback can be called using simpleCallback or advancedCallback for example
function registerCallback(callback: () => Promise<any> | undefined) {
  // EZ
  callback()?.catch(console.error);
}

However, in the case of void being the part of a union type for a function context (this) - it's impossible to just remove the void or replace it with undefined.

@MartinJohns
Copy link
Contributor

@Raiondesu You definitely brought up a valid case for void as part of the union type, but I think optional chaining support for void is the wrong approach to solve this (for the issues mentioned already).

void doesn't mean there is no context, it means there's no type information for the context - but it might very well be present and truthy and screw over optional chaining. So even if they'd allow ?. on union types involving void, they couldn't narrow the type and would have to fall back on any (or add another unsoundness to the type system).

To slightly adjust your example:

// `bar` is not optional anymore.
type Foo = { bar: { baz: string; }; };

function example(this: Foo | void, value: string) {
  // Only use optional chaining for `this`:
  return this?.bar.baz ?? value;
}

this might be truthy, but not match the Foo type, and accessing bar would cause a runtime error.


To solve your issue it would be better to allow to omit the context argument when the type is undefined.

type Foo = { bar?: { baz: string; }; };
function example(this: Foo | undefined, value: string) { return this?.bar?.baz ?? value; }

// Make this legal!
example('default string');

Or alternatively accept that the caller has to pass along undefined.

@Raiondesu
Copy link

Raiondesu commented Jan 3, 2020

I now clearly see the problem with this. I do agree with you.

But I still don't see how the context could just "suddenly" be different from the expected one (assuming we run code in 'use strict' mode).
I mean, unless the user explicitly sets the context using call, bind or apply, the context is undefined by-default. This means that TypeScript would intervene whenever the user changes context to the one that doesn't match the type.

And, also, if (this) still works, so this doesn't really add up with the "this might be truthy" argument. Why is optional chaining outlawed in this case, but a simple if type-guard is not?

type Foo = { bar?: { baz: string; }; };

function example(this: Foo | void, value: string) {
  if (!this) {
    return value;
  }

  // TS doesn't complain here!
  return this.bar?.baz;
}

example('default string');

If what you're implying is indeed true, and void represents that "no type information" is given - then we should've also outlawed if (this) here along with optional chaining on this (which would break the backward compatibility). Or... we could just allow optional chaining, as it is just a shortcut for if (this), is it not?


To solve your issue it would be better to allow to omit the context argument when the type is undefined.

Should I create a separate issue then?

@RyanCavanaugh RyanCavanaugh added the Unactionable There isn't something we can do with this issue label Jan 14, 2020
@RyanCavanaugh
Copy link
Member

This is not an intended use of void and you should never, ever on purpose write something like Foo | void in an input position. Any time you're tempted to write void like that, use unknown, never, or undefined depending on what you mean.

@Raiondesu
Copy link

Raiondesu commented Jan 15, 2020

I get it, @RyanCavanaugh, thanks, but how to deal with the function's context then?
It can't be never, undefined nor unknown. All three break the valid JS code of just calling the function afterwards without specifying the context.
Here are the examples in playground.

All examples listed are broken in some way or another. Even though two of them (void and undefined) are absolutely correct for the circumstance (in my opinion), TS doesn't allow any. Doesn't this mean that at least one of those types is broken in some way?

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed Unactionable There isn't something we can do with this issue labels Jan 15, 2020
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 15, 2020
@RyanCavanaugh
Copy link
Member

So I believe this one in particular should work without error as long as the code is in strict mode:

function exampleUndefined(this: Foo | undefined, value: string) {
  console.log(this);
  // Works here
  return this?.bar?.baz;
}

// Wrongly complains here!
// Valid JS code, though.
exampleUndefined('default string'); // logs 'undefined'

@ne-spal
Copy link

ne-spal commented Jan 24, 2020

@RyanCavanaugh but one more advantage of using void is it cannot be dropped by !

@bschlenk
Copy link

bschlenk commented Oct 1, 2020

Not sure if this is the right issue to leave this, but I think inference of void is a problem, as a function that does not return anything is inferred as () => void, and cannot be assigned to () => undefined, even though in this case TypeScript should know that my function actually returns undefined. See this playground for an example.

I'm also not sure why || works in place of ?? when void is involved.

@bschlenk
Copy link

bschlenk commented Oct 1, 2020

I understand why both exist. What I don't get is why typescript would ever infer a function to return void. If void means that a function could return anything, but we won't use the value, then typescript should never infer void - how does typescript know we won't be using the return value? It seems much more sensible for a function like () => {} to infer as () => undefined, since typescript knows in this case that the return value is undefined, not "anything".

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Oct 2, 2020

So let's say you write something like this

class Base {
  func() {  }
}
class Derived extends Base {
  func() {
    return 0;
  }
}

Is this an illegal class hierarchy because callers of Base#func are guaranteed to get back undefined ?

It doesn't seem like it is. It seems like Base didn't say anything about the return type of func, because it didn't, and Derived is acting in a legally covariant fashion by returning "more" than its base type.

Moreover, it seems nearly pointless to talk about a function whose return value is contractually obligated to be a single particular primitive value. How many functions do you call that are specified to return 5 where you consume that value? Or false ? Or anything else? I've never read docs where they say "Accepts a function; this function must return true". Why would anyone do that? The idea of defaulting to that mode when describing a function this way seems counter to usefulness.

@HanabishiRecca
Copy link

I have faced this in such situation:

const p = await new Promise<{ id: number; }>((resolve) => resolve({ id: 1 })).catch(console.warn);
console.log(p?.id);

Property 'id' does not exist on type 'void | { id: number; }'.

Playground

I'm not using void anywhere in my code, but Promise.catch itself returns void | T which is very annoying.

@mAAdhaTTah
Copy link

@HanabishiRecca The problem isn't catch but console.warn, which returns void.

@HanabishiRecca
Copy link

@mAAdhaTTah yeah, it can be fixed by something like:

.catch((e) => { console.warn(e); return undefined; })

But this is stupid and not resolves the problem.

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

No branches or pull requests

10 participants