Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#903 closed defect (worksforme)

Array object incompatibility with Prototype 1.5.0 rc0

Reported by: jwang Owned by: alex
Priority: high Milestone:
Component: General Version: 0.3
Keywords: prototype Cc:
Blocked By: Blocking:


When running Dojo 0.3.0 AJAX edition with prototype.js 1.5.0 rc0 loaded first, a call will produce the following error in Firefox's JavaScript? Console:


mll[x] is not a function Line: 212

This error is observed using Firefox and 1.0.7, both on Linux.

Attachments (1)

test_coexistance.html (865 bytes) - added by alex 16 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 16 years ago by jwang

Version: 0.20.3

I posted this issue on the rails-spinoffs list and got the following response:

Here's a quote from the reply. The reply also includes some example JS not quoted below.

It's extension of the Array object if perfectly fine, if people would just
stop using the Array object as an associative array (just use a normal
object for that)... Arrays are for numeric indexing and should NOT be used
as dictionaries or hash tables. Using Array in that manner is entirely
useless as it completely circumvents the Array object's built in behaviors
for pushing, popping, indexing, etc... (again, just use an Object if you're
looking for an associative array)

comment:2 Changed 16 years ago by sjmiles

It's perfectly sensible to use an Array with sparse indexing like:

a[0] = 'apple'; a[10000] = 'pear';

Breaking code that uses loop to efficiently iterate this array is harmful.

comment:3 Changed 16 years ago by jwang

It's perfectly sensible to use an Array with sparse indexing like:

a[0] = 'apple'; a[10000] = 'pear';

Sparse indexing seems to break three-part for. In the above example, a.length == 10001 when there's only two elements in the array.

comment:4 Changed 16 years ago by jwang

I can see sjmiles' point. I just want to know what dojo's position is on this. Can/will dojo accomodate this or does the fix need to happen on the prototype side? I'd like the ability to use them together.

comment:5 Changed 16 years ago by James Burke

Milestone: 0.3.1
Owner: changed from anonymous to alex

Talked to Alex on IRC: he wants to look at this one for 0.3.1.

comment:6 Changed 16 years ago by alex

Resolution: worksforme
Status: newclosed

ok, I've investigated, and in both 0.3.0 and in trunk, the offending line is using good-old for(var x=0; x<blah.length; x++) iteration. There is no test page attached to this bug or linked from the referenced mailing list post, and mixing Prototype 1.5.0rc0 into a page that includes both bare-bones Dojo and Dojo with widgets does not elicit an exception of any kind. I tested both Dojo from source and from builds.

What's more concerning to me is this seemingly incorrect understanding of the folks on the Prototype list regarding the way things "should" be done in JavaScript?. That they are not coding defensively shows in the content of their post. It's tone may be defensive but the code sure isn't. Scott is entirely correct about efficient iteration, which the following code snippet from a command-line Spidermonkey session demonstrates:

js> bar = new Array(1000);
js> bar[5] = "blah";
js> bar[500] = "thud"; 
js> var inc = 0;       
js> for(var x in bar){ print(inc++); print(x+": "+bar[x]); }
5: blah
500: thud

In closing, I can't reproduce the bug and the person you're conversing with on the rails-spinoffs list doesn't know what they're talking about. If Dojo is indeed broken with Prototype, I'd love to see a test page. I'm attaching the one I used to verify the non-issue.

Marking "worksforme"


Changed 16 years ago by alex

Attachment: test_coexistance.html added

comment:7 Changed 16 years ago by jwang

Milestone: 0.3.1

Sorry, I got the causality wrong :( The error is generated by dojo.addOnLoad and not prototype.js. On #dojo, I've heard addOnLoad broke during the refactor and was fixed post 0.3.0. Sorry for the misdiagnosis.

Note: See TracTickets for help on using tickets.