Skip to content

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

Closed

Description

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

Activity

  1. PyroVortex commented on Dec 14, 2016

    @PyroVortex

    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}.

  2. DmitryEfimenko commented on Dec 14, 2016

    @DmitryEfimenko
    Author

    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

  3. PyroVortex commented on Dec 15, 2016

    @PyroVortex

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

    const x = { wrong: 1 };
    let w1: MongoFilter<Person> = x;
  4. DmitryEfimenko commented on Dec 15, 2016

    @DmitryEfimenko
    Author

    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?

  5. normalser commented on Dec 15, 2016

    @normalser
  6. changed the title [-]Recursive props of Mapped types don't work as expected[/-] [+]two types should only be compatible if their members are compatible - even if all these members are optional[/+] on Dec 15, 2016
  7. changed the title [-]two types should only be compatible if their members are compatible - even if all these members are optional[/-] [+]Two types should only be compatible if their members are compatible - even if all these members are optional[/+] on Dec 15, 2016
  8. DmitryEfimenko commented on Dec 15, 2016

    @DmitryEfimenko
    Author

    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.

  9. DmitryEfimenko commented on Dec 15, 2016

    @DmitryEfimenko
    Author

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

    So to summarize: #12501 closes #1809, where S. Chris Colbert (@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.

  10. mhegazy commented on Dec 15, 2016

    @mhegazy
    Contributor

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

  11. inner-peace commented on Apr 27, 2017

    @inner-peace

    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
  12. mhegazy commented on Jun 13, 2017

    @mhegazy
    Contributor

    Fixed by #16047

  13. added
    FixedA PR has been merged for this issue
    and removed on Jun 13, 2017
  14. locked and limited conversation to collaborators on Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FixedA PR has been merged for this issueSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions