Assigning a value from Partial of properties of an Object to that Object results in type error

Question

While working on an angular service I stumbled over a problem with assigning the properties of a Partial of an Object to an instance of that Object.

function foo<T>(input: Partial<T>, instance: T) {
  // won't work:
  for (const key in input) {
    instance[key] = input[key];
  }
  // won't work:
  for (const key in input) {
    const i = input[key];
    if (typeof i !== 'undefined') {
      instance[key] = i;
    }
  }
  // works:
  for (const key in input) {
    instance[key] = input[key]!;
  }
}

The assignments throw a type error. In the first case, I get

Type 'T[Extract<keyof T, string>] | undefined' is not assignable to type 'T[Extract<keyof T, string>]'.
  'T[Extract<keyof T, string>]' could be instantiated with an arbitrary type which could be unrelated to 'T[Extract<keyof T, string>] | undefined'.(2322)

second case results in

Type 'Partial<T>[Extract<keyof T, string>] & ({} | null)' is not assignable to type 'T[Extract<keyof T, string>]'.
  'T[Extract<keyof T, string>]' could be instantiated with an arbitrary type which could be unrelated to 'Partial<T>[Extract<keyof T, string>] & ({} | null)'.(2322)

Third case works, but I'd rather not use the bang operator here. I am out of ideas how to solve this in any other way.

Please consider modifying the code here to be a [mre]. You've got a lot of extraneous things (including the apparent dependence on angular) which could probably be replaced entirely with [the code shown here](https://tsplay.dev/wR7ALN). If you [edit] to do that and strip the surrounding text to the minimum needed to describe the code issue, it will improve the question for future readers and you'll get a more targeted answer. — jcalz, May 21 '23 at 20:25
(continued) Once you do that, does [this approach](https://tsplay.dev/NVROqN) meet your needs? Widening from `T` to `Partial` is seen as safe by the compiler, and then the error goes away (not that TS is fully sound when performing generic assignments, but the unsoundness here is harmless because you're copying a property at the same `key` index). If that addresses your issue I'll write up an answer explaining; if not, what am I missing? — jcalz, May 21 '23 at 20:31
@jcalz thx, I edited my answer accordingly. I tried your approach and it does indeed solve the problem, and I think it is an improvement over non-null assertion, so please do add it as an answer so I can select it. — Christian Schmidt, May 21 '23 at 20:44

jcalz · Accepted Answer · 2023-05-21T21:18:22.747

If you have a value of type Partial<T> and just copy all of its properties to a value of type T as-is, then you might find yourself erroneously copying an undefined value into a property that doesn't expect it (e.g., if --exactOptionalPropertyTypes is not enabled).

So it's probably good to get an error here, at least in the case where you don't check for undefined. Of course you also get an error when you do check for undefined, because TypeScript's ability to reason about generic types isn't very good. It doesn't see Partial<T>[K] & ({} | null) as assignable to T[K], even though it is (I think? My ability to reason about generic types might also not be very good?)

Since TypeScript's type system isn't fully sound, it's always possible to fool the compiler into allowing you to do unsafe things. In some sense, anything you do that works around this is no safer than using the non-null assertion operator) as you've done here:

function foo<T>(input: Partial<T>, instance: T) {   
  for (const key in input) {
    if (typeof input[key] !== "undefined") {
      instance[key] = input[key]!;
    }
  }   
}

I don't think there's anything wrong with this, since it conveys your intent fairly clearly to any future developer: you check for undefined and then assert that it's not undefined.

However, if you want to avoid a type assertion, you could take advantage of some unsoundness by widening instance from T to Partial<T>. The language takes the position that objects are covariant in their property types (see Difference between Variance, Covariance, Contravariance and Bivariance in TypeScript ) and since every property of T is assignable to the corresponding property of Partial<T>, then it sees T as assignable to Partial<T>. That gives you this code:

function foo<T>(input: Partial<T>, instance: T) {
  const widenedInstance: Partial<T> = instance;
  for (const key in input) {
    if (typeof input[key] !== "undefined") {
      widenedInstance[key] = input[key];
    }
  }
}

Now there's no error. But, it's not any safer than your other code, really. Objects are only really covariant in their read-only properties. As soon as you start writing to properties, then covariance is unsafe. The above code happens to be safe because I'm testing for undefined, but I hadn't, or if I had made a mistake and tested the wrong way, there would still be no error:

if (typeof input[key] === "undefined") { // oops, no error
  widenedInstance[key] = input[key];
}

So you have to be careful. And since you have to be careful here no matter what you do, it's essentially a matter of opinion whether either of the two approaches is better, or whether some third approach might be even better.

TypeScript's unsoundness is there intentionally, because a fully sound type system for TypeScript is "simply a fool's errand" and would TypeScript almost unusable, so even if you try to rewrite the code to be as verified as safe as possible by the compiler (maybe using generics for the key type as well as the object so that you are assigning T[K] directly instead of T[keyof T], etc) it will almost certainly allow unsafe things to happen in edge cases. You might as well go for ergonomic code that works for you... which again, is subjective.

Like, here's a potential third approach:

for (const key in input) {
  const i: T[Extract<keyof T, string>] | undefined = input[key]
  if (typeof i !== "undefined") {
    instance[key] = i;
  }
}

All I've done here is widen your i to the type the compiler requires for instance[key] = i to work once undefined is eliminated. Is that better? ‍♂️

Playground link to code

Thank you @jcalz for taking the time to write up such a thorough answer. Since it makes no difference I will go with the check for undefined and non-null assertion - seems less confusing than the other approach indeed. — Christian Schmidt, May 21 '23 at 22:29

Assigning a value from Partial of properties of an Object to that Object results in type error

1 Answers1