Map and Set have internal state as enumerable keys

[ljharb]

Object.keys(new Map()) and Object.keys(new Set()) should produce an empty array in an ES5 engine, but in node 0.10, they both produce [ '_i', '_f', '_l', '_s' ].

[zloirock]

I don't see any serious difference between enumerable and non-enumerable keys. Before 1.0 they were non-enumerable, but defineProperty is very slow operation - it's a performance optimisation. Possible completely hide them from all possible methods, but it will produce more serious slowdown. IIRC some time ago you said something like "additional properties for collections is not a problem for a spec compliance" :) If you convince me otherwise - I will hide them.

[ljharb]

I agree that it's not a spec compliance issue. However, since they're enumerable, it currently makes core-js + is-equal tests fail on node 0.10 due to some kind of infinite loop inside $.classof.

Couldn't you make the prototype properties non-enumerable once with defineProperty, and then just use assignment in the constructor, which would preserve the non-enumerability?

[zloirock]

Couldn't you make the prototype properties non-enumerable once with defineProperty, and then just use assignment in the constructor, which would preserve the non-enumerability?

It will not work - in this case, instance properties will be enumerable :( Currently, we have no fast way to add a non-enumerable property.

I will explore possibilities to fix this issue - I already had one issue about it, but not sure it will be fixed.

[ljharb]

Does that apply to engines that aren't v8?

Was this a performance concern that actually occurred and affected somebody, or was it an optimization made because of the ember issue?

[ljharb]

If #144 is fixed, I may not be blocked by this issue anymore - however, instance props should still be non-enumerable ideally :-/

[ljharb]

Also, function Foo() { this.a = 3; this.b = 4; } Object.defineProperty(Foo.prototype, 'a', { enumerable: false }); Object.keys(new Foo()) gives ['b'], so it definitely works, but I can't make claims about its performance.

[zloirock]

Does that apply to engines that aren't v8?

AFAIK for all popular engines.

Was this a performance concern that actually occurred and affected somebody

It was performance optimisation based on tests. The problem here - it's applied not only to collections - to all polyfilled instances, for example, to promises, which should be polyfilled in most actual engines.

Also, function Foo() { this.a = 3; this.b = 4; } Object.defineProperty(Foo, 'a', { enumerable: false }); Object.keys(new Foo()) gives ['b'], so it definitely works, but I can't make claims about its performance.

It gives ['a', 'b'].

If #144 is fixed

It will be fixed in 2.0 in near days.

[zloirock]

Stop... You lost .prototype :) Looks like really I forgot about behavior with writable: false. Thanks, @ljharb, but anyway property just will not be added, so it makes no sense :)

[ljharb]

oops, that defineProperty should be on Foo.prototype, not Foo, and then it gives ['b']

[ljharb]

Put another way, this allows me to detect with core-js is used specifically - and when possible, a polyfill should be indistinguishable from the native implementation :-)

[zloirock]

@ljharb one more time :)

[ljharb]

aha, the default writable false, good point. i guess that leaves defineProperty in the constructor.

[zloirock]

Changing status to discussion - if someone have any opinion about it - welcome here.

[zloirock]

Hidden in v3 branch.