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

Extended class seems to automatically also turn properties into open records #44941

Closed
fregante opened this issue Jul 8, 2021 · 5 comments
Closed

Comments

@fregante
Copy link

fregante commented Jul 8, 2021

Bug Report

🔎 Search Terms

class extended properties interface open record key

🕗 Version & Regression Information

I tried the latest (v4.3.5) and v3.6

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about extended classes

⏯ Playground Link

Playground link with relevant code

💻 Code

interface Person {
    names?: string[];
}

// otherUnrelatedProperties is correctly rejected
class House {
  owner: Person = {
    names: ['Johnny'],
    otherUnrelatedProperties: ':) rejected',
  };
}

// otherUnrelatedProperties is allowed
class Villa extends House {
  owner = {
    names: ['Johnny'],
    otherUnrelatedProperties: ':( accepted',
  };
}

// otherUnrelatedProperties is rejected if I re-define the property type
class Resort extends House {
  owner: Person = {
    names: ['Johnny'],
    otherUnrelatedProperties: ':) rejected',
  };
}

🙁 Actual behavior

otherUnrelatedProperties was accepted on a property of type Person. names is correctly required by TypeScript, so Person is being correctly set.

The type of the owner property appears to be:

owner: Person & Record<string, any>

🙂 Expected behavior

otherUnrelatedProperties should not be accepted on the owner property.

@MartinJohns
Copy link
Contributor

Duplicate of #10570.

In your Villa example the field owner is not typed Person. Instead the type is inferred from the assigned object literal.

image

@fregante
Copy link
Author

fregante commented Jul 8, 2021

Thanks! I was sure an issue already existed but I couldn’t find it

Yes, that’s the unexpected part. If Villa extends House, then I expect Villa.owner to match House.owner. If not, what is extends extending exactly? If this is part of #10570 I can close the issue.

the field owner is not typed Person

Villa.owner.names still needs to match House.owner.names, so it is at least in part inherited.

@jgbpercy
Copy link

jgbpercy commented Jul 8, 2021

It might help to understand that the reason that otherUnrelatedProperties is rejected on House and Resort because of "excess property checking" (here's a pretty reasonable short post on the subject), not because it's strictly illegal under TS's structural type system to have otherUnrelatedProperties on a Person.

In the case of Villa, all TS is going to do is check that owner is assignable to Person, but it wont do excess property checking like it does with House and Resort where there's an explicit type annotation - Villa's owner property is analogous to meaningfulPoint in that linked post. Because an object with otherUnrelatedProperties is assignable to Person, it wont be rejected.

If you remove the name property for Villa's owner, then you get an error. That's what the extends gets you.

@fregante
Copy link
Author

fregante commented Jul 8, 2021

Wow I never knew that TS would treat = obj and = {prop: 1} differently. This might help me understand TS issue in the future.

@fregante
Copy link
Author

fregante commented Jul 8, 2021

Closing as it matches #10570

It seems that using property assignments like that essentially sets the types as well, which sounds like a feature more than a bug 😰

class Villa extends House {
  owner = {
    names: ['Johnny'],
    otherUnrelatedProperties: ':( accepted',
  };
  constructor() {
    super();
    this.owner = {
      names: ['Johnny'],
      otherUnrelatedProperties: ':( totally still accepted',
    };
  }
}

So this is "the right way" to set properties in an extended class without changing the parent’s types:

// otherUnrelatedProperties is rejected in the constructor 🎉 
class Villa extends House {
  constructor() {
    super();
    this.owner = {
      names: ['Johnny'],
      otherUnrelatedProperties: ':) rejected',
    };
  }
}

@fregante fregante closed this as completed Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants