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

Why does { foo: T } not satisfy { [key:string]: T }? #6041

Closed
MrHen opened this issue Dec 10, 2015 · 17 comments
Closed

Why does { foo: T } not satisfy { [key:string]: T }? #6041

MrHen opened this issue Dec 10, 2015 · 17 comments
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@MrHen
Copy link

MrHen commented Dec 10, 2015

Various libraries I use want to see dictionary "loops" that take some number of key/value pairs and operate on them. E.g.:

async.auto<any>({
   'foo': (cb) => getFoo(cb),
   'bar': ['foo', (cb, results) => getBar(results.foo)]
}, (err, results) => { ... });

The typings file for async likes to define helper dictionary types like this:

interface Dictionary<T> { [key: string]: T; }

Periodically, I need to create these key/value pairs conditionally and on the fly but the compiler yells at me if I try to send it in without explicitly typing my temporary object:

var block = {
    'foo': (cb) => getFoo(cb),
    'bar': ['foo', (cb, results) => getBar(results.foo)
}

if (flag) {
    block.foo = getOtherFoo(cb);
}

async.auto<any>(block, (err, results) => { ... });

block now technically satisfies the { [key: string]: T; } because it is an object with two keys, both strings. TypeScript will still yell at me for it, though, unless I explicit add a similar dictionary type to block. I, therefore, find myself putting this dictionary type all over the place just to satisfy what seems to be an arbitrary distinction between what is or is not a key:string.

This comes up enough that I find the type bloat irritating and it also makes it a requirement to only access block.foo as block['foo'].

@MrHen
Copy link
Author

MrHen commented Dec 10, 2015

After browsing the issues a bit deeper, this may be the same thing as #5683.

@sandersn
Copy link
Member

What is the error you get? It looks to me that you'll get a different type inferred for the values of block -- the first is a function and the second is a tuple. I would expect Typescript to complain that it can't infer a type for T.

@ahejlsberg
Copy link
Member

@MrHen Can you post a repro that shows the exact issue?

@ahejlsberg ahejlsberg added the Needs More Info The issue still hasn't been fully clarified label Dec 11, 2015
@MrHen
Copy link
Author

MrHen commented Dec 11, 2015

@sandersn, @ahejlsberg Here is a Playground link. The error I'm used to seeing is:

Argument of type '{ 'foo': (cb: any) => void; 'bar': (cb: any) => void; }' is not assignable to parameter of type 'Dictionary<{}>'.
Index signature is missing in type '{ 'foo': (cb: any) => void; 'bar': (cb: any) => void; }'.

Turns out an important difference is whether I explicitly use <any> on the function call which I had accidentally included above:

async.auto<any>(block, (err, results) => { ... }); // works
async.auto(block, (err, results) => { ... }); // error

I suppose I can use the <any> workaround for now.

@joeskeen
Copy link

Here is another (simplified) example of what I ran into when using Angular's $http (Playground link):

interface HttpHeaders {
    [key: string]: string
}

interface HttpService {
    <T>(path: string, method: string, headers: HttpHeaders): PromiseLike<T>;
}

let httpService: HttpService;
//this works:
httpService('', '', {
    'Content-Type': 'application/x-www-form-urlencoded'
});
//this does not:
const headers = {
    'Content-Type': 'application/x-www-form-urlencoded'
};
httpService('', '', headers); 

It results in the same error:

Argument of type '{ 'Content-Type': string; }' is not assignable to parameter of type 'HttpHeaders'. Index signature is missing in type '{ 'Content-Type': string; }'.

@MrHen
Copy link
Author

MrHen commented Dec 22, 2015

@RyanCavanaugh That's a good explanation for what happens but it's still kind of irritating because of how many libraries take dictionary-esque parameters.

From the SO example:

function c(): StringMap {
   var result = { a: "1" };
   return result; // Error - result lacks index signature, why?
}

It is really annoying to add explicit index signatures to variables like result and many of the typings files I consume don't bother to export their types. This is reasonable:

function c(): StringMap {
   var result: StringMap = { a: "1" };
   return result;
}

But this gets old:

function c(): StringMap {
   var result: {[key: string]: string} = { a: "1" };
   return result;
}

Especially for libraries like async where the types can get weird.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Needs More Info The issue still hasn't been fully clarified labels Dec 22, 2015
@RyanCavanaugh
Copy link
Member

I don't know what the answer is, but it's clear this is becoming really annoying as definition file authors provide increasingly precise type definitions (I think two years ago, we would have seen someone write :any instead of : { [headerName: string]: string }).

@RyanCavanaugh
Copy link
Member

Proposing the following change: If we are trying to assign S to T and T has a string index signature, S does not need to have a string index signature if all its known properties are assignable to the index signature type.

Pre-buttal: What about aliasing? Well, we already have this hole, which is basically equivalent:

let a = { x: 4, y: 'oops' };
let b: { x: number } = a;
let c: { x: number; y?: number } = b;
c.y.toFixed(); // Runtime error

Informally, we can choose to think about the indexer [k: string] T as being equivalent to "An optional property of type T for every possible name".

master...RyanCavanaugh:inferStringIndexers#diff-8b210ad15fe8058f8cadce2a5a0c71ccR1

@ahejlsberg
Copy link
Member

@RyanCavanaugh What about cases like this?

let x: {};  // x can be any object
x = { a: 10, b: true };
let stringMap: { [x: number]: string } = x;  

Effectively the {} type would completely defeat index signature checking because it has no known properties.

@RyanCavanaugh
Copy link
Member

@ahejlsberg I see that as being entirely analogous to this, which we have always allowed

let x: {};
x = { n: 3 };
let y: { n?: string } = x;

@MrHen
Copy link
Author

MrHen commented Jan 20, 2016

👍 for the proposal. That should address my particular issue.

@RyanCavanaugh
Copy link
Member

I ran into this, again.

Let's say I want to say "You can provide me an object, but all properties of that object must be functions of one argument":

interface Foo {
  [x: string]: (n: any) => void;
}

Naturally I implement this with an internal module:

namespace MyFoo {
  export function fn1(x: number) { }
  export function fn2(x: string) { }
  export function fn3(x: {}, y: {}) { } // oops
}

// Desired: error about fn3 having an invalid type
// Actual: complaint about the index signature being missing
let x: Foo = MyFoo;

So I try to fix it by module merging, which doesn't work

// Does nothing useful
interface MyFoo {
  [x: string]: (n: any) => void;
}

Then I try to fix it by declaring an index signature, which doesn't work either

module MyFoo {
  // Syntax error
  [x: string]: (n: any) => void;
}

As far as I can tell, there's no way to add the index signature to the namespace. So in this case the namespace is worse than an object literal, because I could at least contextually type the object literal to get the signature in.

How about having a notion of an optional index signature which doesn't require the index signature to be declared, but does enforce that declared properties are assignable to the index signature type? That would have only the same holes as our existing optional property rules.

@MrHen
Copy link
Author

MrHen commented Feb 7, 2016

How about having a notion of an optional index signature which doesn't require the index signature to be declared, but does enforce that declared properties are assignable to the index signature type? That would have only the same holes as our existing optional property rules.

That seems good. One context to consider is that libraries like async or lodash like to take "collections" which can be either Objects or Arrays with the expectation that all elements match a particular type. What they really want is a way to say, "all properties resolve to T" and "takes Objects and/or Arrays."

E.g., async.parallel and lodash.map.

async.parallel({
   'foo': (cb) => getFoo(cb),
   'bar': (cb) => getBar(cb)
}, (err, results) => { ... });

async.parallel([
   (cb) => getFoo(cb),
   (cb) => getBar(cb)
], (err, results) => { ... });

_.map([4, 8], (n) => n*n);
_.map({ 'a': 4, 'b': 8 }, (n) => n*n);

The typings files for these will often have to jump through all sorts of hoops. Here is _.map:

map<T, TResult>(
    collection: List<T>,
    iteratee?: ListIterator<T, TResult>,
    thisArg?: any
): TResult[];

map<T extends {}, TResult>(
    collection: Dictionary<T>,
    iteratee?: DictionaryIterator<T, TResult>,
    thisArg?: any
): TResult[];

For completeness, here are the helper interfaces:

interface ListIterator<T, TResult> {
    (value: T, index: number, collection: List<T>): TResult;
}

interface DictionaryIterator<T, TResult> {
    (value: T, key?: string, collection?: Dictionary<T>): TResult;
}

...

// Common interface between Arrays and jQuery objects
interface List<T> {
    [index: number]: T;
    length: number;
}

interface Dictionary<T> {
    [index: string]: T;
}

If I am understanding your suggestion, it would alter the List<T> and Dictionary<T> interfaces to something like:

interface List<T> {
    [index?: number]: T;
    length: number;
}

interface Dictionary<T> {
    [index?: string]: T;
}

But it would be nice to make the index type optional, too. The maintainers for the lodash or async typings may care but I don't. All I want is either of these:

interface Collection<T> {
    [index?:string|number]: T;
}

interface Collection<T> {
    [index?:any]: T;
}

So I can do this:

iterator<T, TResult>(
    collection: Collection<T>,
    iteratee?: CollectionIterator<T, TResult>,
    thisArg?: any
): TResult[];

_.map([4, 8], (n) => n*n);
_.map({ 'a': 4, 'b': 8 }, (n) => n*n);

Or this (which is the example that I originally filed):

interface Collection<T> { [index?:string|number]: T; }
interface AsyncCallback<T> { (err: Error, result: T): void; }

iterator<T>(
    collection: Collection<AsyncCallback<T>>,
    callback: AsyncCallback<Collection<T>>)
): void;

var block = {
    'foo': (cb) => getFoo(cb),
    'bar': (cb) => getBar(cb)
}

if (flag) {
    block.foo = (cb) => getOtherFoo(cb);
}

iterator<Foo|Bar>(block, (err, results) => { ... }); // results -> { 'foo': OtherFoo, 'bar': Bar }

var tasks = [
    (cb) => getFoo(cb),
    (cb) => getBar(cb)
]

iterator<Foo|Bar>(tasks, (err, results) => { ... }); // results -> [ Foo, Bar ]

@ahejlsberg
Copy link
Member

@RyanCavanaugh You made this comment:

Informally, we can choose to think about the indexer [k: string] T as being equivalent to "An optional property of type T for every possible name".

Thinking about it some more, I think you're right. I'll experiment a bit with changing our assignability rules such that a type S is assignable to a type T with an index signature if all known properties in S are assignable to T's index signature.

@AlexGalays
Copy link

Correct me If I'm wrong but I think this convenient transformation was only added at the value level, not the type level.

I still can't do :

interface Bla<T extends { [key: string]: number }> {

}

interface Options {
   someKey: 33
}

Then later use Bla<Options> even though this interface respects the constraint.
Having to add an index signature to all the options objects is annoying.

@SamVerschueren
Copy link

SamVerschueren commented Mar 22, 2017

This doesn't work either

interface Foo {
     foo: string;
}

const foo: {[key: string]: any} = {foo: 'bar'};

const test: Foo = foo;

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants