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 narrowing with interface inheritance #12295

Closed
HolgerJeromin opened this issue Nov 16, 2016 · 5 comments
Closed

Type narrowing with interface inheritance #12295

HolgerJeromin opened this issue Nov 16, 2016 · 5 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@HolgerJeromin
Copy link
Contributor

TypeScript Version: 2.0.3
This is related to discriminated unions, but with interface inheritance.

I get a SyncCommand via XHR and need to do different stuff depending on the command.
Right now i need a Type assertions for that, but my interface definition holds all information already.

Code

export interface SyncCmd {
	name: string;
}
export interface SyncCmdFoo extends SyncCmd {
	name: 'Foo';
	contentFoo: string;
}
// In real my code the command comes from XHR
const command: SyncCmd = {name: 'Foo', contentFoo: 'hello'};
switch (command.name) {
	case 'Foo': 
		alert(command.contentFoo);
		break;
	default:
		break;
}

Expected behavior:
Inside the 'Foo' case the type should be narrowed to SyncCmdFoo.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 16, 2016

That isn't narrowing, that is "magical" widening... You asserted command was of type SyncCmd therefore TypeScript is taking your word for it and cannot "magically" widen to something else. If you tell TypeScript it could be SyncCmd | SyncCmdFoo it will then narrow the type appropriately:

export interface SyncCmd {
    name: string;
}
export interface SyncCmdFoo extends SyncCmd {
    name: 'Foo';
    contentFoo: string;
}
// You need to tell TypeScript the widest set of types, so it can then narrow them
const command: SyncCmd | SyncCmdFoo = {name: 'Foo', contentFoo: 'hello'};
switch (command.name) {
    case 'Foo': 
        alert(command.contentFoo);
        break;
    default:
        break;
}

@HolgerJeromin
Copy link
Contributor Author

OK, understood. Thanks.
It would be nice to have a pseudo type "all inherit types from SyncCmd".
But probably this must (and should?) be maintained by myself.

@normalser
Copy link

@kitsonk @HolgerJeromin it will not work in latest TS - command will not be narrowed to SyncCmdFoo

const command: SyncCmd | SyncCmdFoo = {name: 'Foo', contentFoo: 'hello'};
switch (command.name) {  // <- Type: SyncCmd | SyncCmdFoo, at playground type: SyncCmdFoo
    case 'Foo': 
        alert(command.contentFoo);  // <- Type: SyncCmd | SyncCmdFoo
        break;
    default:
        break;
}

We would need to have ability to provide:

export interface SyncCmd {
    name: string - 'Foo';
}

for it to work

Take a look here: #12182 and #7993

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Nov 16, 2016
@HolgerJeromin
Copy link
Contributor Author

HolgerJeromin commented Nov 22, 2016

I removed the name:string from the base interface and all works as expected

@InExtremaRes
Copy link

@kitsonk
I know this is a little old, but I can't figure out how then is implemented in src/server/session.ts L1771.

There the map has value type (request: protocol.Request) => HandlerResponse, but then every value is a function whose parameter is a type extending Request (an specific request, like OpenExternalProjectRequest); here are Request and its subtypes src/server/protocol.ts L137.

Am I misunderstanding? I guess it needs to narrow based on Request.command to an specific Request interface, but maybe there is another implementation in the code i'm not seeing, maybe some explicit type assertion...

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants