Solve the for-in / indexer mess

Vladimir Matveev (@vladima)

/cc Vladimir Matveev (@vladima) Anders Hejlsberg (@ahejlsberg) Bill Ticehurst (@billti)

Motivating Examples

var arr = [1, 2, 3];

This should be an error (k is always a string):

for(var k in arr) {
  if(k === 0) { ... }
}

This should not be an error, even under no implicit any:

for(var k in arr) {
  if(arr[k] === 0) { ... }
}

This should somehow? not be an error, even though it involves a coercion we would normally disallow

for(var k in arr) {
  if (k > 0) { ... }
}

If there was some way of communicating that your string type was actually the domain of strings whose text was numeric (i.e. ToString(ToNumber(text) === text), that would possibly go a long way here.

Anders Hejlsberg (@ahejlsberg)

I had an offline discussion with Anders Hejlsberg (@ahejlsberg) about this. I think we have a proposal. given:

for (var x in a) {...}

if type of a has a numeric index signature, type of x is NumericString
otherwise, typeof x is string

Where NumericString is an builtin type that can not be named, and is only given to for..in indexers. It is identical to string for all type system operations except that it is treated as number when indexing. e.g.:

var a = [1, 2, 3];
for (var x in a) {
    y = a[x];  // y is number
    z = x + 1 // z is string;
}

An alternative is to check whether the expression used to index an array is exactly an identifier declared as a for-in variable for an array that has a numeric indexer. This avoids having a new type and, importantly, avoids having to "erase" that type when it becomes in inferred type of a variable. I will try to work on it and put up a PR.

You mean if it only has a numeric index signature? Or even if it has a string index signature as well?

Daniel Rosenwasser (@DanielRosenwasser)

Daniel Rosenwasser (@DanielRosenwasser) you are correct. it has to have only a numeric index signature.

An object can satisfy a numeric index signature type and still have other non-numeric properties:

var obj: {[n: number]: string};
obj = { 0: 'a', 1: 'b', 2: 'c', length: 3 };
for (var k in obj) {
    console.log(k, obj[k].toUpperCase()); // This throws when k === 'length'
}

Even on a plain array, for/in may include properties that have been added to Array.prototype.

jeffreymorlan

jeffreymorlan Perhaps a better check is to see if the object being for-in'ed is array-like (specifically, if it is assignable to any[]). That is the same check we apply to a spread expression.

One library I'm using returns array-like objects that inherit from Array.prototype but add additional (enumerable) methods that will show up in for/in, making it unsafe to use the for/in keys as indexes.

There are so many ways that for/in on array-likes may return properties other than array indexes...

extra properties on the array object itself
extra properties on prototype inheriting from Array
some library "conveniently" adding extra properties onto Array.prototype itself (MooTools is an infamous example of this)
old JS implementations where .length was enumerable

...not to mention the other surprising behaviors (indexes are strings instead of numbers, indexes not guaranteed to be in numeric order pre-ES6), that for/in on an array-like should just be considered a bug. At any rate it shouldn't be given special treatment from the type system to pretend that it's safe.

jeffreymorlan

jeffreymorlan Completely agree for-in is a messed up construct--but we don't get to change that. The objective is to provide the best possible type checking of for-in as it exists, and unquestionably it is a common pattern to use for-in with arrays and expect indexing with the for-in variable to produce the elements of the array. I agree that there are situations where non-numeric property names may appear because applications or libraries have added extra properties to arrays, but we have to balance that against existing (and more common) code that just uses plain JavaScript arrays. The operating principle here is to find the best possible compromise. I think the changes in #6379 bring us closer to that.

it is a common pattern to use for-in with arrays and expect indexing with the for-in variable to produce the elements of the array

It's a common mistake; all the more reason not to allow it in TS, so people can find places where they have made that mistake and replace it with the correct for/of, .forEach, or good-old-fashioned for (var i = 0; i < arr.length; i++).

Monkey patching of Array.prototype isn't just something obscure that you'll never see in the real world (unfortunately). According to http://w3techs.com/technologies/overview/javascript_library/all MooTools is used by 3.8% of websites, and Prototype by 2.2%. It's a controversial practice; I don't like it myself, but many people take the moderate position that it's fine for polyfilling newer methods like .filter for older browsers - which still breaks for/in. On the other hand, for/in on arrays is nearly universally castigated; google for "for in array" and results # 1, 3, 4, and 7 are all about why you shouldn't do it.

Even TypeScript itself used to add properties to Array.prototype that would globally break for/in on arrays. From TypeScript 1.3 src/services/syntax/syntaxList.ts:

module TypeScript.Syntax {
    ...
    Array.prototype.kind = function () { ... }
    Array.prototype.separatorCount = function (): number { ... }
    Array.prototype.separatorAt = function (index: number): ISyntaxToken { ... }
    ...
}

jods (@jods4)

jods (@jods4) I understand and agree but I would like to have some 'string_index_as_number' type which allows only to be used as indexer and disallow everything else.
So this code will work but typeof will be string.

Shouldn't this work ?

⚠️ ⚠️ ⚠️ No! ⚠️ ⚠️ ⚠️

interface A {
    [index:number]:number;
}

var a : A = {1:1,2:2};
for (var x in a) {
    var y = x + 2; // y: '12', '22'
}

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh) I get your point.
I just want to understand what is the better way to fix my code from older version.

I have something like this:

interface SubStates {
 [subStateId: number]: SubState;
}

class State {
 subStates: SubStates = {};
}

interface States {
 [id: number]: State;
}

// Map like object
states: States = {};

// Recursive traversal
function f(id: number) {
  var subStates = states[id].subState;
  if (subStates) {
    for (var subState in subStates) {
     f(subState);
   }
  }
}

Since I used the 'id' only as indexer it worked without any problem.
The only option to fix it currently is to create f2 which receives string and does the recursion.

I don't understand what doesn't work about your sample.

The issue is remove expects a number.

here are the options:

remove takes number|string
remove(+subState);
remove(subState as any);

define subState before hand:

if (subStates) {
    var subState: any;
    for (subState in subStates) {
        remove(subState);
    }
}

Mohamed Hegazy (@mhegazy)

Sorry, my bad.
I meant f(subState) of course or any function expecting number as Mohamed Hegazy (@mhegazy) mentioned.

the last would be my recommendation, as it has no runtime impact (vs +subState), and easier to read.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) Good idea. I totally forgot that I can put a variable outside of the for loop.
Never encountered in a real usage for this :)

if remove is expecting a number, you really ought to give it one (+subState) rather than hoping whatever its implementation is just happens to work when given a string

Thank you all.

Navigation Menu

Repository navigation

Solve the for-in / indexer mess #6346

Motivating Examples

10 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Solve the for-in / indexer mess #6346

Description

Motivating Examples

Activity

DanielRosenwasser commented on Jan 5, 2016

mhegazy commented on Jan 5, 2016

ahejlsberg commented on Jan 5, 2016

DanielRosenwasser commented on Jan 5, 2016

mhegazy commented on Jan 5, 2016

jeffreymorlan commented on Jan 6, 2016

ahejlsberg commented on Jan 6, 2016

jeffreymorlan commented on Jan 8, 2016

ahejlsberg commented on Jan 8, 2016

jeffreymorlan commented on Jan 8, 2016

10 remaining items

NN--- commented on Mar 9, 2016

RyanCavanaugh commented on Mar 9, 2016

NN--- commented on Mar 9, 2016

RyanCavanaugh commented on Mar 9, 2016

mhegazy commented on Mar 9, 2016

NN--- commented on Mar 9, 2016

mhegazy commented on Mar 9, 2016

NN--- commented on Mar 9, 2016

RyanCavanaugh commented on Mar 9, 2016

NN--- commented on Mar 9, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions