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

Excess properties are not checked e.g. in callbacks #7547

Closed
Ciantic opened this issue Mar 16, 2016 · 13 comments
Closed

Excess properties are not checked e.g. in callbacks #7547

Ciantic opened this issue Mar 16, 2016 · 13 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@Ciantic
Copy link

Ciantic commented Mar 16, 2016

This might be a feature, but it sure is odd. The excess property check is not working on callbacks, e.g.:

interface Test {
    name?: string;
}

function myMutator(fn: () => Test) {
    return fn();
}

/* Not an error */
let a: Test = myMutator(() => ({
    notAProperty : "woot?"
}));

/* This gives error */
function test(): Test {
    return {
        notAProperty : "what"
    }
}

/* This gives error */
let b: Test = {
    notAProperty : "huh"
};

Is there a trick to enforce the excess property check in situations like the one above?

Here is a link to playground.

@RyanCavanaugh
Copy link
Member

@JsonFreeman @ahejlsberg Function return type widening strikes again. Any ideas on a way to catch this?

@T18970237136
Copy link
Contributor

Hi,
sorry to ask a noobish question, but what is the reasoning behind not contextually typing the return type of function expressions as the exact type from the context (at least if it is not any or something like that)?

E.g. if I have

interface A {
    a: string;
    b: number
}

let f: () => A = () => { // Error here
    // do some stuff
    1 + 1 == 42;

    // return a value
    return {
        a: "test"
    };
};

then I get the error Type '() => { a: string; }' is not assignable to type '() => A'. on the line let f: () => A, whereas I would have expected an error at the return statement. Instead, I would have to write

let f: () => A = (): A => {
    // do some stuff
    1 + 1 == 42;

    // return a value
    return { // Error here
        a: "test"
    };
};

to get the error at the return statement (and I would also get errors on declaring extra properties).
This seems to also happen at #7538.

E.g. in the example from @Ciantic, if you write

let a: Test = myMutator((): Test => ({
    notAProperty : "woot?"
}));

then you would also get the extra-properties error.

Thanks!

@RyanCavanaugh
Copy link
Member

what is the reasoning behind not contextually typing the return type of function expressions as the exact type from the context

This does happen.

interface StringCallback {
    (s: string): void;
}

var x: () => StringCallback = () => {
    return s => s.length; // s: string
}

The problem is that extra property checking doesn't occur as a result of contextual typing, it happens during regular assignability when the object type is "fresh". When we widen the return type of the return expressions to produce the return type of the function, the freshness is lost and there's no checking of extra properties.

@DanielRosenwasser
Copy link
Member

#241 is the widening issue for the record.

@JsonFreeman
Copy link
Contributor

Oh boy. This does look like a familiar problem. I agree that it's worth addressing #241 broadly.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Mar 28, 2016
@RyanCavanaugh RyanCavanaugh added Too Complex An issue which adding support for may be too complex for the value it adds Design Limitation Constraints of the existing architecture prevent this from being fixed and removed In Discussion Not yet reached consensus labels May 9, 2016
@RyanCavanaugh
Copy link
Member

Ideally this would be an error. Unfortunately it turns out to be very difficult to fix this without possibly having consequences in terms of runaway recursion and/or performance (see also discussion notes at #8228).

We're still tracking the root cause (widening the types of function expressions return types) at #241. If more symptoms of this behavior become apparent we'll look at addressing this again, but as it stands it's too complex to fix this relative to the improvement in behavior (which can be worked around with a type annotation).

@ghost
Copy link

ghost commented Aug 24, 2017

This just bit me too.

interface HasX { x: number; }
const toX: () => HasX = () => ({ x: 0, y: 0 });

(Would also have been solved by #12936)

@Ciantic
Copy link
Author

Ciantic commented Aug 26, 2017

My original issue seems to error out these days, I tried opening my old code in playground. @andy-ms's example does not cause error.

So part of this was fixed?

@ghost
Copy link

ghost commented Aug 28, 2017

@Ciantic the error is that there is no compile error when there should be.

@Ciantic
Copy link
Author

Ciantic commented Aug 30, 2017

@andy-ms yes, I know, but my original comment contains a code that behaved similarly: It used to not show an error, but now that I tried the compiler gives an error. So partially this is fixed, my original issue seems to be not there anymore.

However you have discovered a different case from my original.

@ghost
Copy link

ghost commented Aug 30, 2017

The fix for that might have been #16047.

@jezzgoodwin
Copy link

Hi, I noticed that this issue was closed over a year ago due to complexity in creating a fix. I wondered if this is still the case?

In the codebase I'm working on, we're using generics to specify expected types returned from callbacks. At the moment, excess properties are allowed through (usually spelling mistakes), which causes us hassle from time to time. (Please ask for code examples if required)

@njgraf512
Copy link

Hi, any help on this issue would be appreciated. Am I correct that this issue causes the generic in TypeScript's type definitions for map, for example, to not enforce types correctly.

map<U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any): U[];

Looking at this, I would think that it would enforce that the return value is an array of type U; however, the compiler is perfectly happy to allow an array of type U elements with extra properties. Is there any plan to address this? Typing the return value of map's callback does work to enforce the types as shown below, but I have to think there should be some overload to handle this? Any help appreciated!

type V = {
  foo: string
}

arr.map<V>( el => {
  return {
    foo: string
    bar: 24 // No Error!
  }
}

arr.map( (el): V => {
  return {
    foo: string
    bar: 24 // Error!
  }
}

@microsoft microsoft locked and limited conversation to collaborators Jul 18, 2018
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Sep 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants