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

typescript not finding break in interface when type constructed from conditional type #30198

Closed
dagda1 opened this issue Mar 3, 2019 · 3 comments
Labels
Duplicate An existing issue was already created Fix Available A PR has been opened for this issue

Comments

@dagda1
Copy link

dagda1 commented Mar 3, 2019

TypeScript Version:

3.3.1

Search Terms:
conditional types default
Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.

I want to give the user the ability to supply their own interface or default to a different structure:

export interface Config {
  [key: string]:
    | number[]
    | string[]
    | number
    | string
}


export type AddArrayLike<T> = unknown extends T ? Config | Config[] : {
  [P in keyof T]: T[P] | T[P][]
}

export interface INodeGroupProps<T = unknown, State = unknown> {
  data: T[];
  keyAccessor: (data: T, index: number) => string | number;
  start: (data: T, index: number) => unknown extends State ? Config : State;
  enter?: (data: T, index: number) => AddArrayLike<State>;
  update?: (data: T, index: number) => AddArrayLike<State>;
  leave?: (data: T, index: number) => AddArrayLike<State>;
}

So I have a State type argument that defaults to unknown. If it is unknown then it should be Config.

Everything works fine in this example of supplying your own State.

export type Point = {x: number, y: number};

export interface NodesState {
  top: number;
  left: number;
  opacity: number;
}

const points: Point[] = [
  {x: 0, y: 0},
  {x: 1, y: 2},
  {x: 2, y: 3}
] 

const Test: INodeGroupProps<Point, NodesState> = {
    data: points,
    keyAccessor: (d) => {
    return d.x
    },
    start: (point) => {
        return {
            top: point.y,
            left: point.x,
            opacity: 0
        };
    },
    enter: (point) => {
        return {
            top: [point.y],
            left: [point.x],
            opacity: [0]
        };
    },
    update: (point) => {
        return {
            top: [point.y],
            left: [point.x],
            opacity: 0
        };
    },
    leave: (point) => {
        return {
            top: [point.y],
            left: [point.x],
            opacity: 0
        };
    },
};

I get type safety if I change one of the types from the NodeState interface to be not a number, e.g.

return {
  top,
  left,
  opacity: true
};

But if I add a prop that is not part of the NodeState interface then this is not recognised as a break.

e.g.

return {
  top,
  left,
  opacity: 0,
  fook: 'blah' // not part of NodeState
};

Expected behavior:

fook should be identified as not part of the NodeState interface

Actual behavior:

No type error

Playground Link:

playground

Related Issues:

@jack-williams
Copy link
Collaborator

Duplicate of #241 / #29390. Excess property checking isn't being performed in the body of the function expression. Smaller repro:

interface Foo {
  x: () => { x: 'hello' };  
}

const a: Foo = {
  x: () => {
    return { x: 'hello', excess: 3 }; // no error
  }
}

@dagda1
Copy link
Author

dagda1 commented Mar 3, 2019

@jack-williams I see they use neverize as one solution.

Is this possible, I tried this but the extra properties are not flagged.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Mar 4, 2019
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

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.

4 participants