Doubled array entry using index '' in node v8.4.0

[lucaelin]

• Version: v8.4.0
• Platform: Linux hostname 4.4.0-93-lowlatency test-require-resolve: use case insensitive compare #116-Ubuntu SMP PREEMPT Fri Aug 11 22:13:00 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

As of node v8.4.0, assigning a value to the array index '' causes a duplicate entry to be created. The duplicate disappears (and cannot be recreated) as soon as a value is assigned to a numerical index.

let x = [];
x[''] = 'foo';
console.log(x); 
// [ '': 'foo', '': 'foo' ]

console.log(x[0]); 
// undefined
x[0] = 'bar';
console.log(x);
// [ 'bar', '': 'foo' ]

x[''] = 'baz';
console.log(x);
// [ 'bar', '': 'baz' ]

[TimothyGu]

This is because of a bug when printing the array, not because the array actually has a duplicated property.

/cc @BridgeAR

[keangkai]

I think array is duplicate but I'm not sure

[mscdex]

It's not possible to have duplicate keys for normal JS objects like that (the "exception" being Map instances), so if anything there is a bug in util.inspect() as @TimothyGu mentioned.

[targos]

I'm looking at this

[targos]

Well, actually it seems to be already fixed on master. Can someone else confirm?

[benjamingr]

@targos looks fixed in master on macOS:

~/Documents/OpenSource/node [master] $ ./out/Release/node
> var a = []
undefined
> a[''] = 'foo'
'foo'
> console.log(a)
[ '': 'foo' ]
undefined
> 
(To exit, press ^C again or type .exit)
> 
~/Documents/OpenSource/node [master] $ node -v
v8.4.0
~/Documents/OpenSource/node [master] $ node
> var a = []
undefined
> a[''] = 'foo'
'foo'
> console.log(a)
[ '': 'foo', '': 'foo' ]
undefined
> 

Thanks for the report @chancho4321 this bug appears to have been fixed already - thanks anyway, reporting was the right thing to do :)

[TimothyGu]

That's great to hear! We should make sure to have a test case for it if one doesn't yet exist.

[BridgeAR]

This got fixed by 3a886ff. Thanks for the report as this is a interesting edge case. It could only happen if the array contained a empty string as key and no regular entry. As @TimothyGu pointed out it would be nice to add a additional test case.

Therefore I am adding the good first contribution label.

[mscdex]

@addaleax Shouldn't this still be tagged with 'v8.x' since that is where the issue still occurs (the aforementioned commit is not present in either the 'v8.x-staging' or 'v8.x' branches)?

[addaleax]

@mscdex I can only speak for me, but as somebody who put together a number of the previous v8.x releases, having that label on issues that were also affecting master (which is our implied default, iirc) is confusing, because it gives the impression that some special care is needed for that release line. Unlike with LTS, we land anything in v8.x that also landed in master and is not semver-major by default, so this wouldn’t have been forgotten. Other releasers might disagree, and I’m not saying that I’m right about this being the best approach.

(It’s totally fine to add that label for bugs in LTS, or for bugs that only occur in specific release lines, of course.)

[Trott]

There's a PR open for this so I'm removing the good first contribution label.

[BridgeAR]

Fixed by 21a3ae3