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

Possible bug: Hybrid between two interfaces of a union passes type checking #14383

Closed
dsifford opened this issue Mar 1, 2017 · 4 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@dsifford
Copy link
Contributor

dsifford commented Mar 1, 2017

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

Went to StackOverflow with this first and the consensus there seems to be that this is a bug.

Here's my question again for convenience:


Consider a situation where an object can have exactly the FooBar or Baz interface listed below.

interface FooBar {
    foo: string;
    bar: string;
}

interface Baz {
    baz: string;
}

It's my understanding that this "one or the other" situation can be handled by creating a union between FooBar and Baz.

type FooBarOrBaz = FooBar | Baz;

So far so good...

The issue I'm having is that the following object passes type checking:

const x: FooBarOrBaz = {
    foo: 'foo',
    baz: 'foo',
};

Expected behavior: x should not pass type checking. It's neither FooBar nor Baz, but rather a combination between the two.

Actual behavior: x passes type checking.

Playground Example

@ahejlsberg
Copy link
Member

ahejlsberg commented Mar 1, 2017

This is working as intended. The type { foo: string, baz: string } is indeed assignable to { baz: string } because a subtype is free to add additional properties. The thing that is perhaps surprising is that you don't get an error in your example, but you do get an error in the following:

const x: Baz = {
    foo: 'foo',  // Error, excess property
    baz: 'foo',
};

This is because when the target is a union type our excess property check for object literals (#3823) only checks that each property exists in some variant of the target. It should perhaps be stricter and check that the source is assignable to at least one variant with no excess properties (which would then error on your example).

@dsifford
Copy link
Contributor Author

dsifford commented Mar 1, 2017

@ahejlsberg Ah, okay that makes sense I suppose.

I'm admittedly ignorant as to how much added complexity it would take to add the stricter checking but, intuitively speaking, I would assume the example above to only compile successfully with interfaces having an index signature.

The use-case I had when I ran into this issue was when I was attempting to create a function parameter that accepted two different forms of authentication: One being basic username and password auth (in which case, both of these fields is required), and the other being browser cookie authentication where only a single field is required.

I ended up just creating a union between a BasicAuth interface and string which solved the problem.

Thanks for the education on this.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Mar 1, 2017
@fcrick
Copy link

fcrick commented Mar 7, 2017

I'd love to see these stricter checks at some point - any way to work around this issue without changing the types themselves? I'm trying to validate some existing data, and I can describe all the types I need, but the lack of the stricter check lets bad data through.

// no error - this is what I hoped would work
let x: { a: number; } | { b: number; } = {
    a: 5,
    b: 5
};

// errors as expected
x = { a: 5 };
x.b = 4;

// errors as expected
const y: { b: number; } = {
    a: 5,
    b: 5
};

@RyanCavanaugh
Copy link
Member

Some relevant discussion of this at #12936 (comment)

@mhegazy mhegazy closed this as completed Apr 21, 2017
@jcalz jcalz marked this as a duplicate of #17256 Jul 18, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants