Prototype overwrites native array functionality
Reported by fearphage | August 31st, 2008 @ 10:41 PM | in 1.6.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
-
-
fearphage 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 September 11th, 2008 @ 03:23 PM
Are you planning on supporting IE6/7, Opera 9.25 ? Prototype should check for the natives. But I think it should work identical to the natives. If Native Array#filter allows NodeLists and other Array-Like objects then Prototype's Array#filter should allow the same thing.
-
Juriy Zaytsev September 11th, 2008 @ 03:40 PM
- → Assigned user changed from to Juriy Zaytsev
- → Tag changed from array native needs_patch performance to array native needs_patch needs_tests performance
- → Milestone changed from to 1.6.1
- → State changed from new to enhancement
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.
-
fearphage 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?
-
fearphage September 11th, 2008 @ 04:00 PM
- → Assigned user changed from to Juriy Zaytsev
- → Tag changed from array native needs_patch performance to array native needs_patch needs_tests performance
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 September 11th, 2008 @ 04:03 PM
- → Assigned user cleared.
- → Tag changed from array native needs_patch needs_tests performance to array native needs_patch performance
Could this replace $A?
Sort of. $A also delegates its logic to an object if an object implements
toArray. -
John-David Dalton September 11th, 2008 @ 04:18 PM
- → State changed from enhancement to bug
@fearphage Once Prototype is wrapped in a closure I think there will definetly be something like:
var slice = Array.prototype.slice; //.... slice.call(arguments, 0); can be used.AFAIK IE errors on slice.call(nodeList, 0) so it cannot be used for everything. There are ways to detect node lists and we could add an Object.isNodeList() method that would aid in detection and use $A() for the nodeLists or perform a one time capability test and use slice.call if supported and $A if not.
-
fearphage 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 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 usingslicewhere appropriate.Am I missing something?
-
fearphage September 11th, 2008 @ 05:36 PM
Native is almost guaranteed faster than anything else. That's the only reason I would suggest.
-
Juriy Zaytsev September 11th, 2008 @ 06:05 PM
@fearphage
Native array methods - yes, but not
slicevs$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. -
fearphage 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 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: 125msYou're right.
sliceis ~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 insliceexample).All the nitpicking aside, I agree that even such a small difference could be important. We actually have plans to replace
$Awithslicewhere appropriate (but leaving $A internals as they currently are).I would just like to focus this discussion on Array methods, not
$Aperformance ; ) -

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 September 14th, 2008 @ 07:32 PM
(function() { var tmp = [], window = this, natives = ['Array', 'Number', 'String']; function extend(constructor, methods) { methods = methods.split(' '); for(var i = 0, m; m = methods[i++];) Prototype[constructor].prototype[m] = window[constructor].prototype[m]; } function setConstructor(constructor, value) { value.constructor = Prototype[constructor]; return value; } function setProto(constructor, value) { value.__proto__= Prototype[constructor].prototype; return setConstructor(constructor, value); } if ((tmp.__proto__ = { }) && !tmp.push) { // Firefox, Safari for (var i = 0, n; n = natives[i++];) { Prototype[n] = (n == 'Array') ? function(value) { return setProto(n, value) } : function(value) { return setProto(n, new window[n](value)) }; } extend('Array', 'concat every filter indexOf join forEach lastIndexOf map pop push ' + 'reduce reduceRight reverse shift slice some sort splice unshift'); extend('String', 'charAt charCodeAt concat indexOf lastIndexOf match replace search ' + 'slice split substr substring toLowerCase toUpperCase'); } else { // IE 6/7, Opera var idoc, iroot, docElement = document.documentElement, iframe = document.createElement('iframe'); iframe.style.cssText = 'position:absolute;display:none;' + 'left:-5px;top:-5px;width:0px;height:0px;overflow:hidden'; docElement.insertBefore(iframe, docElement.firstChild); (idoc = window.frames[0].document).open(); idoc.write('<script>parent.Prototype._iroot = this;<\/script>'); idoc.close(); iframe.parentNode.removeChild(iframe); iroot = Prototype._iroot; delete Prototype._iroot; for (var i = 0, n; n = natives[i++];) { Prototype[n] = (n == 'Array') ? function(value) { return setConstructor(n, iroot.Array.prototype.slice.call(value, 0)) } : function(value) { return setConstructor(n, new iroot[n](value)) }; Prototype[n].prototype = iroot[n].prototype; } } })();/* TESTS */ var foo = Prototype.Array([]); foo[2] = 'apple'; foo[1] = 'orangle'; foo[0] = 'banana'; alert(foo.length); //3 Prototype.Array.prototype.customized = function(){ return this }; alert(typeof foo.customized); // function var bar = ['a', 'b', 'c']; alert(typeof bar.customized); // undefined bar = Prototype.Array.prototype.slice.call(bar, 0); alert(typeof bar.customized); // function -
fearphage 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); // falseThink of
JSON.stringifyfor instance which usesinstanceof. 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 September 13th, 2008 @ 04:56 PM
If you are using Prototype you can use
Object.isArray()which will still return true for Arrays from iframes and you can use Prototype's JSON methods.Ideally we would be only extending the "clean" array so that the native one remains untouched.
Object.extend(Prototype.Array.prototype, Enumerable); Object.extend(Prototype.Array.prototype, ....);We would make
$A()return anew Prototoype.Arrayor people could alias it asvar array = Prototype.Arrayor we could have aObject.toArray()which does thePrototype.Array.prototype.slice.call(arguments[0], 0);Either way you end up with an array that has the
toJSONmethod attached and it becomes a non-issue. -
fearphage September 13th, 2008 @ 06:31 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 September 13th, 2008 @ 06:34 PM
I was proposing Prototype use the "clean" Array so it left the normal native array untouched. I think that is addressing the problem head on. It would allow any other library to interact with normal arrays without incident.
-
fearphage 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 September 13th, 2008 @ 08:09 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.)
-
fearphage September 13th, 2008 @ 08:29 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 September 13th, 2008 @ 11:32 PM
- → Tag cleared.
Yep, we need to deprecate prototype's
map,filter,every,someandreduce. Next release - 1.6.0.3 - is a bugfix/performance optimization only, so the deprecation will need to wait till 1.6.1 : / -
fearphage 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 useObject.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.
-
fearphage September 14th, 2008 @ 11:32 AM
Have you tested this solution in IE6? IE6 is complaining about
this.eachnot being a function which is what I got before I started using the work around. -
fearphage September 14th, 2008 @ 12:30 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 September 14th, 2008 @ 05:54 PM
I tried setting the document.domain to match the pages via in
.write()call and I get a permission denied error. Ahh do to an IE6/7 bug you can only set its document.domain after the iframe loads so calling.idoc.domain = document.domain;should work...Updated
Didn't error but didn't work :(
-
fearphage September 14th, 2008 @ 05:51 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.domainis set. -
John-David Dalton September 14th, 2008 @ 06:33 PM
There is also a memory leak associated with cross-frame communication, but the way we handle it avoids the leak. http://novemberborn.net/javascript/edgvl
-
John-David Dalton September 14th, 2008 @ 07:15 PM
Updated my example to use an alternate method in Safari and Firefox that manipulates the object instances proto property.
However, using this approach makes
var foo = Prototype.Array([]); alert(foo instanceof Array); -> true for Firefox,Safari and false for IE, OperaThis shouldn't be a big deal. Discussion about namespacing Prototype. http://groups.google.com/group/prototype-core/browse_thread/thread/16d0517ecc605a00
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
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.
