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

Cannot type a function that copies a property in an object #31725

Closed
arielshaqed opened this issue Jun 2, 2019 · 6 comments
Closed

Cannot type a function that copies a property in an object #31725

arielshaqed opened this issue Jun 2, 2019 · 6 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@arielshaqed
Copy link

TypeScript Version: Version 3.6.0-dev.20190602

Search Terms: "generic return type" (returns too many results, sorry)

Code

function copy<K extends string, J extends string, T extends Record<K, number>>(
    o: T, s: K, t: J
): T & Record<J, number> {
    return { ...o, [t]: o[s] };
}

const { b } = copy({ a: 2 }, 'a', 'b');

Expected behavior:

Code compiles with 3.4.x. It is unsound when explicitly specifying type params, but with implicit type params it lets me express the type of copy.

Actual behavior:

With 3.5.1 compilation fails:

../../../../../../tmp/x.ts:4:5 - error TS2322: Type 'T & { [x: string]: T[K]; }' is not assignable to type 'T & Record<J, number>'.
  Type 'T & { [x: string]: T[K]; }' is not assignable to type 'Record<J, number>'.

4     return { ...o, [t]: o[s] };
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Found 1 error.

Here's the thing: the new compiler is correct, the claimed type is unsound. I can specialize the call to copy to make it unsound, e.g.

const { c }: Record<'c', number> = copy<'a', 'b' | 'c', Record<'a', number>>({ a: 2 }, 'a', 'b');

actually compiles with 3.4.5, and now c is undefined and 3.4.5 thinks it has type number. 3.5.1 prevents that (yay!).

I would like a way to type copy; I used to have an unsound type for it (but safe unless used with explicit type params), now I have no way to type it.

Playground Link: https://www.typescriptlang.org/play/#src=function%20copy%3CK%20extends%20string%2C%20J%20extends%20string%2C%20T%20extends%20Record%3CK%2C%20number%3E%3E(%0A%20%20%20%20o%3A%20T%2C%20s%3A%20K%2C%20t%3A%20J%0A)%3A%20T%20%26%20Record%3CJ%2C%20number%3E%20%7B%0A%20%20%20%20return%20%7B%20...o%2C%20%5Bt%5D%3A%20o%5Bs%5D%20%7D%3B%0A%7D%0A%0Aconst%20%7B%20c%20%7D%3A%20Record%3C'c'%2C%20number%3E%20%3D%20copy%3C'a'%2C%20'b'%20%7C%20'c'%2C%20Record%3C'a'%2C%20number%3E%3E(%7B%20a%3A%202%20%7D%2C%20'a'%2C%20'b')%3B%0A

Related Issues:
Even found a suspect PR!

Probably introduced in #30769, which I found via #31672.

@fatcerberus
Copy link

fatcerberus commented Jun 3, 2019

The unsoundness actually extends a bit further than you think.

T extends Record<K, number>

This doesn't mean what you think it means. See #31661 (comment), in particular:

In general, the constraint Record<string, XXX> doesn't actually ensure that an argument has a string index signature, it merely ensures that the [known] properties of the argument are assignable to type XXX.

In other words the index signature doesn't mean that "it has every possible property"--which is logistically impossible--but rather that "it can have any possible property"; for the purposes of structural subtyping these are not equivalent.

Thus: Because T can have fewer properties than its index signature constraint suggests (i.e. infinity), we can't prove that a type known to have an index signature is assignable to it.

@arielshaqed
Copy link
Author

Thanks, that makes sense!

My question remains: what is the type of a function copy that adds a new field k which is a copy of a field j in an object o? Previous versions of TypeScript were unsound, but let me write a type that fits into my vague intuitive category of "sound as long as you only ever rely on argument generic type deduction and never try to specify it manually". SALAYOEROAGTDANTTSIM is a useful (if nasty) concept...

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Jun 13, 2019
@RyanCavanaugh
Copy link
Member

Can you specify a few more examples of valid and invalid calls to copy? That helps us give you a more precise definition of what you're looking for

@arielshaqed
Copy link
Author

You're right, I under-specified the desired type.

I would like to type this JavaScript function:

function copy(o, s, t) {
    return { ...o, [t]: o[s] };
}

where s must be a key of o.

Desired types:

copy({ a: 'one', b: 2 }, 'b', 'c');   // Returns { a: 'one', b: 2, c: 2}, type { a: string, b: number, c: number }

const b: B = makeB();
copy({ a: 1, b: makeB() }, 'b', 'c');  // Returns value of type { a: number, b: B, c: B }

copy({ a: 'one' }, 'b', 'c');   // Does not compile, `o` has no property `b`.

const k: string = 'b';
copy({ a: 'one', b: 1 }, k, 'c');  // Don't care if it compiles (with less useful type) or fails to compile.

Hope this makes better sense!

@RyanCavanaugh
Copy link
Member

The signature you want is:

function copy<T, K extends keyof T, J extends string>(obj: T, srcKey: K, dstKey: J): T & Record<J, T[K]>

The implementation is just going to need a type assertion somewhere; this is inherent in the fact that you can't really be sure that you're not self-overwriting in a broken way inside the function body

@arielshaqed
Copy link
Author

Thanks, that works -- and I understand why we need a type assertion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

3 participants