Object spread drops index signature

[ajafff]

TypeScript Version: 3.0.1

Search Terms:
spread index signature, spread indexer

Code

declare var v: Record<string, string>;

let v2 = {...v}; // works
let v3 = {a: '', ...v}; // index signature lost
let v4 = {...v, a: ''}; // same as above
let v5 = {a: 1, ...v}; // index signature lost, should be '{a: number, [x: string]: number | string}'
let v6 = {[Number()]: 1, ...v}; // empty object type ò,Ó, should be '{[x: string]: number | string}'

Expected behavior:
Index signature exists on all object types.

Actual behavior:
Index signature is lost if object literal contains another property assignment. I'm not entirely sure what the type of v6 should be.

Playground Link: https://agentcooper.github.io/typescript-play/#code/CYUwxgNghgTiAEA3WSBc8BK4D2NgB4BnAFxgEsA7AcwBp4TzqA+AbgCg2IRikAmeALzwA3gDpxiAL4t4AelnwA7rgDWhTtyQBmQSKjoA5AbrjRUmfPiVQAD3pkqFKMQCuceBGwkNPRABZdMQk6fXgjaTkFQigAWwQoQngoACNsRBAfJABWQNCARhMJCMtrEDtCBydXd08SOkIAC2wXCGB4ZIQDYVCKFxiOmDoAbRt0BkoqAF10Xv6QGHgAH3pSCckDTMQANkChgDk+gYAKAEpp+AL4U3NI+BAYgAdiAE94bGSAK3AeF4eEACeaABl+pNFptDphYQjMarajnWYDJYrRhUdZAA

Related Issues:
#16694
#16373

[RyanCavanaugh]

I think it's extremely arguable one way or the other. Certainly if you introduce a non-conforming property, the original intent of the index signature is lost.

[jsingerlenovo]

Object.assign is able to handle the types correctly, why can't Object Spread?

declare var v: Record<string, string>;

const v3 = { a: '', ...v }; // index signature lost, type is only { a: string; }
const v7 = Object.assign({ a: '' }, v);  // works, type is { a: string; } & Record<string, string>

const s3 = v3.stuff; // Property 'stuff' does not exist on type '{ a: string; }'.ts(2339)
const s7 = v7.stuff; // works fine

[joealden]

As mentioned above, the type inference of Object.assign currently differs from the type inference of the object spread syntax. While to my understanding their behaviors don't map 1:1, in this scenario, I would personally expect them to both result in the same types being inferred. I'm not advocating for which way to type them (although personally I think that the less restrictive option seems more intuitive), but I do think that it makes sense for these two "object merge" techniques to be consistent?

Here's an newer playground link that shows that this behaviour still exists in TS 4.5:

https://www.typescriptlang.org/play?#code/MYewdgzgLgBA5gUygIQIYQQeQEYCsHCwC8MAFAJQBcMASgSAE4AmAPNAwJZhwA0M7XODAA+MAK5gmCAGZcETAHwwiSgN4AoGFpgMkYhmBiqYABwYgTCBlACeARmoAiRzAC+AbnUf160JFggeARQdsowOPiEAHToEBxwYKSIKOhYQYQUfMZmFla2AExOUAjQLq7knn4QIAA2CFE1IHCkgZEhUTmW1vYVPlUB6VD5YcZRY8loGBHBmabmXQVFJVBlleDVdQ1NLYP5HfN5PZ7qQA

[bmillwood]

I was linked here from a StackOverflow question I asked, after noticing during code review a variable being assigned an incorrect type in a way that the compiler did not catch. My minimal reproduction:

const br: Record<string, number> = {b: 0};
const abr: Record<string, string> = {a: "", ...br};
console.log(abr);

Here abr contains both string and number values, despite the type saying it should only contain strings. It feels like in the presence of this bug I either need to manually type-check spread operators where I see them, or else avoid using them at all.

[coolCucumber-cat]

Why is this still open? It's a clear type violation and there's no reason for it.

[coolCucumber-cat]

@RyanCavanaugh How is this arguable both ways? It's objectively a type violation, what possible argument could there be to not fix it? It's been 5 years.

[joealden]

@coolCucumber-cat maybe it wasn't your intent, but the above messages don't give off a great vibe IMO - while it can be frustrating to run into problems, this is an open source project that you use for free, and this issue makes it clear that the TS team have discussed this on multiple occasions over the years. It's still open for a reason, and it isn't going to be solved any quicker but saying stuff like "it's been 5 years", etc.

[coolCucumber-cat]

@joealden Yes but it's a really common issue, it's a clear violation, it shouldn't be hard to fix, it has Microsoft behind it, it's open source and it's been 5 years. What could you possibly have against fixing this? Or does fixing this require knowing what the dog is doing, what Victoria's secret is and what Obama's last name is?

[joealden]

@coolCucumber-cat feel free to create a PR for this if you think it'd be straight forward - as you can see in this thread I would like this resolved too, the difference is that I'm not demanding it

[coolCucumber-cat]

@joealden Straight forward for someone who has a basic understanding of the codebase and how it works. If someone that knows a place really well gets us lost there, doesn't mean I can fix it just because I know we're lost.

[coolCucumber-cat]

@joealden didn't you say it was open for a reason and they're still discussing it? So my PR will just be rejected. You're implying the only reason it hasn't been fixed is because everyone is too lazy. So why don't you fix it then? Or some of the developers?

[fatcerberus]

@coolCucumber-cat Please tone it down or you're going to get a warning from the maintainers. Unlike #56431, there is no type violation demonstrated in this issue - all the types are inferred, and none of the inferred types are wrong, just inconvenient because the properties described by the index signature become inaccessible.

[bmillwood]

@coolCucumber-cat Please tone it down or you're going to get a warning from the maintainers. Unlike #56431, there is no type violation demonstrated in this issue - all the types are inferred, and none of the inferred types are wrong, just inconvenient because the properties described by the index signature become inaccessible.

I think my example illustrates a type unsoundness here, not merely an inconvenience? I end up with a record that says it only has string values, but in reality it has a number value.

[fatcerberus]

@bmillwood I was talking specifically about the example in the OP (the original bug report). Your example is covered by #56431, and I agree it is incorrect.