Map.keys() returns empty object (4.1.1)

[RobAtticus]

Calling keys() on a Map returns an empty object rather than the keys of the Map:

$ node --version
v4.1.1
$ node
> const x = new Map();
undefined
> x.set("a", 1);
Map { 'a' => 1 }
> x.keys();
{}

This works in Chrome45 which is using a similar v8 (V8 4.5.103.35):

> const x = new Map();
undefined
> x.set("a", 1);
Map {"a" => 1}
> x.keys();
MapIterator {"a"}

[vkurchatkin]

It's not an empty object, it's just how console represents iterator object

[RobAtticus]

Ah I see, I also realize I was using the wrong paradigm to iterator over it which is why it seemed empty to me.

This can be closed unless you think the representation should be updated.

[bnoordhuis]

Suggestions on how to improve it? I'm actually not sure what a good way is of detecting iterator objects apart from V8 specific magic like %_ClassOf(it) === 'Map Iterator'.

[vkurchatkin]

Well, Object.prototype.toString reports [object Map Iterator]

[bnoordhuis]

I don't think that's tamper resistant enough. With --harmony_tostring:

> var it = { [Symbol.toStringTag]: 'Map Iterator' }
undefined

> Object.prototype.toString.call(it)
'[object Map Iterator]'

> %_ClassOf(it)
'Object'

[vkurchatkin]

@bnoordhuis right, but the same applies to RegExp, Date, etc, and we really use this approach with them

[vkurchatkin]

Also we can use mirrors, they are already used to inspect promises

[bnoordhuis]

the same applies to RegExp, Date, etc

Yes, but that's a) a status quo appeal, and b) code that predates ES6.

we can use mirrors, they are already used to inspect promises

That's not a bad idea although it might be slow for large object graphs. I suppose we could test with v8::Value::IsMapIterator() and v8::Value::IsSetIterator() before creating the actual iterator mirror.

[vkurchatkin]

@bnoordhuis didn't know about those, if we have them, then why use mirrors? though we could probably take advantage of preview method to show iterators content, I bet it's what Chromium does

[bnoordhuis]

That sounds plausible. We use a promise mirror for the same reason, so we can print the status.

In case you're wondering why we're not using v8::Value::IsPromise() now: we can use Debug.ObjectIsPromise() in JS once we upgrade V8.

[thefourtheye]

Ben, why Debug.ObjectIsPromise() is better?

[bnoordhuis]

It's faster and creates less garbage than Debug.MakeMirror() or v8::Value::IsPromise().

[targos]

Do these three methods return true for instances of class MyPromise extends Promise {...} ?

[bnoordhuis]

@targos Yes.

[bnoordhuis]

I just realized we don't have to wait for a V8 upgrade to switch to ObjectIsPromise(): #3130