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

3.5.1 Regression, Generics, Indexing Constraints Not Effective #31808

Closed
zaripych opened this issue Jun 7, 2019 · 8 comments
Closed

3.5.1 Regression, Generics, Indexing Constraints Not Effective #31808

zaripych opened this issue Jun 7, 2019 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@zaripych
Copy link

zaripych commented Jun 7, 2019

TypeScript Version: > 3.5.1, 3.6.0-dev.20190606

Search Terms:
Type 'string' cannot be used to index type for generic type

Code

function set_extends<
  T extends {
    [key: string]: string;
  }
>(obj: T, key: string, value: string) {
  obj[key] = value; // <-- error TS2536: Type 'string' cannot be used to index type 'T'.
}

const set_union = <T>(
  obj: T & { other: string; [key: string]: string },
  key: string,
  value: string
) => {
  obj[key] = value; // <-- error TS2536: Type 'string' cannot be used to index type 'T & { [key: string]: string; other: string; }'.
};

function set_works<T>(obj: T, key: keyof T, value: T[keyof T]) {
  obj[key] = value;
}

Expected behavior:
No compilation errors.

Actual behavior:
Compilation errors for set_extends:
Type 'string' cannot be used to index type 'T'
Compilation errors for set_union:
Type 'string' cannot be used to index type 'T & { [key: string]: string; other: string; }'

NOTE: The TypeScript version 3.4.5 gives no compilation errors

Playground Link:

NOTE: Not sure the playground is running latest TypeScript yet, so cannot be reproduced there

https://www.typescriptlang.org/play/index.html#src=function%20set_extends%3C%0D%0A%20%20T%20extends%20%7B%0D%0A%20%20%20%20%5Bkey%3A%20string%5D%3A%20string%3B%0D%0A%20%20%7D%0D%0A%3E(obj%3A%20T%2C%20key%3A%20string%2C%20value%3A%20string)%20%7B%0D%0A%20%20obj%5Bkey%5D%20%3D%20value%3B%0D%0A%7D%0D%0A%0D%0Aconst%20set_union%20%3D%20%3CT%3E(%0D%0A%20%20obj%3A%20T%20%26%20%7B%20other%3A%20string%3B%20%5Bkey%3A%20string%5D%3A%20string%20%7D%2C%0D%0A%20%20key%3A%20string%2C%0D%0A%20%20value%3A%20string%0D%0A)%20%3D%3E%20%7B%0D%0A%20%20obj%5Bkey%5D%20%3D%20value%3B%0D%0A%7D%3B%0D%0A%0D%0Afunction%20set_works%3CT%3E(obj%3A%20T%2C%20key%3A%20keyof%20T%2C%20value%3A%20T%5Bkeyof%20T%5D)%20%7B%0D%0A%20%20obj%5Bkey%5D%20%3D%20value%3B%0D%0A%7D

Related Issues:
#21760

@fatcerberus
Copy link

fatcerberus commented Jun 7, 2019

#31661 (comment)

Using keyof T instead of string should work.

@fatcerberus
Copy link

Basically - an index signature constraint doesn't actually guarantee T has an index signature, so T[string] is not type safe. 3.4 and earlier just didn't catch it.

@russelldavis
Copy link
Contributor

Per #31661 (comment), I disagree with the assessment that it's not type safe -- at least, not any more unsafe than a non-generic equivalent. I'd love to see an example to the contrary. In the meanwhile, I agree this is a regression that should be fixed.

@zaripych
Copy link
Author

zaripych commented Jun 7, 2019

@fatcerberus Ok. What if the property type in my case is not just a string, but something more complicated that requires manipulation from within the function itself - the whole reason I introduced the constraint.

Here is another similar problem:

type Signature = () => void;

function decorate<
  T extends {
    [key: string]: Signature;
  }
>(obj: T) {
  Object.keys(obj).forEach(key => {
    const previous = obj[key];
    obj[key] = () => { // <-- error TS2536: Type 'string' cannot be used to index type 'T'.
      console.log('do something else first');
      previous();
    };
  });
}

decorate({
  key: () => {
    console.log('do something');
  },
});

This used to compile before and now it doesn't. If I change the forEach like you suggested:

type Signature = () => void;

function decorate<
  T extends {
    [key: string]: Signature;
  }
>(obj: T) {
  Object.keys(obj).forEach((key: keyof T) => {
    const previous = obj[key];
    obj[key] = () => { // <-- error TS2322: Type '() => void' is not assignable to type 'T[keyof T]'.
      console.log('do something else first');
      previous();
    };
  });
}

decorate({
  key: () => {
    console.log('do something');
  },
});

I get another kind of error.

Maybe somebody can suggest how to fix the latter?

I was only able to make it work by externalising the function that transforms the value:

type Signature = () => void;

function decorate<
  T extends {
    [key: string]: Signature;
  }
>(obj: T, transform: (previous: T[keyof T]) => T[keyof T]) {
  Object.keys(obj).forEach((key: keyof T) => {
    const previous = obj[key];
    obj[key] = transform(previous);
  });
}

decorate(
  {
    key: () => {
      console.log('do something');
    },
  },
  previous => {
    return () => {
      console.log('do something else first');
      previous();
    };
  }
);

But the purpose of decorate in my case wasn't to be that generic. And when you try to move the transform function inside as a local variable - it stops compiling again.

@russelldavis
Copy link
Contributor

russelldavis commented Jun 7, 2019

@zaripych in your particular example, you don't need to be using generics at all. Just use a regular type, and the errors will go away:

function decorate(obj: {[key: string]: Signature}) {

For an explanation, see this lint rule that would have caught this for you: https://github.com/microsoft/dtslint/blob/master/docs/no-unnecessary-generics.md

(I still believe this to be a regression, since there are similar cases where generics do need to be used.)

@zaripych
Copy link
Author

zaripych commented Jun 7, 2019

@russelldavis Thanks, I think this is second time I tried to simplify my real code and missed the point. I cannot simply remove the generic as it also returned from the function and it is even more complex. That might be really unnecessary though. I will try to simplify things and spend time in the direction of removing the generic parameter before I can get back with additional feedback.

@garethl
Copy link

garethl commented Jun 10, 2019

I'm also seeing this, and cannot remove the generic argument due to my code needing to return a function that takes in the generic argument.

I've come up with a minimal reproduction of this:

function testFunction<T extends {[key: string]: string}>(value: T): void {
    // error TS2536: Type '"test"' cannot be used to index type 'T'.
    value["test"] = "test";
}

My workaround currently is inside the function to cast it using as to {[key: string]: string}, while feeling pretty hacky, works.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jun 25, 2019
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants