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

Incorrect type inferred after threading destructured object with getter #54577

Closed
nmattia opened this issue Jun 8, 2023 · 5 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@nmattia
Copy link

nmattia commented Jun 8, 2023

Bug Report

Javascript will not call getters when destructuring an object:

> class Person { get name(){ return "Sammy"; } }
undefined
> { name } = { ... new Person()}
{}

TypeScript correctly infers this when such an object is used directly:

class Person {
  public get name() { return "Sammy"; }
}


function greet(person: Person){
  console.log("Hi, " + person.name + "!");
}

const sammy = new Person();
greet(sammy); // works
// greet({...sammy}) // correctly fails

If the object is threaded through another function however, the (getter) type information seems to get lost:

function checkAndGreet(person: { name: string} ){
  if(person.name === "Josh") { throw new Error("Not you Josh!") }

  const accepted = { ... person };
  greet(accepted);
}

checkAndGreet(sammy); // works but shouldn't, prints "Hi, undefined!"

🔎 Search Terms

typescript, getter, destructuring

🕗 Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

💻 Code

class Person {
  public get name() { return "Sammy"; }
}

function checkAndGreet(person: { name: string} ){
  if(person.name === "Josh") { throw new Error("Not you Josh!") }

  const accepted = { ... person };
  greet(accepted);
}

function greet(person: Person){
  console.log("Hi, " + person.name + "!");
}

const sammy = new Person();
greet(sammy); // works
// greet({...sammy})// correctly fails
checkAndGreet(sammy); // works but shouldn't

🙁 Actual behavior

TypeScript did not warn that {... person} did not have the name field anymore

🙂 Expected behavior

TypeScript should have warned that the getters have not been called

@fatcerberus
Copy link

I'm not really sure how this could be fixed given the current design of the type system, unless you're proposing that sammy shouldn't be assignable to { name: string } (which would be a huge breaking change and also very inconvenient). The root of the problem is that TS doesn't make a distinction, structurally, between different kinds of properties (own vs. prototype, accessor vs. data, etc.), so that would have to happen first to even begin to move the needle on this.

if(person.name === "Josh") { throw new Error("Not you Josh!") }

Poor Josh, what did he ever do to anyone to deserve this? 😄

@nmattia
Copy link
Author

nmattia commented Jun 8, 2023

Poor Josh, what did he ever do to anyone to deserve this? 😄

🙈 He's too cool to hang out with Sammy!

unless you're proposing that sammy shouldn't be assignable to { name: string }

Yeah, that's what I kind of had in mind, though as you say:

would be a huge breaking change and also very inconvenient

I half hoped that TypeScript kept some internal knowledge about the getters and that the displayed type (e.g. { name: string}) was purely a frontend simplification. That being said, I was surprised that TypeScript simply inferred { name: string }.

I think I'll just need to be careful when using getters! To be honest I didn't actually come here expecting an easy fix hehe

@fatcerberus
Copy link

Yeah, if you directly have a type (like Person) where a property is declared as a getter then TS can track that in e.g. spreads, but in terms of type compatibility there's no real distinction. Once you assign it to a { name: string } you lose all information that you ever had a getter.

@jcalz
Copy link
Contributor

jcalz commented Jun 8, 2023

Essentially the same as #26547. TS doesn't currently track own/enumerable properties on types; it can do so for classes because it knows which things are actually going to end up on the prototype vs the instance, but when you indirect through types then such information can't be represented.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jun 12, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants