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

Excess object literals property checking fails when the literal includes a spread #13878

Closed
sandersn opened this issue Feb 3, 2017 · 7 comments
Labels
Breaking Change Would introduce errors in existing code

Comments

@sandersn
Copy link
Member

sandersn commented Feb 3, 2017

Excess object literal property checking fails when the literal includes a spread.

let x = { b: 1, extra: 2 }
let xx: { a, b }  = { a: 1, ...x, z: 3 } // error for 'z', no error for 'extra'

Actual: No Error

Expected:
Error for 'z', no error for 'extra'

@sandersn
Copy link
Member Author

sandersn commented Feb 3, 2017

Probably checkObjectLiteral needs to set ObjectLiteral and FreshObjectLiteral flags on the result of getSpreadType (if the result is an object type — if it's a type parameter then freshness wouldn't apply anyway).

sandersn added a commit that referenced this issue Feb 6, 2017
Previously, object literals with spreads in them would not issue object
literal freshness errors.  Fixes #13878
@mhegazy
Copy link
Contributor

mhegazy commented Feb 6, 2017

We discussed this in #12997, and decided it worked as intended. what changed?

@mhegazy mhegazy added the Needs More Info The issue still hasn't been fully clarified label Feb 6, 2017
@sandersn
Copy link
Member Author

sandersn commented Feb 6, 2017

I just forgot when we noticed it again on Friday afternoon. On the surface it looks like an oversight.

@wclr
Copy link

wclr commented Apr 5, 2017

let x = { b: 1, extra: 2 }
let xx: { a, b }  = { a: 1, ...x, z: 3 } // error for 'z', no error for 'extra'

I see currenlty no error in 2.3 (march) for z was that fixed?

And I also don't get why there should not be an error for extra if we are trying to assign it to {a, b}

@sandersn
Copy link
Member Author

sandersn commented Apr 5, 2017

@whitecolor See the last section #12997 for discussion.

@sandersn sandersn added Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it and removed Needs More Info The issue still hasn't been fully clarified labels Apr 5, 2017
@sandersn sandersn reopened this May 17, 2017
@mhegazy mhegazy removed the Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it label May 17, 2017
@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label May 17, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 17, 2017
@wclr
Copy link

wclr commented May 17, 2017

So it will be fixed, but in 2.4?

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2017

yes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

No branches or pull requests

3 participants