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

Spread of set into object not detected as type error #53747

Closed
cmtm opened this issue Apr 12, 2023 · 3 comments
Closed

Spread of set into object not detected as type error #53747

cmtm opened this issue Apr 12, 2023 · 3 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@cmtm
Copy link

cmtm commented Apr 12, 2023

Bug Report

A spread of a Set into an object is assignable to a Set. This shouldn't be allowed, because the object isn't a Set. tsc should be able to detect this at compile time.

🔎 Search Terms

set spread

🕗 Version & Regression Information

This is the behavior in the version I tried (3.3.3, 4.6.2, 5.0.4, 5.1.0-dev.20230411), and I reviewed the FAQ for entries about Common "Bugs" That Aren't Bugs

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

⏯ Playground Link

Playground link with relevant code

💻 Code

const my_set1: Set<number> = new Set();
// The type of my_set2 is actually Object.
// tsc doesn't detect this, and thinks it's a `Set<number>`.
const my_set2: Set<number> = {...my_set1};

// This will fail at runtime because my_set2 isn't a Set.
my_set2.has(2);

🙁 Actual behavior

tsc doesn't detect the type error.

🙂 Expected behavior

tsc should detect the type error.

@fatcerberus
Copy link

fatcerberus commented Apr 12, 2023

This is a more general limitation than implied - { ...new A() } (for some class A) likewise isn’t an A, but TS thinks it is because structural typing. I'm thinking this will probably be considered a design limitation and not a bug.

Playground

class U {
    foo = "fooey";
    bar = "bard";
}

const notU = { ...new U() };
const u: U = notU;
console.log(u instanceof U);  // false, oops

@RyanCavanaugh
Copy link
Member

Yeah, { ...x } is always assumed to be the same type of x. Non-enumerability isn't tracked anyway, and expanding it out into { a: x.a, b: x.b, c: x.c } etc breaks a bunch of scenarios that people rightly expect to work.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Apr 12, 2023
@MartinJohns
Copy link
Contributor

Non-enumerability isn't tracked anyway

Related: #9726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants