Typescript: Don't widen return types of function expressions

[RyanCavanaugh]

This change courtesy @JsonFreeman who is trying it out

(context elided: widening of function expression return types)

The problem with this widening is observable in type argument inference and no implicit any. For type argument inference, we are prone to infer an any, where we should not:

function f<T>(x: T, y: T): T { }
f(1, null); // returns number
f(() => 1, () => null); // returns () => any, but should return () => number

So after we get to parity, I propose we do the following. We do not widen function expressions. A function is simply given a function type. However, a function declaration (and a named function expression) introduces a name whose type is the widened form of the type of the function. Very simple to explain and simple to implement.

I’ve been told that this would be a breaking change, because types that used to be any are now more specific types. But here are some reasons why it would be okay:

• In the places where you actually need the type to be any (because there are no other inference candidates), you would still get any as a result
• In places where there was a better (more specific) type to infer, you’d get the better type.
• With the noImplicitAny flag, you’d get fewer errors because there are actually fewer implicit anys

Questions:

Is a principle of design changes going forward to not switch from 'any' to a more precise type because it can be a breaking change?

Going with 'not a breaking change' here because this is unlikely to break working code, but we need to verify this.

Would this manufacture two types?

In essence, we already have two types: The original and the widened type. So by that measure this is not really a change

Has someone tried it?

Jason willing to try it out and report back

[DanielRosenwasser]

Bumping this since we ran into a situation with a leaked any in our own compiler. Minimal example:

function forEach<T, U>(array: T[], callback: (element: T, index: number) => U): U {
    if (array) {
        for (let i = 0, len = array.length; i < len; i++) {
            let result = callback(array[i], i);
            if (result) {
                return result;
            }
        }
    }
    return undefined;
}


forEach([1, 2, 3, 4], () => { 
    // do stuff
    if (blah) {
        // bam! accidental any
        return undefined;
    }
});

[mhegazy]

We need an implementation proposal for this one.

[JsonFreeman]

What if we just did this:

• Don't widen when you type a function body
• When you widen a type, and it's a function type, you widen the return type
• When you resolveName and it's a function expression or function declaration, you widen its type

[myitcv]

I've arrived here via #7220. Would appreciate some guidance on whether the following is expected behaviour or not (playground):

interface G<K, R> {
    (v: K): R;
}

function F<T>(v: T): T {
    return v;
}

interface A {
    __Type: "A";
}

interface B {
    __Type: "B";
}

let a: A;
let b: B;

a = b; // OK: error as expected, B is not assignable to A
b = a; // OK: error as expected, A is not assignable to B

let x: G<A, B> = F;  // How is this possible?

It's the final assignment I cannot understand... because the compiler does not complain.

[RyanCavanaugh]

During assignment checking, type parameters are replaced with any. So the effective signature of F here is (a: any) => any

[JsonFreeman]

@myitcv See #138

[RyanCavanaugh]

Time to take this back up since it's affecting a lot of people writing reducers

[pelotom]

Any updated status on this?

[aczekajski]

bump

[grantila]

I came here by Daniel's reference from #20859. I'm having trouble writing typesafe libraries where users can provide callbacks which should follow the type contract of the library function.

And, as shown in my examples in #20859, this affects not only generic funtions (as depicted here) but also non-generic strictly typed functions. Basically, the return of a callback function is type unsafe, always, since the callback is a variable, and any assigning of a function to a variable (of a function type) ignores the return value of the r-value.

A lot of safety is missed by the current behavior.

[JasonGore]

This is another one I've been having to work around. I've added helper types so that users of Fabric don't have to duplicate entire function signatures for type safety, like so:

const circularTokens: IButtonComponent['tokens'] = (props, theme) => { };

However, this loses some (EDIT: not all) type safety on the return type, which is really important for having us make sure Fabric users are doing the right thing in their implementations. So for now I've had to make helper types:

export type IButtonTokenReturnType = ReturnType<Extract<IButtonComponent['tokens'], Function>>;

Which requires users to do this to get full type safety:

const circularTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenReturnType => { };

Which is a bit frictiony and error prone. I'd love to see this issue fixed!

[weswigham]

I assume by "safety" you mean excess property checking?

[JasonGore]

I guess the way I'm looking at it is I want the same level of type safety for the return type, whether it's explicitly specified or applied via functional type as shown here:

// Error based on return type:
const function1 = (input: number):  { testKey?: number } => { return { testKey: 42, invalidKey: 42 } }

// Same return type via functional signature but no error:
type TestFunction = (input: number) => { testKey?: number };
const function2: TestFunction = (input) => { return { testKey: 42, invalidKey: 42 } }

[JipingWang]

Is MUI team right saying the following is a TypeScript widen issue? I think TS is just right (see my test here).

Type '{ display: string; flexDirection: string; }' is not assignable to type 'CSSProperties'.

Types of property 'flexDirection' are incompatible.

Type 'string' is not assignable to type '"column" | "-moz-initial" | "inherit" | "initial" | "revert" | "unset" | "column-reverse" | "row" | "row-reverse" | undefined'.

[kalbert312]

Just ran into this trying to capture a subtype of an object using spread but no error is thrown on excess properties.

Example

interface Props extends ThirdPartyOptions {
    bar: "baz",
}
const foo = (props: Readonly<Props>): ThirdPartyOptions => {
    const { ...rest } = props;
    return rest;
};

EDIT: Found a workaround for my case:

function keysTyped<T>(obj: T): Array<keyof T> {
    return Object.keys(obj) as Array<keyof T>;
}
interface Props extends ThirdPartyOptions {
    bar: "baz",
}
const foo = (props: Readonly<Props>): ThirdPartyOptions => {
    const { ...rest } = props;
    const test: Array<keyof ThirdPartyOptions> = keysTyped(rest); // Error
    return rest;
};

[berwyn]

I filed #33908 based on behaviour a coworker found in NGRX where NGRX's ActionReducer<S, A> interface has a callable signature of (state: S | undefined, action: A) => S but the widening issue allows for subtle bugs as extra keys can be provided by the reducer that don't match the provided interface.

This may factor into whether this is a breaking change or not, given how popular NGRX is amongst the Angular community.

[Minho-Lee]

I filed #33908 based on behaviour a coworker found in NGRX where NGRX's ActionReducer<S, A> interface has a callable signature of (state: S | undefined, action: A) => S but the widening issue allows for subtle bugs as extra keys can be provided by the reducer that don't match the provided interface.

This may factor into whether this is a breaking change or not, given how popular NGRX is amongst the Angular community.

Also to add on, it would make it very difficult to catch typos on existing keys.

ie.

const initialState: SomeState = {
  someID: number
}

createReducer(
  initialState,
  on(someAction, (state, { id }) => {
    return {
      ...state,
      someId: id  // this typo wouldn't throw errors
    }
  }
)

[AnyhowStep]

"Fixing" this would be useful for my SQL query builder library.

tsql.from(myTable)
    .select(columns => [columns])
    .insertInto(
        connection,
        otherTable,
        columns => {
            return {
                otherTableColumn0 : columns.myTableColumn0,
                otherTableColumn1 : columns.myTableColumn1,
                //Should give a compile error because `otherTable`
                //does not have column `doesNotExist`
                doesNotExist : 32,
            };
        }
    );

If "exact types" were implemented, I could use that for this constraint, too

---

declare function foo(
  callback: () => {x:string}
): void

foo(() => {
  return {
    x: "hi",
    y: "bye", //should error
  };
})

Playground

Related,
https://github.com/microsoft/TypeScript/issues/12632

[crazyair]

export interface Demo {
    items?: () => { demo?: string };
}

export const dd: Demo = {
    items: (): ReturnType<Required<Demo>['items']> => ({ demo: '1', str: 'error' }),
};

Playground

[pronebird]

We've noticed the same issue after some of the reducers returned garbage. Big surprise.

function reducer(state: IState, action: IAction): IState {
  switch (action.type) {
    case 'FOOL_TYPESCRIPT':
      return {
        ...state,
        ...{ ANY_NON_EXISTING_FIELD_GETS_SWALLOWED_WITH_NO_ERROR: "BOOM" },
      };

    default:
      return state;
  }
}

[mortoray]

This problem affects any factory function that is intended to create a specific type. (From the form of https://github.com/microsoft/TypeScript/issues/12632 )

interface OnlyThis {
  field: number
}
type FactoryFunc = () => OnlyThis
function makeOnly(ctor: FactoryFunc) { }

const test = makeOnly(()=> ({
  field: 123,
  extra: 456, // expected to fail, but does not.
}))

There's no way to prevent makeOnly from accepting a function returning an object with excess fields. It will detect missing fields, but not excess fields. Thus the user of makeOnly will no know that their values are being ignored.

[dwelle]

@mortoray yes, this is my main issue with this. Currently, the only workaround is to explicitly type the arrow func, which kinda defeats the purpose:

const test = makeOnly((): OnlyThis => ({
  field: 123,
  extra: 456, // error
}))

[essenmitsosse]

Here is another minimal example of the problem:

const getFoo2: () => Foo =  {
    foo: {
        bar: number
    }
}  => ({
    foo: {
        bar: 1,
        buz: 2, // Doesnt throw an excessive property error
    }
})

The weird thing is, its not like the function is untyped. Changing the name of foo, or adding an excessive property to the main return object, works as expected. It only fails on the nested object.

[jasonkuhrt]

I believe my StackOverflow question relates to this issue: https://stackoverflow.com/questions/63258306/how-to-force-typescript-excess-property-check-on-function-return-value

[DanielRosenwasser]

Hey everyone, I opened #40311 a couple of days ago which I think should fix most of the issues that have brought people here. If you'd like to give it a try, we have a build available over at https://github.com/microsoft/TypeScript/pull/40311#issuecomment-683255340.