#317 ✓resolved
Phred

Prototype overwrites native array functionality

Reported by Phred | August 31st, 2008 @ 10:41 PM | in 1.7.0.1

Prototype is overwriting the native implementations of the array extensions of js version 1.6: http://developer.mozilla.org/en/New_in_JavaScript_1.6#Array_extras

Array#every Array#filter Array#map Array#some

The native implementations would be faster.

Comments and changes to this ticket

  • John-David Dalton
  • Phred

    Phred September 11th, 2008 @ 01:46 PM

    To illustrate how unfortunate this is, I am currently working on a widget-like project and the owner of the site has included prototype. The native code I would like to use is:

    @@@ javascript: Array.prototype.filter.call(document.getElementsByTagName('input'), function(elem) { return ((elem.type == 'checkbox') && (elem.className == 'catpants') && elem.checked); });

    
    
    However, prototype overwrites Array#filter with its custom code and when I execute this it throws because NodeList#each is not a function. It would be nice if prototype at least saved a native reference for me to use. I don't want to use prototype for the widget because i would like it to be independent of js libraries.
    
  • John-David Dalton
  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 03:40 PM

    • Milestone set to 1.7
    • State changed from “new” to “enhancement”
    • Tag changed from array, native, needs_patch, performance to array, native, needs_patch, needs_tests, performance
    • Assigned user set to “Juriy Zaytsev”

    Just steal it from an iframe : )

    
    document.body.innerHTML += '<iframe id="__iframe" style="display:none"></iframe>';
    var filter = $('__iframe').contentWindow.Array.prototype.filter;
    filter.call([1,2,3,4,5], function(n){ return n>3 }); // [4,5]
    

    I'll see what we can do about this.

  • Phred

    Phred September 11th, 2008 @ 03:58 PM

    • Assigned user cleared.
    • Tag changed from array, native, needs_patch, needs_tests, performance to array, native, needs_patch, performance

    Indeed i am supporting them and I don't want to overwrite the customer's code so I'm working around it.

    
    if (!Array.prototype.filter) {
      Array.prototype.filter = function(fn, that) {
        var result = [];
        for (var i = 0, length = this.length; i < length; i++) {
          if (i in this) {
            var value = this[i]; // in case fn mutates this
            if (fn.call(that, value, i, this))
              result.push(value);
          }
        }
        return result;
      };
    }
    

    arguments is a good example of something that is not an array that can be passed to all the array functions though.

    A quick way to convert things into an array: Array.prototype.slice.call(arguments)

    I also use it to copy things into arrays.

    Could this replace $A?

  • Phred

    Phred September 11th, 2008 @ 04:00 PM

    • Tag changed from array, native, needs_patch, performance to array, native, needs_patch, needs_tests, performance
    • Assigned user set to “Juriy Zaytsev”

    Weird system. I posted after you and cleared stuff. Sorry.

    Also, it is not an enhancement. It is a bug. You are preventing native functionality from taking place.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 04:02 PM

    • Tag changed from array, native, needs_patch, needs_tests, performance to array, native, needs_patch, performance
    • Assigned user cleared.

    Could this replace $A?

    Sort of. $A also delegates its logic to an object if an object implements toArray.

  • John-David Dalton

    John-David Dalton September 11th, 2008 @ 04:18 PM

    • State changed from “enhancement” to “bug”
  • Phred

    Phred September 11th, 2008 @ 04:22 PM

    
    (function(slice) {
     $A = function() {
        if (typeof this.toArray == 'function')
          return this.toArray();
        return slice.apply(this, arguments);
      };
    })(Array.prototype.slice)
    

    Just throwing that out there. Slice also does what clone does.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 04:39 PM

    @fearphage

    I don't see a reason to change $A to use slice. Current approach does its job just fine. Besides, nothing is stopping developer from using slice where appropriate.

    Am I missing something?

  • Phred

    Phred September 11th, 2008 @ 05:36 PM

    Native is almost guaranteed faster than anything else. That's the only reason I would suggest.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 06:05 PM

    @fearphage

    Native array methods - yes, but not slice vs $A.

    Array.prototype.slice.call(arrayLikeObj) has almost identical performance as $A(arrayLikeObj). For short-length objects as function arguments, the difference is barely noticeable. For longer collections (e.g. NodeList's) it's almost negligible.

  • Phred

    Phred September 11th, 2008 @ 06:35 PM

    For short-length objects as function arguments, the difference is barely noticeable. For longer collections (e.g. NodeList's) it's almost negligible.

    Please try this 1,000 times with a 1,000 element node list and tell me again that its negligible. It is not. I am from the belief system that a performance increase is a performance increase is a performance increase. To translate: if you trim 5ms off of 10 calls, you earned 50ms. That's 50ms faster than before.

    Look at your performance numbers here: http://prototype.lighthouseapp.c...

    // old: 57ms // new: 30ms

    That's 27ms which is negligible but still important. I don't think only a few ms faster is something to ignore.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 07:17 PM

    @fearphage

    
    // FF 3.0.1, Mac OS X 10.5
    
    var arr = document.getElementsByClassName('foo');
    // arr.length; // 1000
    
    console.time('$A');
    for (var i=0;i<100; i++) {
      $A(arr);
    }
    console.timeEnd('$A');
    
    console.time('slice');
    for (var i=0;i<100; i++) {
      Array.prototype.slice.call(arr);
    }
    console.timeEnd('slice');
    
    $A: 190ms
    slice: 125ms
    

    You're right.

    slice is ~50% faster when applied on a 1000-length collection 100 times. Whether this is negligible is a subjective argument. e.g. "function patch" that you cited has almost 100% improvement when run 1000 times (not 100,000 as in slice example).

    All the nitpicking aside, I agree that even such a small difference could be important. We actually have plans to replace $A with slice where appropriate (but leaving $A internals as they currently are).

    I would just like to focus this discussion on Array methods, not $A performance ; )

  • howardr

    howardr September 13th, 2008 @ 03:52 AM

    @Juriy Zaytsev

    I am assuming you will not be adding the iframe to the DOM via innerHTML:

    
    
    document.body.innerHTML += '<iframe id="__iframe" style="display:none"></iframe>';
    
    

    That will remove any event handlers bound to elements

  • John-David Dalton
  • Phred

    Phred September 13th, 2008 @ 03:46 PM

    The only problem with that is that arrays created with the clean iframe are no longer instances of the array created with the main window.

    
    bar = Prototype.Array.prototype.slice.call(bar, 0);
    alert(typeof bar.customized); // function
    alert(bar instanceof Array); // false
    

    Think of JSON.stringify for instance which uses instanceof. Now you have to check for an instance of an array from both windows. So anything I use this "clean array" on will not be stringify-able.

  • John-David Dalton
  • Phred

    Phred September 13th, 2008 @ 06:27 PM

    I think you are missing the point. I also believe you are underestimating the importance of this bug. I am not trying to make my project dependent on any library.

    I want to use native functionality that is built into the browser but Prototype is preventing this from taking place.

    I want my widget to be library agnostic because I don't want there to be any external dependencies.

  • John-David Dalton
  • Phred

    Phred September 13th, 2008 @ 08:04 PM

    @John-David Dalton: that sounds like a marvelous idea. I wasn't viewing your solution properly. That seems like a global solution to leave all native properties, objects, and methods untouched.

  • Juriy Zaytsev

    Juriy Zaytsev September 13th, 2008 @ 08:08 PM

    @fearphage

    The importance of the issue is actually pretty clear. The naive iframe workaround (that I showed before) was obviously a temporary "solution" that one could use. This workaround is not ideal, of course.

    The reason why we can't just go ahead and stop replacing native methods is back compatibility. Prototype.js has made some unfortunate decisions that we now have to keep for compatibility reasons. Replacing native methods is one of them (extending host objects/host objects' [[Prototype]]'s is another, etc.)

  • Phred

    Phred September 13th, 2008 @ 08:28 PM

    Can't you deprecate them and just say Prototype is getting better? This is one of the first times where Prototype has unrecoverably contaminated the native environment for me.

  • Juriy Zaytsev

    Juriy Zaytsev September 13th, 2008 @ 11:31 PM

    • Tag cleared.

    Yep, we need to deprecate prototype's map, filter, every, some and reduce. Next release - 1.6.0.3 - is a bugfix/performance optimization only, so the deprecation will need to wait till 1.6.1 : /

  • Phred

    Phred September 14th, 2008 @ 11:24 AM

    Thinking about this, the only new problem i see will be that you will be forcing users to use Prototype more.

    Let's say I do something and returns an instance of Prototype.Array. if (pArray instanceof Array) will now fail. I am forced to use Object.isArray(pArray). I'm not sure if this is a huge problem but it is something to consider nonetheless.

    Side note: The posting of unpreviewable, uneditable comments sucks.

  • Phred

    Phred September 14th, 2008 @ 11:31 AM

    Have you tested this solution in IE6? IE6 is complaining about this.each not being a function which is what I got before I started using the work around.

  • Phred

    Phred September 14th, 2008 @ 12:29 PM

    Scratch that last one. My problem was document.domain related. If you set document.domain before creating the iframe, the iframe has to set document.domain as well. Maybe that is something for the release notes.

    "Include prototype before setting document.domain"

  • John-David Dalton
  • Phred

    Phred September 14th, 2008 @ 05:49 PM

    It's not possible unless you load a special page in the iframe which then makes the whole process asynchronous which sucks. You have to capture your clean instances before document.domain is set.

  • John-David Dalton
  • John-David Dalton
  • Yaffle

    Yaffle April 14th, 2009 @ 02:25 PM

    Every,map,some use Prototype.K as default iterator function. And native function don't use it. :( All latest browsers(except IE) supports native implementations and Ecma 5th edition... ans it's really faster. Adding wrappers,I think, is a bad idea.

  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    [responsible:none bulk edit command]

  • Tobie Langel

    Tobie Langel March 1st, 2010 @ 12:42 AM

    • Tag set to section:lang
  • Tobie Langel

    Tobie Langel March 1st, 2010 @ 02:05 AM

    • Assigned user set to “Tobie Langel”
  • Arthur Schreiber

    Arthur Schreiber March 14th, 2010 @ 09:10 PM

    • Tag changed from section:lang to patched, section:lang

    I fixed this issue in the ecma5 branch of my Prototype fork - http://github.com/NoKarma/prototype/tree/ecma5.

    Additionally, I fixed #765 by making the fallback methods provided for IE and other browsers that do not implement Array#filter, Array#every, Array#some and friends to be callable as generics, as specified in the ES5 spec.

  • Arthur Schreiber

    Arthur Schreiber March 14th, 2010 @ 09:57 PM

    • Tag changed from patched, section:lang to benchmarked, patched, section:lang

    Here are some benchmarks for my ecma5 branch, showing some nice performance improvements thanks to the usage of the native iterator functions: http://gist.github.com/332230

    The benchmark itself can be accessed here: http://github.com/NoKarma/prototype/blob/benchmarks/test/benchmark/

  • Andrew Dupont

    Andrew Dupont October 19th, 2010 @ 04:00 AM

    • Milestone changed from 1.7 to After 1.7
    • Importance changed from “” to “High”

    We didn't manage to get this into 1.7, but it's at the top of my list for 1.7.1 (or even sooner). Pushing it back.

  • Andrew Dupont

    Andrew Dupont December 27th, 2011 @ 12:20 AM

    • Milestone changed from After 1.7 to 1.7.0.1
  • Andrew Dupont

    Andrew Dupont February 18th, 2012 @ 12:43 AM

    • State changed from “bug” to “resolved”

    Just committed Arthur's work to master — here's the bulk of it.

    We still wrap some native methods, but very lightly, and in a way that should not break ordinary ES5-style usage of these methods (e.g., generics). Performance impact should be minimal.

  • Overy753

    Overy753 March 31st, 2018 @ 01:25 PM

    Now click okay and also register with your username and password Snapchat Login your computer system matches the specifications needed for Snapchat.

  • Proctor357

    Proctor357 April 4th, 2018 @ 07:30 AM

    The warm routine login web page from the website assistance hotschedules you could make the procedure with your web browser application.

  • gmailloginfix

    gmailloginfix May 25th, 2018 @ 04:31 PM

    gmail account login
    We didn't manage to get this into 1.7, but it's at the top of my list for 1

  • annashetty

    annashetty September 26th, 2018 @ 10:51 AM

    Pretty good post. I found your website perfect for my needs
    run 3

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Referenced by

Pages