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

Index signature & iterators #2862

Closed
jbondc opened this issue Apr 22, 2015 · 14 comments
Closed

Index signature & iterators #2862

jbondc opened this issue Apr 22, 2015 · 14 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@jbondc
Copy link
Contributor

jbondc commented Apr 22, 2015

interface Files {
    [key: number]: SomeFile
    length: number
}
var data: Files = []
// no complaints
for (var i in data) {
}

// Type 'Files' is not an array type or a string type
for (let file of data) {
}

I'm not sure what is the right move here. Files has an index but that doesn't mean it's iterable/enumerable. Think the check should just be removed.

@JsonFreeman
Copy link
Contributor

This code is reasonable in ES5, but in ES6 you would have to declare it Iterable (which I think you allude to later). I would be fine with ES5 type check allowing iteration on ArrayLike things (types with a numeric indexer and a length). This could only be allowed for ES5 though, not ES3 or ES6.

@RyanCavanaugh
Copy link
Member

This could only be allowed for ES5 though, not ES3 or ES6.

😨

@JsonFreeman JsonFreeman added the Suggestion An idea for TypeScript label Apr 22, 2015
@JsonFreeman
Copy link
Contributor

@jbondc, those cannot work in ES5 because the downlevel emit for ES5 depends on the object having a length and an indexer. Since neither a length nor an indexer is present in r or b, it has to be an error.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 22, 2015

one issue to keep in mind that strings cannot be treated as an array if it has surrogate pairs. e.g.: `

"𠮷".length === 2;
for(var i of "𠮷) {
    charCount ++;
}
charCount === 1;

@JsonFreeman
Copy link
Contributor

Oh yeah, so in effect, it is not even correct to allow strings in ES5

@JsonFreeman
Copy link
Contributor

Actually, does ES5 even have surrogate pairs? Do they even support characters that involve surrogate pairs?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 22, 2015

Actually, does ES5 even have surrogate pairs? Do they even support characters that involve surrogate pairs?

There is nothing wrong with surrogate pairs, the issue is when you iterate on them.

Oh yeah, so in effect, it is not even correct to allow strings in ES5

We can not get the correct semantics 100% of the time without a helper function that can understand surrogate pairs. I do not know if that means we should/should not consider enabling strings though.

@JsonFreeman
Copy link
Contributor

I think we can still support iterating over strings. Surrogate pairs are a pretty marginal case.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 22, 2015

i agree :)

@JsonFreeman
Copy link
Contributor

Your second and third examples compile in TypeScript when target is ES6. But they will not compile in ES5 because neither Symbol nor Array.from is defined in ES5. This is an accurate representation of the runtime behavior.

@JsonFreeman
Copy link
Contributor

Oh I see what you mean now. If data, were iterable in ES6, we would emit:

for (let file of data) { }

But if it is only ArrayLike, we would emit

for (let file of Array.from(data)) { }

However, this violates a key principle of TypeScript emit, which is that we do not change emit based on type information. The reason is that the view of the types in your program is not perfect, and it is merely descriptive, a glimpse of what will happen at runtime. But it is intended to be used as a guide to the user, and should not impact the behavior of your program.

@saschanaz
Copy link
Contributor

Can we implement WebIDL-style iterable keyword to enable this?

interface NodeList iterable<Node> {}

This code in ES6 will be same with:

interface NodeList {
    [Symbol.iterator](): IterableIterator<Node>
}

... while still allowing for-of loop in ES5 without Symbol support.

@saschanaz
Copy link
Contributor

@jbondc My thought is we may add an internal flag for iterable interfaces in ES5 while not changing emit.

for (let node of document.querySelectorAll(".foo .bar")) {} // allowed

let nodeList: NodeList;
nodeList = document.querySelectorAll(".baz"); // okay
nodeList = <{ [key: string]: Node; length: number }>{}; // no `iterable` flag, not allowed.

@saschanaz
Copy link
Contributor

Your first example with iterable flag will be:

interface Files iterable<SomeFile> {
    [key: number]: SomeFile;
    length: number;
}
var data: Files = [] // okay, as arrays are iterable
for (let file of data) {} // data is iterable, so okay

PS: for-of loop in this case should require all three iterable flag, index signature and length property in ES5 mode.

@mhegazy mhegazy added the In Discussion Not yet reached consensus label Dec 10, 2015
@jbondc jbondc closed this as completed Dec 20, 2015
@RyanCavanaugh RyanCavanaugh added Too Complex An issue which adding support for may be too complex for the value it adds and removed In Discussion Not yet reached consensus labels Dec 21, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

5 participants