Typescript: 3.5.1 (regression): "string cannot be used to index T" where T is Record<string, any>

[lll000111]

This is new as of 3.5.1, there was no error in 3.4.5.

Error message:

TS2536: Type 'string' cannot be used to index type 'T'

Obviously wrong, since T is defined as Record<string, any>.

function isString (thing: unknown): thing is string {
    return typeof thing === 'string';
}

function clone<T extends Record<string, any>>(obj: T): T {
    const objectClone = {} as T;

    for (const prop of Reflect.ownKeys(obj).filter(isString)) {
        objectClone[prop] = obj[prop];
    }

    return objectClone;
}

Playground Link — It will not show an error until the Playground uses the new TS 3.5.x

[felix9]

Here's a similar case I ran into:

interface Bag {
  [key: string]: number;
}

function put<T extends Bag>(a: T, k: string, v: number): void {
  a[k] = v;
}

Compiling that produces this:

$ tsc test.ts
test.ts:6:3 - error TS2536: Type 'string' cannot be used to index type 'T'.

6   a[k] = v;
    ~~~~

Found 1 error.

[RyanCavanaugh]

This is the intended behavior but I need to write several pages of documentation for "How do index signatures in generic constraints work [if at all]"

[felix9]

Confusingly, this will compile without error (though --noImplicitAny will complain):

interface Bag {
  [key: number]: boolean;
}
function put<T extends Bag>(a: T) {
  a["x"] = "y";
}

[ahejlsberg]

This is an effect of #30769 and a known breaking change. We previously allowed crazy stuff like this with no errors:

function foo<T extends Record<string, any>>(obj: T) {
    obj["s"] = 123;
    obj["n"] = "hello";
}

let z = { n: 1, s: "abc" };
foo(z);
foo([1, 2, 3]);
foo(new Error("wat"));

In general, the constraint Record<string, XXX> doesn't actually ensure that an argument has a string index signature, it merely ensures that the properties of the argument are assignable to type XXX. So, in the example above you could effectively pass any object and the function could write to any property without any checks.

In 3.5 we enforce that you can only write to an index signature element when we know that the object in question has an index signature. So, you need a small change to the clone function:

function clone<T extends Record<string, any>>(obj: T): T {
    const objectClone = {} as Record<string, any>;

    for (const prop of Reflect.ownKeys(obj).filter(isString)) {
        objectClone[prop] = obj[prop];
    }

    return objectClone as T;
}

With this change we now know that objectClone has a string index signature.

[felix9]

I'm still a little confused by that explanation. Given this dubious bit of code, in both --strict and non-strict mode, the only complaint is the baggish["z"] = 3 line. I'm not sure why that line in particular gets to be the fatal error, and the only fatal error?

```typescript
interface Bag {
}

function put1(bag: Bag) {
bag["z"] = 3;
}

function put2(baggish: T) {
baggish["z"] = 3;
}

const bag: Bag = {};
put1(bag);
put2(bag);

type Point = {x: number};
const point = {x: 3};
put1(point);
put2(point);

[lll000111]

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

I don't accept this "explanation".

Why is there even a place for the key type in Record<keyType, valueType if it has no meaning???

So, in the example above you could effectively pass any object and the function could write to any property without any checks.

That is the whole point!!! That's why the type is Record, string,...>! Note that "string" as type for the key and not some limited union.

[lll000111]

Also: Why would it EVER complain — in Javascript! — if I use a string as index in an object? By using Record I already told TS that it is meant to be a generic map object. The explanation does not make sense.

In general, coming from (the more strict!) Flow, all those many errors I now have in TypeScript when I iterate over my objects (using Object.keys) that "there is no index signature" are really, really... strange.

Alternatively, please document how to document generic (key-prop/value) objects when what I do is iterate over the keys. Without "any". Thus far I read the documentation to mean that Record<string, any> was _meant_ to do exactly that?

[patroza]

Not sure if this helps the case: const typedKeysOf = <T>(obj: T) => Object.keys(obj) as Array<keyof T>

[lll000111]

@patroza If that helps, and maybe in general anyway (I have not thought about wider implications), what if Object.keys and Reflect.ownKeys were declared that way (TS standard lib), instead of as Array<string | number | symbol> ?

As an experiment I changed the definition in lib.es2015.refelect.d.ts

from (PropertyKey is string | number | symbol)

function ownKeys(target: object): PropertyKey[];

to

function ownKeys<T extends object>(target: T): Array<keyof T>;

and everything is PERFECT now! I had to remove the .filter(isString) because that widens the type to string again, but that filter is not needed in this context.

So maybe the underlying TS-lib definition can be looked and any maybe changed?

[patroza]

@lll000111 yea I was also surprised that the standard library didn't implement it this way at least for Object.keys, but I suppose it may have backwards incompatibility implications.

[ahejlsberg]

@patroza For the reason Object.keys isn't typed as (keyof T)[], see https://github.com/microsoft/TypeScript/pull/12253#issuecomment-263132208.

[patroza]

@ahejlsberg makes sense, thanks for shedding light on it. We can constrain it ourselves with for example the helper I posted, when we need it. That's good enough for me.

[ahejlsberg]

@felix9 The distinction becomes evident when you consider both input (parameter) and output (function result) positions. Imagine that put1 and put2 returned the object passed to them:

function put1(bag: Bag): Bag {
  bag["z"] = 3;
  return bag;
}

function put2<T extends Bag>(baggish: T): T {
  baggish["z"] = 3;  // Error
  return baggish;
}

const p1: Point = put1({ x: 3 });  // Error
const p2: Point = put2({ x: 3 });

Following a call to put1 you now have a Bag and Bag is not assignable to Point. And because the argument has been promoted to Bag, the function is permitted to write to any property.

However, following a call to put2 you still have a Point. So, the type checker needs to restrict the function to only allow writing to properties that are known to exist in T. In other words, it should only allow assignments to T[K] where K is a keyof T or something constrained to keyof T. In effect, constraining a type parameter to Bag merely checks that T is assignable to Bag, but it doesn't imply that T is a Bag.

[bajtos]

(1)

In effect, constraining a type parameter to Bag merely checks that T _is assignable_ to Bag, but it doesn't imply that T _is a_ Bag.

Here is the part I find highly confusing:

• when I write class Foo extends Bag, it means Foo _is_ a Bag
• when I write function put<Foo extends Bag>, it DOES NOT mean that Foo _is_ a Bag

As I am reading the discussion above, it seems to me that {x: 3} should not be assignable to Bag type, because it does not have any indexer property to satisfy the Bag interface.

If that was the case, the the example put2({x: 3}) above would trigger a compilation error.

(2)
Now going back to foo example:

function foo<T extends Record<string, any>>(obj: T) {
    obj["s"] = 123;
    obj["n"] = "hello";
}

IMO, the code above is valid in the sense that it's ok to assign to s and n properties, because Record<string, any> does allow that. What I don't consider as correct is to call foo with such obj value that does not have indexer property mapping arbitrary key to any value.

I think a more correct definition of foo would say that the Record type has only keys of T, something along the following lines:

function foo<T extends Record<keyof T, any>>(obj: T) {
    obj["s"] = 123;
    obj["n"] = "hello";
}

Such code is triggering the following error for me: Element implicitly has an 'any' type because type 'Record<keyof T, any>' has no index signature.ts(7017), which is a good sign that we are doing something wrong here. Personally, I would expect a different error - Property 's' does not exist on type 'T'.

Because foo wants to work with s and n properties, I think it should list them in the Record template arguments.

function foo<T extends Record<keyof T | 's' | 'n', any>>(obj: T) {
    obj["s"] = 123;
    obj["n"] = "hello";
}

That way the compiler can verify:

• inside foo that we are accessing only those obj properties that exist on T
• code calling foo provides obj value that allows s and n properties

(3)
When I think about the clone function mentioned earlier in the discussion, I would rewrite it as follows:

function clone<T extends Record<keyof T, any>>(obj: T): T {
  const objectClone = {} as T;

  const ownKeys = Reflect.ownKeys(obj) as (keyof T)[];
  for (const prop of ownKeys.filter(isString)) {
    objectClone[prop] = obj[prop];
  }

  return objectClone;
}

This seems to work with TypeScript v3.5, although I am surprised that Reflect.ownKeys is returning string[] instead of (keyof T)[].

Thoughts?

[ahejlsberg]

As I am reading the discussion above, it seems to me that {x: 3} should not be assignable to Bag type, because it does not have any indexer property to satisfy the Bag interface.

That would one way to do it. However, it is very common to initialize map-like objects using object literals, and for an object literal we know the exact set of properties from which to compute an implicit index signature. So we permit the assignment for types originating in object literals.

When I think about the clone function mentioned earlier in the discussion, I would rewrite it as follows

I'd write it like this:

function clone<T extends object>(obj: T): T {
  const objectClone = {} as T;
  const ownKeys = Reflect.ownKeys(obj) as (keyof T)[];
  for (const prop of ownKeys) {
    objectClone[prop] = obj[prop];
  }
  return objectClone;
}

No need to use index signatures anywhere. And, to recap, for the reason Reflect.ownKeys is not typed as (keyof T)[], see https://github.com/microsoft/TypeScript/pull/12253#issuecomment-263132208.

[fatcerberus]

Just throwing my two cents in: In general I expect T extends SomeType (where SomeType is structural) to mean that T has at least the properties of SomeType, as that is what structural subtyping is all about, typically.

So my first instinct is that T extends Record<...> should likewise mean that T always has an index signature. But then I realize that this implies an index signature stands in for “all possible properties” which can’t be the case as this would be a bald faced lie: it’s physically impossible for an object to have “all possible properties”. So the best we can do is to say “it can have all possible properties”, in which case the current constraint behavior makes perfect sense.

[russelldavis]

I'm trying to understand this change, but having trouble with the current explanations. In particular, it's unclear why the change should apply specifically to a generic type T extends Bag and not to a regular non-generic Bag. For example, the "previously allowed crazy stuff" (from https://github.com/microsoft/TypeScript/issues/31661#issuecomment-497138929) is still allowed if you change function foo<T extends Record<string, any>>(obj: T) to function foo(obj: Record<string, any>). It's equally crazy in that context, no?

@ahejlsberg, you tried to address the distinction at https://github.com/microsoft/TypeScript/issues/31661#issuecomment-497474815, but it seems like that explanation has a similar problem. Your example shows that the new behavior prevents put2 from adding an extra (unknown) property and returning it as the original type (a Point). However, the following code compiles without any errors:

function put3(point: Point): Point {
  point["z"] = 3;
  return point;
}

const p3: Point = put3({ x: 3 });

So, again, why should the generic code be restricted more than the non-generic equivalent? I've been trying to unpack the following for a clue:

In effect, constraining a type parameter to Bag merely checks that T is assignable to Bag, but it doesn't imply that T is a Bag.

But again it's not clear how this is different from non-generic parameters -- if I pass a value to a non-generic parameter of type Bag, isn't it also just an assignability check? Isn't ~everything just an assignability check in a structural type system?

I think it will help a lot to see some specific examples of real-world errors this is intended to fix (if you only have time for a quick response, this would be the most helpful). So far, the compelling error cases I've seen (the "previously allowed crazy stuff" mentioned above) seem to be a result of the special behavior of Record<string, any> being assignable to any type.

Are there many errors you're seeing that don't involve that? If not, I would put forth an alternate proposal: for generic constraints, only ignore index signatures equivalent to Record<string, any>, and go back to the old behavior for all others.

This would hopefully make the language simpler and more consistent, and save @RyanCavanaugh from having to write several pages of documentation 😄 (would love to hear your take on this, Ryan, and/or Anders).

[russelldavis]

I would put forth an alternate proposal: for generic constraints, only ignore index signatures equivalent to Record<string, any>

Thinking about this a bit more, this still doesn't feel great in terms of consistency, though at least it would be a more targeted special case. A better, even more targeted change would be to not ignore any index signatures, but to ignore the "Record<string, any> is assignable to any type" rule when it appears as a generic constraint.

That would eliminate the "previously allowed crazy stuff" while preserving expected/consistent behavior everywhere else (including the code from the top of this issue). I'm curious how much existing code it would break (though it may also uncover new errors that even #30769 doesn't catch).

If anyone can point me to any context/history/motivation for the "Record<string, any> is assignable to any type" rule, that would also be helpful.

[fatcerberus]

So having thought about this some more, particularly in light of #31102, it turns out the aforementioned "crazy stuff" is still allowed, and for the same reason even. We've only closed the loophole for generics, but this is still fine:

type X = { [key: string]: number };
type Y = { pig: number, cow: number, ape: number };

let y: Y = { pig: 812, cow: 1208, ape: 128 };
let x: X = y;
x.whale = 9001;  // yeah, there's no way this is fitting on the elevator.
console.log(y);

As long as the above continues to work, I suspect we will never see the end of issues like #31808... ☹️

edit:
It turns out I didn't read very closely and @russelldavis above me already made the exact same point. Oops.

[fatcerberus]

But again it's not clear how this is different from non-generic parameters -- if I pass a value to a non-generic parameter of type Bag, isn't it also just an assignability check? Isn't ~everything just an assignability check in a structural type system?

I kind of agree with this, but then again I kind of don't. The difference is that in the generic case, you have a type variable T whose type you don't actually know. You can't do an actual structural comparison against it--all you know that it's a subset of its extends constraint (for whatever "subset" means structurally); there is no lower bound so this could in fact be very, very specific (it could even be never!). That seems subtly different from the non-generic case, where you do know the exact type, and the unsoundness is merely a consequence of not enforcing contravariance. In other words, the assignment of y to x should be an error only because the source object is mutable, whereas even in that case it still would be a valid type to use for T. At least, that's my thinking, anyway.

That being said, it does still look like inconsistency from an end-user point of view, and that's probably all that matters.

[russelldavis]

@fatcerberus thanks for the input. I'm not 100% sold on the reasoning without seeing concrete examples, but either way I agree with your final point. If there are practical reasons for the change, we should focus on those.

[seancheung]

What about this?

target can be indexed while source cannot?

function deepMerge<T1 extends Record<string, any>, T2 extends Record<string, any>>(
  source: T1,
  target: T2
): T1 & T2 {
  Object.keys(target).forEach(key => {
    if (source[key] == null) {
      source[key] = target[key];
    } else if (typeof source[key] === 'object') {
      if (typeof target[key] === 'object') {
        deepMerge(source[key], target[key]);
      } else {
        source[key] = target[key];
      }
    } else {
      source[key] = target[key];
    }
  });

  return source as any;
}

[haohello]

Any update on the deepMerge type implementation? I'm also stuck on this.

[haohello]

How about this one:

export const deepMerge = <T extends object = object, U extends object = object>(a: T, b: U) => {
    let result = Object.assign(a, b)
    for (const key in a) {
        if (!a[key] || (b.hasOwnProperty(key) && typeof b[(key as unknown) as keyof U] !== 'object')) continue

        Object.assign(result, {
            [key]: Object.assign(a[key], b[(key as unknown) as keyof U])
        })
    }

    return result
}

[BBGONE]

Inside deepMerge it's better to cast all to any. No need to strongly type it. We need to know the inputs and the output type, and what is inside is a black box. Compromise.

[Amourspirit]

Here is what I came up with. TypeScript Playground
````ts
class Util {
public static DeepCopy(target: T): T {
if (target === null) {
return target;
}
if (target instanceof Date) {
return new Date(target.getTime()) as any;
}
if (target instanceof Array) {
const cp = [] as any[];
(target as any[]).forEach((v) => { cp.push(v); });
return cp.map((n: any) => Util.DeepCopy(n)) as any;
}
if (typeof target === 'object' && target !== {}) {
const cp = { ...(target as { [key: string]: any }) } as { [key: string]: any };
Object.keys(cp).forEach(k => {
cp[k] = Util.DeepCopy(cp[k]);
});
return cp as T;
}
return target;
}
public static MergeDefaults(defaults: U, ...opt: T[]) {
// Example: https://jsfiddle.net/6p4rzmxo/1/
let result = Util.DeepCopy(defaults);
Util.DeepMerge(result, ...opt);
return result;
}
public static IsObject(obj: any) {
return (obj && typeof obj === 'object' && !Array.isArray(obj));
}
public static DeepMergeGeneric(target: T, ...sources: U[]): any {
return Util.DeepMerge(target, ...sources);
}
public static DeepMerge(target: T, ...sources: any[]): any {
if (!sources.length) {
return target;
}
const source = sources.shift();

    if (Util.IsObject(target) && Util.IsObject(source)) {
    for (const key in source) {
        if (Object.prototype.hasOwnProperty.call(source, key)) {
        const el = source[key];
        if (Util.IsObject(el)) {
            if (!(target[(key as unknown) as keyof T])) {
            Object.assign(target, { [key]: {} });
            };
            const newTarget = target[(key as unknown) as keyof T];
            Util.DeepMerge(newTarget as Object, el);
        } else {
            Object.assign(target, { [key]: source[key] });
        }
        }
    }
    }
    return Util.DeepMerge(target, ...sources);
}

}
````