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

Function interface doesn't type check return value for extra keys #12632

Closed
carloslfu opened this issue Dec 3, 2016 · 9 comments
Closed

Function interface doesn't type check return value for extra keys #12632

carloslfu opened this issue Dec 3, 2016 · 9 comments
Labels
Duplicate An existing issue was already created Fix Available A PR has been opened for this issue

Comments

@carloslfu
Copy link

carloslfu commented Dec 3, 2016

TypeScript Version: 2.1.1 / nightly (2.2.0-dev.201xxxxx)

See next code, also in This playground.

Code

interface F {
    (p: {a:number}): {a: string}
}

// there are no type check for extra keys
// but F interface has return type
let g: F = ({a}) => ({
    a: '5',
    b: '',
})

// we need to do that!, but F interface also has return type
let f: F = ({a}): {a: string} => ({
    a: '5',
    b: '',
})

Expected behavior:

'g' function should be detected as an error, because return type of interface F doesn't have key 'b', just haves 'a' key

Actual behavior:

'g' function doesn't display any error

@DanielRosenwasser DanielRosenwasser added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Design Limitation Constraints of the existing architecture prevent this from being fixed labels Dec 3, 2016
@DanielRosenwasser
Copy link
Member

I'm not necessarily sure at which point in the process the object literal loses its freshness.

@carloslfu is there any reason you're trying to prohibit excess properties here?

@carloslfu
Copy link
Author

carloslfu commented Dec 3, 2016

Suppose this scenario:

playground link

function fun1 (params: { a: number, b: string }) : { c: number, d: string } {
    return {
        c: 0,
        d: '',
    }
}

// this is detected by compiler
function fun2 (params: { a: number, b: string }) : { c: number, d: string } {
    return {
        c: 0,
        d: '',
        x: 0
    }
}

// I want to be able to do (to be DRY):

interface F {
    (params: { a: number, b: string }): { c: number, d: string }
}

const fun1_v2: F = function (params) {
    return {
        c: 0,
        d: '',
    }
}

// this function has the same type as fun2, but is not detected by compiler
const fun2_v2: F = function (params) {
    return {
        c: 0,
        d: '',
        x: 0
    }
}

@mquandalle
Copy link

mquandalle commented Jun 27, 2017

As @DanielRosenwasser asked

is there any reason you're trying to prohibit excess properties here?

I'd like give an additional use case for this feature.

I've an API that is similar to webpack-block. So the basic idea is that we want to create a configuration object with a lot of fields, and we'd like to use some form of composition to encapsulate similar patterns in that object.

To keep it simple let's say that to produce the following configuration object:

const config = {
  entry: "./main.js",
  output: {
    path: __dirname + "/build",
    filename: "bundle.js"
  }
};

we want to write the following code:

const config = createConfig([
  entryPoint('./main.js'),
  setOutput('./build/bundle.js'),
]);

and have the type checker ensure that the config is valid:

interface IConfig {
  entry: string;
  output: {
    path: string,
    filename: string,
  };
}

This composition design could almost already work as shown in #12769 (comment). We could indeed create a partial config type:

type Partial<T> = { [P in keyof T]?: Partial<T[P]> };
type PartialConfig = Partial<IConfig>;

and have entryPoint and setOuput utilities return a PartialConfig, and createConfig take a Array<PartialConfig> as an input. Then if there is a typo and setOutput define a fileName instead of filename the compiler will catch that.

Now the problem is that we don't want entryPoint and setOutput utilities to return PartialConfig but instead to return a partial config transformer, ie a function of type:

type PartialConfigTransformer = (input: PartialConfig | {}) => PartialConfig;

that way utilities functions could execute more complex transformations on the configuration object, and not be limited to inserting additional keys.

However now if we introduce a typo again (setOutput uses fileName instead of filename), it won't be caught by the compiler anymore.

The minimal problem is exposed in this snippet:

minimal example callback weak types

Note that if you had an explicit type annotation for the callback return type, the typechecker is working as expected:

with type annotation

but I would expect that this annotation would be inferred from the barWithCallback signature.

@ghost
Copy link

ghost commented Dec 7, 2017

Just ran into this:

interface I { x: number; y: number; }
[1,2,3].map<I>(n => ({ x: n, y: n, n }));

This is a pretty common pattern in TypeScript's own codebase so we should support getting the contextual type on the object literal and checking for extra properties there.

@RyanCavanaugh
Copy link
Member

@sandersn is this fixed?

@sandersn
Copy link
Member

sandersn commented Feb 5, 2018

I only changed freshness checking of nested object literals when contextually typed by a union. I think that’s the only recent change, so I don’t think it’s fixed. I’d have to test it to know for sure, though.

@WhaleMade
Copy link

So.... any timeline on when this will be fixed?

TypeScript Version: 2.7.2

our use case:

enum EConstants {
    FOO = 'foo',
    BAR = 'bar',
}

interface IState {
    id: string,
    name: string,
}

function converter<EKeys extends string, T>(
    functionMap: {[Key in EKeys] : (objectIn: T) => T}
) {
    return functionMap
}

let result = converter<EConstants, IState>({
    [EConstants.FOO]: (objectIn) => ({
        ...objectIn,

        extraProp: 'extra' // <-- this isn't detected as invalid!
                           // even though the intellisense for converter 
                           // can see that this function should return an IState type
    }),
    [EConstants.BAR]: (objectIn) => ({
        ...objectIn
    })
})

Playground Link

@sandersn
Copy link
Member

Object literals lose their freshness when widened, and we widen return types. I tried a couple of experiments:

  1. Retain flags when widening. The compiler failed to build with this setting. Numerous casts are no longer allowed.
  2. Retain flags when widening return types only. This caused freshness to persist on the function return type, which caused a couple of bugs like:
function f(name: string, age: number) { return { name, age } }
let justName: { name: string } = f("Bob", 42)

Now you get an error that age is an excess property because the fresh object literal { name, age } gets persisted as the return type of f and of course { name, age } is excess when assigned to just { name: string }.

To make this work, we would need to think about all the places where freshness would need to be removed: function declarations would be one, assignments another. I think there are others.

@RyanCavanaugh
Copy link
Member

Going to track at #241

@DanielRosenwasser DanielRosenwasser added Duplicate An existing issue was already created and removed Suggestion An idea for TypeScript labels Sep 2, 2020
@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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants