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

Type inference defaults to '{}' in call sites. #5254

Closed
drarmstr opened this issue Oct 14, 2015 · 28 comments
Closed

Type inference defaults to '{}' in call sites. #5254

drarmstr opened this issue Oct 14, 2015 · 28 comments
Labels
Discussion Issues which may not have code impact

Comments

@drarmstr
Copy link

I'm using TypeScript 1.6.3 in Visual Studio 2015. The es6-promise declaration I'm using contains:

declare class Promise<R> implements Thenable<R> {
    constructor(callback: (resolve : (value?: R | Thenable<R>) => void, reject: (error?: any) => void) => void);
    then<U>(onFulfilled?: (value: R) => U | Thenable<U>, onRejected?: (error: any) => U | Thenable<U>): Promise<U>;
}

The following works with an explicit typing of the Promise generic:

new Promise < { foo: number }>((resolve) => {
    resolve({ foo: 1 });
}).then((value) => {
    value.foo;
});

However, shouldn't type inference allow the following to work?

new Promise((resolve) => {
    resolve({ foo: 1 });
}).then((value) => {
    value.foo;
});

Instead, I am getting an error when accessing value.foo that foo does not exist on type '{}'. Why is TypeScript defaulting to the {} type?

@RyanCavanaugh
Copy link
Member

{} is the type that appears when there are no inference sources during generic type inference. We don't do any inference from call sites, which is the only place where the type { foo: number } appears in the second example.

@drarmstr
Copy link
Author

Well, not doing type inference from call sites will cause this. Is there a reason for that? Can this be a feature request?

@drarmstr drarmstr changed the title Type inference defaults to '{}' Type inference defaults to '{}' in call sites. Oct 14, 2015
@kitsonk
Copy link
Contributor

kitsonk commented Oct 14, 2015

You can though infer the type from the arguments passed the function constructor. The following should work:

new Promise((resolve: (value: { foo: number }) => void) => {
    resolve({ foo: 1 });
}).then((value) => {
    value.foo;
});

@kitsonk
Copy link
Contributor

kitsonk commented Oct 14, 2015

Which call sites? TypeScript doesn't know that the argument can only be called once. How would you type this?

new Promise((resolve) => {
    resolve({ foo: 1 });
    resolve({ bar: false });
}).then((value) => {
    value.foo;
});

@drarmstr
Copy link
Author

@kitsonk, You could specify the types in the call-site arguments, but that's just another workaround like specifying the types directly and we lose the benefit of type inference.

@drarmstr
Copy link
Author

@kitsonk, You could infer the type as {foo:number} | {bar:number}

@myitcv
Copy link

myitcv commented Oct 14, 2015

Related tslint proposal

@RyanCavanaugh
Copy link
Member

Having multiple inference sources for the same type parameter isn't a problem (it happens already, e.g. compare<T>(lhs: T, rhs: T): number).

One reason we haven't considered call expressions (or assignments, for that matter) as "valid" inference sites is that they rarely make sense from a runtime perspective. The Promise constructor can't plausibly inspect the implementation of the resolve callback to know whether it should pass a function accepting a string, a function accepting a number, or a function accepting some other thing. In this case it's more like a container that's completely agnostic to the type of its content, but just taking that literally and saying all Promises hold any isn't useful from a typing perspective.

@rbuckton probably has some input here.

@drarmstr
Copy link
Author

To help demonstrate the utility of the Promise example, please consider the similar situation where the types are properly inferred in the rest of the thenable chain:

var x = new Promise<{ foo: number }>((resolve) => {
    resolve({ foo: 1 });
}).then((value) => {
    value.foo;    // UNABLE TO INFER WITHOUT TYPE SPECIFICATION ABOVE
    return value;
}).then((value) => {
    value.foo;  // OK TO INFER as {foo:number}
    if (...) return value
    else return "hi";
}).then((value) => {  // OK TO INFER as {foo:number}|string
    (<{ foo: number }>value).foo;
    (<string>value).substr(2);
});

@DanielRosenwasser
Copy link
Member

Discussion should either continue here or on #5884.

@DanielRosenwasser
Copy link
Member

One reason we haven't considered call expressions (or assignments, for that matter) as "valid" inference sites is that they rarely make sense from a runtime perspective.

I know you said "rarely", but that's not the case with promises. It makes no sense immediately, but at the point that the promise resolves, there's clearly a runtime mechanism at work here which makes the promise hold a Foo.

This might be worth experimenting with.

@yazla
Copy link

yazla commented Mar 29, 2016

The annoying thing is that when typescript fails to infer a type and use {}, there are no errors whatsoever and it compiles just fine. So later during runtime you face issue.
It would be really useful to have it showing an error for such cases or providing a flag for a compiler which will make such occurrences an error. Are there any plans for this?

@DanielRosenwasser
Copy link
Member

I worked on a flag like that a while ago, but I think there was the idea that we wouldn't need it going forward. I'm not convinced that's really the case.

@yazla
Copy link

yazla commented Mar 30, 2016

Honestly, as for me the fact that you can have things silently screwed up for you contradicts the whole idea of type safety.

@DanielRosenwasser
Copy link
Member

The annoying thing is that when typescript fails to infer a type and use {}, there are no errors whatsoever and it compiles just fine. So later during runtime you face issue.

Can you elaborate on this? When did you get {} as an inferred type where you ran into a runtime bug?

@yazla
Copy link

yazla commented Apr 6, 2016

It was plenty of times, I will prepare an example and post here. Just need to find some time.

@kitsonk
Copy link
Contributor

kitsonk commented May 14, 2016

I don't think I have every encountered a runtime error where {} was inferred. I would expect errors if any were inferred because that can be dangerous (though, with noImplicitAny on, you guard yourself against that). Especially with strict object literals that came in 1.6, there is very little "dangerous" about this, in my opinion though it just is more of an annoyance, because is a flow of code, you have your types figured out for you, but then once in a while you try to do something where the types are "obvious" to you and you get something like "type {} does not have property 'foo'".

I think it is just more unintuitive and surprising, to such a point when I first saw this issue it was logical, why it was the way it was, only to have 7 months later felt totally annoyed that it was the way it was.

@s0
Copy link
Contributor

s0 commented May 18, 2016

@DanielRosenwasser @yazla @kitsonk:

Here is an example of a type error that occurs using Promises due to {} as an inferred type:

https://gist.github.com/samlanning/fd9da5d5811dc8f0156ba327df97c5aa

Personally, i'm very fond of the solution @DanielRosenwasser worked on in the branch downWithDreadedCurlyCurly. This problem has caused some runtime type errors we were hoping to avoid with our current setup.

@s0
Copy link
Contributor

s0 commented May 18, 2016

pinging @chrisgavinme (colleague working on this together)

@s0
Copy link
Contributor

s0 commented May 18, 2016

I've narrowed it down to this being the cause of our type errors:

let functionWithObjectParameter: (value: {}) => void;
let functionWithNumberParameter: (value: number) => void;

functionWithObjectParameter = (value: string) => {
  console.log('i really expect this to be a string:', value);
  console.log(value.substr(0));
};
// this assignment above should be considered invalid
functionWithNumberParameter = functionWithObjectParameter; // results in a function we assume should take a number, but actually takes a string.

let objectValue: {} = "string";
let numberValue: number = objectValue; // <- correctly does not type check

@s0
Copy link
Contributor

s0 commented May 18, 2016

Okay i've thought about it a little more, and here's a (hopefully easily digestible) explanation of why I now feel that {} is insufficient for when types cannot be inferred.

Suppose that you have an interface like so:

interface Foo<T> {
  get(): T;
  set(value: T): void;
}

When type inference fails, you may end up with a value that has this type:

let bar: Foo<{}> = //...

If you want to fetch the value, that's fine, you know nothing about that value, you can't do much with it, but that's fine:

let value = bar.get(); // <- the type of value is inferred to be {}

This next line will not type check, because we don't know anything about the value, this is perfect, this is what is designed to happen when type inference fails:

value.doSomething(); // <- this won't type check, perfect.

However the following does type check:

bar.set(1234);

this is bad, because we don't know what the type parameter of bar is, bar could have a type parameter of string, and then do something assuming the value we passed to set() is a string. So now we get a runtime error.

TL;DR

Inferring the types of function parameters to be {} leaves us open to runtime errors, because really what we're trying to assert here is that any values we pass to this function implement ALL INTERFACES so that whatever that function is trying to do is valid, and won't cause a runtime error due to inability to infer types.

@pimterry
Copy link
Contributor

An ultra simple case where this can easily bite you at runtime, via Twitter:

function foo(): Promise<boolean> {
  return new Promise(res => res(123));
}

This compiles happily with es6-promise.d.ts (inferring {}), but is about to throw a number somewhere that somebody will definitely be expecting a boolean (or potentially any other type - the function return type's generic parameter is basically just ignored).

Changing to the below to move the generic to the call site catches this correctly:

function foo() {
  // Fails: number is not assignable to boolean|Thenable<boolean>
  return new Promise<boolean>(res => res(123));
}

@DanielRosenwasser
Copy link
Member

@pimterry thanks, I was about to link to @tomdale to this issue.

@ikokostya
Copy link
Contributor

ikokostya commented Jan 25, 2017

Any news at this issue? flow (0.38.0) correct founds error for the code above:

// @flow
function foo(): Promise<boolean> {
  return new Promise(res => res(123));
}
test.js:3
  3:   return new Promise(res => res(123));
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructor call
  3:   return new Promise(res => res(123));
                                     ^^^ number. This type is incompatible with
  2: function foo(): Promise<boolean> {
                             ^^^^^^^ boolean


Found 1 error

even without generic type specification:

// @flow
function foo(): Promise<*> {
  return new Promise(res => res(123));
}

foo().then((x) => x.length);
test.js:6
  6: foo().then((x) => x.length);
                         ^^^^^^ property `length`. Property not found in
  6: foo().then((x) => x.length);
                       ^ Number


Found 1 error

Have you any plans to start do type inference in call sites?

@masaeedu
Copy link
Contributor

@ikokostya I believe this is fixed by #16072. In TypeScript 2.4.0-dev.20170609, the following snippet:

function foo(): Promise<boolean> {
  return new Promise(res => res(123));
}

Results in the error: [ts] Argument of type '123' is not assignable to parameter of type 'boolean | PromiseLike<boolean> | undefined'.

BarneyStratford added a commit to BarneyStratford/online-go.com that referenced this issue Jul 13, 2017
…e type Promise<string> itself and needs a little help. See the following for more discussion:

microsoft/TypeScript#5884
microsoft/TypeScript#5254
@zeluisping
Copy link

zeluisping commented Nov 12, 2018

I had this issue bite me when infering a type from an object field that could not exist (optional field); I was expecting to get an infered type of undefined but to my surprise got the empty object {}.

If you're using this in a conditonal type you can catch that default {} like this:

type CoalesceInfer<T, D> = keyof T extends never ? D : T;

This works because keyof {} === never

So in my case:

type Something<T> = T extends { params?: infer P, query?: infer Q }
    ? {
        params: keyof P extends never ? undefined : ProcessedType<P>
        query: keyof Q extends never ? undefined : ProcessedType<Q>
    }
    : never

or simplified:

type SafeProcessedType<T> = keyof T extends never ? undefined : ProcessedType<T>;

type Something<T> = T extends { params?: infer P, query?: infer Q }
    ? {
        params: SafeProcessedType<P>,
        query: SafeProcessedType<Q>
    }
    : never

@vkrol
Copy link

vkrol commented May 31, 2019

Can it be closed now?
https://github.com/microsoft/TypeScript/wiki/Breaking-Changes#generic-type-parameters-are-implicitly-constrained-to-unknown
#30637

@zeluisping
Copy link

Yeah, seems fine by me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests