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

prototype.js breaks _.reduce #7

Closed
jou opened this issue Dec 10, 2009 · 11 comments
Closed

prototype.js breaks _.reduce #7

jou opened this issue Dec 10, 2009 · 11 comments

Comments

@jou
Copy link

jou commented Dec 10, 2009

When prototype.js is loaded, it adds a reduce function to Array's prototype. The problem is that it does something completely different than expected.

Either we to check for presence of Prototype (made a patch for that) or some other way to check if obj.reduce does the right thing.

@documentcloud
Copy link

Thanks for the patch. I'd really rather not be sniffing for specific libraries, but there really doesn't seem like any other way to detect Prototype's overwrite. I'll leave this ticket open for a little while, and see if anyone else can come up with a better way to provide compatibility.

That said, if we add this, it should be part of a larger effort to ensure that Underscore can work without conflicts with Prototype. (jQuery is, naturally, not a problem.) Are you really using both of them together on the same page? There's a ton of overlap between the two -- you're going to get a lot of duplicate code and functionality. What's your use case?

@jou
Copy link
Author

jou commented Dec 15, 2009

I'm working on a project that uses jQuery while the rest of the stuff provided by the surrounding CMS uses Prototype. I'd like to keep my code independent of whether or not Prototype is present.

Another possibility I see is to make a smoke test on Array.prototype.reduce. We could run it once when initializing (though it'll still break if Prototype is loaded later) or making the test every time _.reduce is called (no idea what impact that would have on performance).

I'll experiment a bit with that when I've got some time.

@documentcloud
Copy link

Better to sniff for Prototype then to rely on a smoke test -- have you run into any other Prototype/Underscore compatibility problems in your project?

@jou
Copy link
Author

jou commented Dec 16, 2009

Beside stuff that relies on _.reduce, none so far.

I took a further look into the matter and ran the unit tests with different versions of Prototype.

The problem only occurs with Prototype < 1.6.1 (I had 1.6.0.2) since Prototype's implementation of reduce has been removed as of 1.6.1.

The other test that failed (with both 1.6.0 and 1.6.1) was _.rest. Didn't look into that yet.

@jou
Copy link
Author

jou commented Dec 18, 2009

Well, I did a bit of work on Prototype compatibility: http://github.com/jou/underscore/commits/prototype-compat.

All tests are running and green with Prototype (both 1.6.0 and 1.6.1) loaded now. Feel free to pull it.

@documentcloud
Copy link

Copied from an email:

I did some looking into it this morning, and heard from Tobie Langel
that the next release of Prototype is going to have full ES5 (and
therefore Underscore) compatibility. So I'm going to let them fix it
the right way. Your branch is still quite valuable for anyone who
needs compatibility with old versions of Prototype, so I'll be pointing
anyone who has any trouble straight to it.

Closing the ticket...

@jdalton
Copy link
Contributor

jdalton commented Aug 2, 2011

Prototype still has several issues to fix before their methods are ES5 compliantish.
I recently created a post covering some of the incorrect ES5 fallbacks found in many popular JavaScript libs https://gist.github.com/1120592.

You can incorporate small feature tests before hand like

(function(undefined) {
  // ...
  // the rest of Underscore.js
  // ...
  // check for sparse array support
  nativeMap && (nativeMap = Array(1).map(String) < 1 && nativeMap);
  nativeIndexOf && (nativeIndexOf = Array(1).indexOf(undefined) < 0 && nativeIndexOf);
  // ...

Also, you could call them generically because Underscore.js has references to the native methods via variables such as nativeIndexOf and the like. So instead of

if (nativeIndexOf && array.indexOf === nativeIndexOf) return array.indexOf(item);

It would be

if (nativeIndexOf) return nativeIndexOf.call(array, item);

This would avoid the problem if Underscore.js was included in the page before Prototype.js.

@michaelficarra
Copy link
Collaborator

+1 @jdalton's suggestion everywhere that pattern exists

@jashkenas jashkenas reopened this Aug 8, 2011
@jashkenas
Copy link
Owner

I was thinking about taking after this idea ... but call or apply -ing a function when you don't have to can be over 90% slower than just invoking it normally.

http://jsperf.com/invoke-vs-call-vs-apply

Is it really worth it just to fix things for old versions of Prototype.js on the same page?

@jdalton
Copy link
Contributor

jdalton commented Aug 25, 2011

I was thinking about taking after this idea ... but call or apply -ing a function when you don't have to can be over 90% slower than just invoking it normally.

It's millions of operations vs millions of operations on most desktop browsers and hundreds of thousands on mobile. There is most likely no real world concern for perf. If perf was ever major factor the first thing devs should do is cut the sugar in the first place and use basic loops.

Is it really worth it just to fix things for old versions of Prototype.js on the same page?

Prototype 1.7 (the latest stable) currently paves several other native Array methods still.

You could also implement something like this to be a stronger inference of nativeness than a simple truthy check and even detect when other libs have implemented their own, most likely non-spec compliant, alternative: https://github.com/dperini/nwmatcher/blob/master/src/nwmatcher.js#L183-190

@ghost
Copy link

ghost commented Aug 26, 2011

This may have been discussed before, but why not simply remove the native method delegation entirely? This would incur a performance cost, but Underscore wouldn't need to concern itself with strict ECMAScript conformance or depend upon potentially unreliable native implementations.

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

No branches or pull requests

4 participants