Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding indexer to global Object makes other interfaces incompatible with Object #835

Closed
csnover opened this issue Oct 6, 2014 · 6 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@csnover
Copy link
Contributor

csnover commented Oct 6, 2014

Between 0.9.1 and 0.9.5 the explicit indexer was removed from Object. Between 1.0 and 1.1, code that adds the indexer back to Object causes type comparison failures:

interface Object {
    [key:string]:any;
}

interface Foo {}

function a(b:Object) {}

var foo:Foo;
a(foo); // error TS2345: Argument of type 'Foo' is not assignable to parameter of type 'Object'.

Putting an indexer on Foo makes it work.

This is probably an OK error since adding back the indexer like this to Object should probably be considered an anti-pattern, and since in ES5 you can create inheritance patterns that don’t extend from Object, but it should at least be documented as a known breaking change.

@danquirk
Copy link
Member

danquirk commented Oct 6, 2014

I think this is just a bug and we aren't considering the indexer extensions to Object when checking the apparent type of foo.

@danquirk danquirk added the Bug A bug in TypeScript label Oct 6, 2014
@RyanCavanaugh
Copy link
Member

This is seems to be according to the spec -- the augmented apparent type of Foo (3.8.1) does not include the index signatures of the Object type, only properties are added to the apparent type. It's possible this an oversight; following up.

@RyanCavanaugh RyanCavanaugh added By Design Deprecated - use "Working as Intended" or "Design Limitation" instead and removed Bug A bug in TypeScript labels Oct 6, 2014
@RyanCavanaugh
Copy link
Member

Confirmed by design. The ideal fix would be to replace b: Object with b: { } -- they're basically equivalent except for this deviation.

We can talk about other fixes if you have more detailed use cases. Were you using the indexer to avoid noImplicitAny warnings?

@csnover
Copy link
Contributor Author

csnover commented Oct 6, 2014

Were you using the indexer to avoid noImplicitAny warnings?

I suspect this is one of the reasons that it was used; I am still looking through to figure out and will update with more information as I have it.

@csnover
Copy link
Contributor Author

csnover commented Oct 6, 2014

@RyanCavanaugh Here’s one pattern that adding the indexer to Object was “fixing”:

interface Foo {
  // ...
}

function mixin(target:Foo, source:Foo):Foo {
  for (var key in source) {
    if (key === 'somethingNotToCopy') {
      continue;
    }

    // error TS7017: Index signature of object type implicitly has an 'any' type.
    target[key] = source[key];
  }

  return target;
}

1.0 would give TS7017 also until putting the indexer on Object; in 1.1 it doesn’t matter, it will complain anyway. Using explicit any type for the parameters, or coercing inside the method, would “solve” the problem, but I wouldn’t feel great about it.

@csnover
Copy link
Contributor Author

csnover commented Oct 7, 2014

Here are some other patterns triggering the new(ish) noImplicitAny behaviour:

Feature detect something that isn’t in lib.d.ts:

has.add('dom-pointerevents', Boolean(window['PointerEvent']));

This one could use in, or it could do some interface merge.

Late binding methods:

function lateBind(target:Object, method:string):(...args:any[]) => any {
  return function ():any {
    return target[method].apply(target, arguments);
  };
}

This one would need to define target as any, which would fail at runtime if someone passed a non-object, like the first example.

I think my remaining examples are all pretty much the same; foo[bar ? 'a' : 'b'], this[foo]

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

3 participants