microsoft/TypeScript

Two types should only be compatible if their members are compatible - even if all these members are optional

DmitryEfimenko opened this issue · 10 comments

DmitryEfimenko commented

I wanted to see if I can update definitions a MongoDB filter object, which utilizes these expressions. Below is just a start, but I already see an issue when the code compiles when it should not. This issue possibly has similar underlying issue as #12769

TypeScript Version: 2.1.4

Code

type MongoFilter<T> = $and<T> | $or<T> | propertyRule<T>;

type logical<T> = $and<T> | $or<T>;

type $and<T> = { $and: Array<logical<T> | propertyRule<T>> }
type $or<T> = { $or: Array<logical<T> | propertyRule<T>> }

type propertyRule<T> = {
  [P in keyof T]?: T[P] | comparison<T[P]>
}

type comparison<V> = $eq<V> | $gt | $lt;

type $eq<V> = { $eq: V };
type $gt = { $gt: number };
type $lt = { $lt: number };

// See if MongoFilter works:
interface Person {
  id: number;
  name: string;
  age: number;
}

// should compile
let f1: MongoFilter<Person> = { $or: [{ id: 1 }, { name: 'dima' }] };
let f2: MongoFilter<Person> = { $and: [{ id: 1 }] };
let f3: MongoFilter<Person> = { id: 1 };

// should not compile
let w1: MongoFilter<Person> = { wrong: 1 }; // does not compile - good
let w2: MongoFilter<Person> = { $or: [{ wrong: 1 }] }; // compiles

Expected behavior:
Code below should not compile

let w1: MongoFilter<Person> = { $or: [{ wrong: 1 }] };

Actual behavior:
But it does

PyroVortex commented

My understanding of trying to do this myself is that since all properties in type propertyRule<T> are optional, the compiler is resolving the type as {}, which is assignable to { wrong: 1}.

DmitryEfimenko commented

that would make sense, but I don't think is the case since the following code does not compile, which is expected behaviour:

let w1: MongoFilter<Person> = { wrong: 1 };

In this case the propertyRule<T> is used to evaluate { wrong: 1}, and it properly tells that there is no property wrong on type T

PyroVortex commented

Note that this, on the other hand, compiles just fine:

const x = { wrong: 1 };
let w1: MongoFilter<Person> = x;
DmitryEfimenko commented

wow... so clearly there is a disconnect somewhere. My conclusion is that either type propertyRule<T> should not resolve to { } or the code below should not compile:

type s = { };
let t: s = { wrong: 1 }

How does it make sense that it compiles given the example above? Is it by design?

DmitryEfimenko commented

Thanks for this link. Fantastic FAQ, I've read through the whole thing.

Types with no members can be substituted by any type.

However, FAQ does not really provide the reason WHY it was decided to do it this way. I also didn't find any discussion about this. In my opinion { } != any. I saw a mention of some mysterious side effects which can be caused by creating an object of type { }, but didn't find what the side effects are.

The bottom line is that an object with the wrong property should not be assignable to an object, optional properties of which do not contain property wrong:

interface Person {
  id?: number;
  name?: string;
  age?: number;
}

let wrongObject = { wrong: 1 };
let p: Person = wrongObject; // should not compile

This supports the statement of what is structural typing, which is used by TypeScript (also from FAQ):

two types are compatible if their members are compatible.

Changing the title now that I phrased the issue better.

  • 👍1
DmitryEfimenko commented

Glad to see I'm not the first one to see an issue with this!

So to summarize: #12501 closes #1809, where @sccolbert said:

This proposal will also solve the problem of typing "object bags" where each property is optional, but other primitive types should be disallowed.

Correct me if I'm wrong - that's not the same as my issue. I could not understand, will this PR address this issue?

#9841 and #12914 indeed discusses this issue, but is labeled with "Working as intended"? However, it mentions #3842, which I believe was intended to solve this issue and had a PR, but was never merged.

Side note: the relevance of this issue will only increase now, that Partial<T> type is introduced.

mhegazy commented

Excess property checks would have caught that if it operated on unions. tracked by #12745

inner-peace commented

Side note: the relevance of this issue will only increase now, that Partial type is introduced

Yep, i think this behaviour is very strange:

const itFails: Partial<{ foo: number }> = { bar: 42 }; // TS2322:Type '{ bar: number; }' is not assignable

const ext = {bar: 42};
const butItWorks: Partial<{ foo: number }> = ext; // OK
  • 👍3
mhegazy commented

Fixed by #16047