mozilla / rhino

Rhino is an open-source implementation of JavaScript written entirely in Java
https://rhino.github.io
Other
4.18k stars 850 forks source link

Array.from does not follow spec to prioritize iterable over array-like #1518

Open tonygermano opened 4 months ago

tonygermano commented 4 months ago

According to the spec, if the object passed to Array.from has an iterator method with a Symbol.iterator key, then that should be used before checking for a length property to see if the object is array-like. This is true even for native arrays.

function test() {
    let counter = 0
    Array.prototype[Symbol.iterator] = function* () {
        for (let i = 0; i < this.length; i++) {counter++; yield this[i]}
    }
    Array.from(['a','b','c'])
    return counter
}

In Chrome

console.log(test())
3

Rhino

js> print(test())
0
tonygermano commented 4 months ago

In regard to #1128 , the optimization could probably still be performed for a NativeArray if we check that the value stored in the Symbol.iterator property still points to the built-in iterator method and has not been changed by the user.

dreamphridge commented 2 weeks ago

Hi @tonygermano! I went through the issue. I reproduced this behavior locally and traced the issue to the js_from function in NativeArray.java: private static Object js_from(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {

I believe that this issue is because of the following check: if (!(items instanceof NativeArray) && (iteratorProp != Scriptable.NOT_FOUND) && !Undefined.isUndefined(iteratorProp)) {

Since items is an instance of NativeArray, this condition fails because items is an instance of NativeArray, which is why it skips the iterable handling. According to the ECMAScript specification, even native arrays should use [Symbol.iterator] if it’s defined.

I’d like to work on this issue and would appreciate any feedback on my analysis. Could you please assign it to me?