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

Fix Function.apply to work with generic array-like object instead of an array #99

Open
termi opened this issue Feb 1, 2012 · 17 comments

Comments

@termi
Copy link
Contributor

termi commented Feb 1, 2012

Can we fix that in IE < 9 ?

I fix it (outdated)https://github.com/termi/es5-shim/blob/master/es5-shim.js#L198
But I am not sure, what my solution quite well. Can you check it out?

P.S. In my own lib I use Array.from instead of:

args = toObject(args);
var i = -1, arr = [], length = args.length >>> 0;

while (++i < length) {
    if (i in args) {
        arr.push(args[i])
    }
}
@kriskowal
Copy link
Member

Perhaps,

var needsPatch;
try {
    var result = (function (a) {return a}).apply(null, {0: 10, length: 1});
    needsPatch = result !== 10;
} catch (exception) {
    needsPatch = true;
}

if (needsPatch) {
    Function.prototype.apply = (function (apply) {
        return function (thisp, args) {
            return apply.call(this, thisp, Array.prototype.slice.call(args));
        };
    })(Function.prototype.apply);
}

Your Array.from(x) is equivalent to the generic Array.prototype.slice.call(x). The initial index of undefined (by omission) is coerced to 0 internally, though it would be a good thing™ if Array.from became a standard, especially if it handled Harmony iterables.

@termi
Copy link
Contributor Author

termi commented Feb 1, 2012

@kriskowal
Due this patch is only actual in IE < 9 I have to note that IE < 9 throw exception if we try to Array.prototype.slice.call(x) on DOM object, eg NodeList: "this" is not a JavaScript object
And for performance we have to try original function first.
Also Array.prototype.slice.call don't throw exception if args is a String, that is not valid.

Thx for the syntax idea!

@termi termi closed this as completed Feb 2, 2012
@termi termi reopened this Oct 12, 2012
@termi
Copy link
Contributor Author

termi commented Oct 12, 2012

Allowing array-like object in Function.prototype.apply is very important to me due DOM4 mutation methods:

var node = document.querySelector("div");
node.append.apply(node, document.querySelectorAll("p"));

This code should append all <p> nodes into a given <div> node.

Since I implement DOM4 mutation methods in IE6+ I have to path Function.prototype.apply to allow array-like object as a second parameter - link to path

If you find it useful, I can make Pull Request

My previous Pull Request #100 contains a error which I fixed

@abozhilov
Copy link

Native ES5 apply, doesn't skip holes in array-like objects as your approach. If you are going to use it, you should fix it.

@termi
Copy link
Contributor Author

termi commented Nov 5, 2012

@abozhilov Yes you are right. And I fixed this. My Array.from function now is:

Array.from = function(iterable) {
    if(iterable instanceof Array || Array.isArray(iterable))return iterable;
    if(iterable.toArray)return iterable.toArray();

    var object = Object(iterable),
        length = object.length >>> 0,
        result;

    try {
        result = Array.prototype.slice.call(object);//Old IE throw error here when object is NodeList
    }
    catch(e) { }

    if(result && result.length === length)return result;

    result = [];

    for(var key = 0 ; key < length ; key++) {
        if(key in object)
            result[key] = object[key];
    }

    return result;
};

And here is a new Function.prototype.apply fix without try/catch:

var /** @const */
    _Function_apply = Function.prototype.apply;

var canUseArrayLikeObjectInApply = false;

try {
    canUseArrayLikeObjectInApply = isNaN.apply(null, {})
}
catch(e) { }

if(!canUseArrayLikeObjectInApply) {
    Function.prototype.apply = function(contexts, args) {
        // 1. simple case, no args at all
        if(!args) {
            return _Function_apply.call(this, contexts);
        }

        // 2. args is not an array-like object or just an array
        if (
            typeof args !== "object" // when args is not an object -> IE throw error for this case and would be right
            || args instanceof Array // if args is array -> IE can eat this case
            || !("length" in args)   // if args is an abject but not array and also has no "length" prop -> IE throw error for this case and would be right
            ) {
            return _Function_apply.call(this, contexts, args);
        }

        // 3. args is array-like object
        return _Function_apply.call(this, contexts, Array.from(args));
    };
}

@ljharb
Copy link
Member

ljharb commented Dec 23, 2013

@termi Array.from is part of ES6 - I suggest using a different name to avoid collision, or following the spec. A tested implementation that does so is available here: https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L214-L237

Considering Array.from is in ES6 already, this might be an issue better fixed in the ES6 shim? Please feel free to file it there as well - I'll leave this open too.

@termi
Copy link
Contributor Author

termi commented Dec 25, 2013

@ljharb I will update my Array.from as soon as i am starting using my es6-transpiler in a real project. At the time when I wrote Array.from, it conforms to the specifications. Now it needs refactoring as well as my entire es5/6 shim project (I started it when I first started learning javascript).

As far as I see, es6-shim implementation doesn't match the specification since it doesn't support iterator protocol. In my es6-transpiler I want to use Array.from for spread emulation (I suggest that native implementation would be much faster than something like this var iter = obj.iterator(), v; while(v = iter.next().done !== false){ /*do*/ } - but it's only an assumption, that must be verified on native Array.from).
But back to es6-shim implementation - since it not support iterator protocol, it's breaks the code that generated by my transpiler, so I must to reshim it again:

var obj = {a: 9};
obj.iterator = function() {
    var iterableObject = this;
    var first = true;

    return {
        next: function() {
            var result = {
                value: first ? iterableObject.a : void 0
                , done: !first
            }
            first = false;
            return result;
        }
    }
}

function Array_from_test() {
    var result;
    try {
        result = Array.from(obj);
    }
    catch(){}
    return Array.isArray(result) && result.length === 1 && result[0] === obj.a;
}

if( !Array_from_test() ) {
    Array.from = function() {
        // my implementation
    }
}

Considering Array.from is in ES6 already, this might be an issue better fixed in the ES6 shim? Please feel free to file it there as well - I'll leave this open too.

Array.from is not related to the topic of this issue. As I mentioned before, Function#apply in IE < 9 doesn't allow array-like object as second parameter. It's doesn't match the specification of ecmascript 5! So I think it's must be fixed in es5-shim.

@ljharb
Copy link
Member

ljharb commented Dec 26, 2013

Can you file an issue on es6-shim about the iterator stuff? We'll verify that separately.

I think you're right; fixing apply should be done here. However, shimming Array.from should decidely not be here, since it's not part of ES5. In addition, there should be no code in the es5-shim that has anything to do with iterators, for the same reason - if it works, great, but the code shouldn't attempt to deal with it.

@Yaffle
Copy link
Contributor

Yaffle commented Dec 26, 2013

@ljharb, lol, why? if this code is needed, it could be used.

@termi
Copy link
Contributor Author

termi commented Dec 26, 2013

@ljharb
Copy link
Member

ljharb commented Dec 26, 2013

@Yaffle this is the ES5 shim. ES6 doesn't belong in it. If you want to get any ES6 behavior, you'd use both the ES5 and ES6 shims together (they're designed to be layered)

@Yaffle
Copy link
Contributor

Yaffle commented Dec 27, 2013

@ljharb , better fix es6-shim

@ljharb
Copy link
Member

ljharb commented Dec 27, 2013

That was my suggestion, but @termi's point about apply being broken in ES5 is still valid here. Please see the discussion in paulmillr/es6-shim#182 on why iterators can't be shimmed.

ljharb added a commit that referenced this issue Apr 17, 2014
@ljharb ljharb added this to the v3.1.0 milestone Apr 19, 2014
@ljharb
Copy link
Member

ljharb commented May 14, 2014

This affects node 0.6 as well

@ljharb ljharb modified the milestones: v4.0.0, v4.1.0 Aug 14, 2014
@ljharb
Copy link
Member

ljharb commented Mar 23, 2015

This also affects OmniWeb v5.11.2

@Xotic750
Copy link
Contributor

This is a subject that I have been looking at for some of my own code and I thought I'd share my investigation code with you as it may be of some help (maybe). Sorry about the size of it, I don't know how to make the code block scrollable. :)

Available on jsFiddle

/*global Date, Function, Math, Object, String */

/*properties
    abs, apply, call, floor, length, log, max, min, prototype, random
*/

/*jslint browser: true, devel: true, node: true, sloppy: true*/

(function (global) {
    /**
     * Returns an arguments object of the arguments supplied.
     *
     * @private
     * @function $returnArgs
     * @param {...*} [varArgs]
     * @return {Arguments}
     */
    function $returnArgs() {
        return arguments;
    }

    (function (call, apply) {
        'use strict';

        /*jslint vars: true */

        /**
         * @name forceCallShim
         * @type {boolean}
         */
        var forceCallShim = false;

        /**
         * @name forceCallPatch
         * @type {boolean}
         */
        var forceCallPatch = false;

        /**
         * @name forceApplyShim
         * @type {boolean}
         */
        var forceApplyShim = false;

        /**
         * @name forceApplyPatch
         * @type {boolean}
         */
        var forceApplyPatch = false;

        /**
         * @name MAX_SAFE_INTEGER
         * @type {number}
         * @const
         * @default 9007199254740991
         */
        var MAX_SAFE_INTEGER = 9007199254740991;

        /**
         * Shortcut
         * The Object constructor creates an object wrapper.
         * Keeps jslint happy
         *
         * @private
         * @function $Object
         * @param {*} inputArg
         * @return {Object}
         */
        var $Object = Object;

        /**
         * Shortcut
         * The String constructor creates an object wrapper.
         * Keeps jslint happy
         *
         * @private
         * @function $String
         * @param {*} inputArg
         * @return {Object}
         */
        var $String = String;

        /**
         * Shortcut
         *
         * @private
         * @function $min
         * @param {number} number
         * @return {number}
         */
        var $min = Math.min;

        /**
         * Shortcut
         *
         * @private
         * @function $max
         * @param {number} number
         * @return {number}
         */
        var $max = Math.max;

        /**
         * Shortcut
         *
         * @private
         * @function $floor
         * @param {number} number
         * @return {number}
         */
        var $floor = Math.floor;

        /**
         * Shortcut
         *
         * @private
         * @function $abs
         * @param {number} number
         * @return {number}
         */
        var $abs = Math.abs;

        /*
         * Shortcut
         *
         * @private
         * @function $random
         * @returns {number}
         */
        var $random = Math.random;

        /**
         * Shortcut
         * Returns true if the operands are strictly equal with no type conversion.
         *
         * @private
         * @function $strictEqual
         * @param {*} a
         * @param {*} b
         * @return {boolean}
         * @see http://www.ecma-international.org/ecma-262/5.1/#sec-11.9.4
         */
        function $strictEqual(a, b) {
            return a === b;
        }

        /**
         * Shortcut
         * Returns true if the operand inputArg is undefined.
         *
         * @private
         * @function $isUndefined
         * @param {*} inputArg
         * @return {boolean}
         */
        function $isUndefined(inputArg) {
            return $strictEqual(typeof inputArg, 'undefined');
        }

        /**
         * The abstract operation converts its argument to a value of type string
         *
         * @private
         * @function $toString
         * @param {*} inputArg
         * @return {string}
         * @see http://www.ecma-international.org/ecma-262/5.1/#sec-9.8
         */
        function $toString(inputArg) {
            var val;

            if (inputArg === null) {
                val = 'null';
            } else if ($isUndefined(inputArg)) {
                val = 'undefined';
            } else {
                val = $String(inputArg);
            }

            return val;
        }

        /**
         * The function evaluates the passed value and converts it to an integer.
         *
         * @private
         * @function $toInteger
         * @param {*} inputArg The object to be converted to an integer.
         * @return {number} If the target value is NaN, null or undefined, 0 is returned. If the target value is false, 0 is returned and if true, 1 is returned.
         * @see http://www.ecma-international.org/ecma-262/5.1/#sec-9.4
         */
        function $toInteger(inputArg) {
            var number = +inputArg,
                val = 0;

            if ($strictEqual(number, number)) {
                if (!number || number === Infinity || number === -Infinity) {
                    val = number;
                } else {
                    val = (number > 0 || -1) * $floor($abs(number));
                }
            }

            return val;
        }

        /**
         * The abstract operation ToLength converts its argument to an integer suitable for use as the length
         * of an array-like object.
         *
         * @private
         * @function $toLength
         * @param {*} inputArg The object to be converted to a length.
         * @return {number} If len <= +0 then +0 else if len is +Infinity then 2^53-1 else min(len, 2^53-1).
         * @see https://people.mozilla.org/~jorendorff/es6-draft.html#sec-tolength
         */
        function $toLength(inputArg) {
            return $min($max($toInteger(inputArg), 0), MAX_SAFE_INTEGER);
        }

        /**
         * Returns the this context of the function.
         *
         * @private
         * @function $returnThis
         * @return {*}
         */
        var $returnThis = function () {
            return this;
        };

        /**
         * Indicates if running in strict mode.
         * True if we are, otherwise false.
         *
         * @private
         * @name isStrictMode
         * @type {boolean}
         */
        var isStrictMode = !$returnThis();

        console.log('isStrictMode', isStrictMode);

        /**
         * Indicates if the this argument used with call does not convert to an object when not strict mode.
         * True if it does, otherwise false.
         *
         * @private
         * @name hasCallBug
         * @type {boolean}
         */
        var hasCallBug = (function () {
            var rtn;

            // try in case call is missing
            try {
                rtn = !isStrictMode && typeof $returnThis.call('foo') === 'string';
            } catch (eHasCallBug) {
                rtn = false;
            }

            console.log('hasCallBug', rtn);

            return rtn;
        }());

        /**
         * Indicates if the this argument used with apply does not convert to an object when not strict mode.
         * True if it does not, otherwise false.
         *
         * @private
         * @name hasApplyBug
         * @type {boolean}
         */
        var hasApplyBug = (function () {
            var rtn;

            // try in case apply is missing
            try {
                rtn = !isStrictMode && typeof $returnThis.apply('foo') === 'string';
            } catch (eHasApplyArrayBug) {
                rtn = false;
            }

            console.log('hasApplyBug', rtn);

            return rtn;
        }());

        /**
         * Indicates if apply works with ArrayLike objects.
         * True if it does not, otherwise false.
         *
         * @private
         * @name supportsApplyArrayLike
         * @type {boolean}
         */
        var supportsApplyArrayLike = (function () {
            var returnThis,
                rtn;

            returnThis = function (arg) {
                return arg;
            };

            // try in case apply is missing
            try {
                rtn = returnThis.apply('foo', $returnArgs(1)) === 1;
            } catch (eHasApplyArrayBug) {
                rtn = false;
            }

            console.log('supportsApplyArrayLike', rtn);

            return rtn;
        }());

        /**
         * @private
         * @function $getName
         * @param {Object} object
         * @param {string} name
         * @return {string}
         */
        function $getName(object, name) {
            while (!$isUndefined(object[name])) {
                name += $toString($toInteger($random() * 100));
            }

            return name;
        }

        /**
         * @private
         * @function $evalCallApply
         * @param {*} thisArg
         * @param {string} name
         * @param {Function} func
         * @param {ArrayLike} args
         * @param {number} start
         * @return {*}
         */
        function $evalCallApply(thisArg, name, func, args, start) {
            var length = $toLength(args.length),
                last = length - 1,
                argsStrings = '',
                index,
                rtn;

            for (index = start; index < length; index += 1) {
                argsStrings += 'args[' + index + ']';
                if (index < last) {
                    argsStrings += ',';
                }
            }

            name = $getName(thisArg, name);
            thisArg[name] = func;
            /*jslint evil: true */
            rtn = new Function('fn', 'name', 'args', 'return function () { return fn[name](' + argsStrings + '); }();')(thisArg, name, args);
            /*jslint evil: false */
            delete thisArg[name];

            return rtn;
        }

        /**
         * Sets the arguments as per ES3 or ES5 non-strict spec.
         *
         * @private
         * @function $toObjectThisArg
         * @param {*} thisArg
         * @return {Object}
         */
        function $toObjectThisArg(thisArg) {
            if (!isStrictMode) {
                if (thisArg === null || $isUndefined(thisArg)) {
                    thisArg = global;
                } else {
                    thisArg = $Object(thisArg);
                }
            }

            return thisArg;
        }

        /**
         * @global
         * @function Function.prototype.call
         */
        if (forceCallShim || !call) {
            // ES3 spec
            Function.prototype.call = function (thisArg) {
                return $evalCallApply($toObjectThisArg(thisArg), '__call__', this, arguments, 1);
            };

            console.log('Function.prototype.call: ES3 shim');
        } else if (forceCallPatch || hasCallBug) {
            // ES5 patch
            Function.prototype.call = (function () {
                return function (thisArg) {
                    var name = '__call_',
                        args = [],
                        length = $toLength(arguments.length),
                        index,
                        rtn;

                    name = $getName(this, name);
                    this[name] = apply;
                    args.length = length - 1;
                    for (index = 1; index < length; index += 1) {
                        args[index - 1] = arguments[index];
                    }

                    rtn = this[name]($toObjectThisArg(thisArg), args);
                    delete this[name];

                    return rtn;
                };
            }());

            console.log('Function.prototype.call: ES5 patch');
        }

        /**
         * @global
         * @function Function.prototype.apply
         */
        if (forceApplyShim || !apply) {
            // ES3 spec
            Function.prototype.apply = function (thisArg, arrayLike) {
                return $evalCallApply($toObjectThisArg(thisArg), '__apply__', this, arrayLike, 0);
            };

            console.log('Function.prototype.apply: ES3 shim');
        } else if (forceApplyPatch || hasApplyBug || !supportsApplyArrayLike) {
            // ES5 patch
            Function.prototype.apply = (function () {
                return function (thisArg, arrayLike) {
                    var object = $Object(arrayLike),
                        name = '__apply_',
                        args,
                        length,
                        index,
                        rtn;

                    name = $getName(this, name);
                    this[name] = apply;
                    if (supportsApplyArrayLike) {
                        args = object;
                    } else {
                        args = [];
                        length = $toLength(object.length);
                        args.length = length;
                        for (index = 0; index < length; index += 1) {
                            args[index] = object[index];
                        }
                    }

                    rtn = this[name]($toObjectThisArg(thisArg), args);
                    delete this[name];

                    return rtn;
                };
            }());

            console.log('Function.prototype.apply: ES5 patch');
        }
    }(Function.prototype.call, Function.prototype.apply));

    (function () {
        console.log('\nNon-Strict Mode\n');

        /**
         * Function to test call and apply
         *
         * @private
         * @function testFn
         * @param {...*} [varArgs]
         */
        var testFn = function (title) {
            console.log('\n' + title + '\n');
            console.log('typeof this: ', typeof this);
            console.log('this: ', this);
            console.log('arguments: ', arguments);
        };

        testFn.call('foo', 'call', undefined, null, 1, '2', true, Object, new Date());
        testFn.apply('foo', $returnArgs('apply', undefined, null, 1, '2', true, Object, new Date()));
        testFn.call(undefined, 'call', undefined, null, 1, '2', true, Object, new Date());
    }());

    (function () {
        'use strict';

        console.log('\nStrict Mode\n');

        /**
         * Function to test call and apply
         *
         * @private
         * @function testFn
         * @param {...*} [varArgs]
         */
        var testFn = function (title) {
            console.log('\n' + title + '\n');
            console.log('typeof this: ', typeof this);
            console.log('this: ', this);
            console.log('arguments: ', arguments);
        };

        testFn.call('foo', 'call', undefined, null, 1, '2', true, Object, new Date());
        testFn.apply('foo', $returnArgs('apply', undefined, null, 1, '2', true, Object, new Date()));
        testFn.call(undefined, 'call', undefined, null, 1, '2', true, Object, new Date());
    }());
}(this));

@Xotic750
Copy link
Contributor

@ljharb I now have apply working on IE8
https://rawgit.com/Xotic750/es5-shim/apply/tests/index.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants