Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7475 (closed enhancement: wontfix)

Opened 2 years ago

Last modified 2 years ago

[PATCH] Object.prototype vaccine

Reported by: mislav Assigned to: sam
Priority: high Milestone: 1.x
Component: Prototype Version: edge
Severity: major Keywords: ready discuss
Cc: savetheclocktower

Description

The framework should be immune to Object.prototype augmentation. Yeah, that means conditionals in for ... in loops. What we should do is add a generic iterator for objects so not to repeat ourselves.

Attachments

protosafe.diff (6.5 kB) - added by mislav on 02/07/07 02:00:11.
now with more $break support! (thnx Andrew)

Change History

02/03/07 13:59:27 changed by mislav

Did I mention this would solve #6579 and similar problems?

02/03/07 16:40:28 changed by mislav

  • priority changed from normal to high.
  • type changed from defect to enhancement.

I'm bumping priority here because this needs decision and is quite a blocker for some people.

02/06/07 21:34:55 changed by savetheclocktower

Plus one, dude. Great work. Unfortunately, this will cause some headaches down the road, but it's the right thing to do.

Only one request: could you wrap the whole thing in a try block like Enumerable.each (to catch $break)? That way we don't have to give up breaks.

02/07/07 02:00:11 changed by mislav

  • attachment protosafe.diff added.

now with more $break support! (thnx Andrew)

02/07/07 02:04:24 changed by mislav

I forgot to credit Dean Edwards, whose generic enumeration technique inspired this.

02/08/07 06:43:32 changed by savetheclocktower

  • cc set to savetheclocktower.

Oh, one more question: why use Function.prototype.each instead of Object.each? As far as I know Object is the only object that needs this method.

(follow-up: ↓ 8 ) 02/08/07 09:35:33 changed by mislav

Function.prototype.each is more generic than Object.each because it then applies to all functions, meaning all class constructors.

var Person = function(name) { this.name = name }
var p = new Person("Freddy")
Person.each(p, function(prop){ ... } )

02/08/07 13:02:46 changed by mislav

  • keywords set to ready discuss.

(in reply to: ↑ 6 ) 02/09/07 11:12:27 changed by savetheclocktower

Replying to mislav:

Function.prototype.each is more generic than Object.each because it then applies to all functions, meaning all class constructors.

Yes, but it's serving as a generic method, so why does it need to be in more than one place? You'll still need to pass the object as the first parameter. Calling it Function.prototype.each means there'll be a crapload of methods that do exactly the same thing but are named different things. I find that confusing.

02/10/07 21:51:55 changed by Tobie

Doesn't this add a lot of overhead ? (It's a genuine question, I haven't looked into the patch that much).

Extending Object.prototype is bad practice in the first place (Prototype had been significantly bashed when it was doing so) and is rarely found in 3rd party libs (the only one I know which does that is json.js and it has good reasons to do so, as it's meant to be integrated into future versions of JavaScript, not used as is). Should Prototype account for it by gracefully degrading in such case ? I don't think so.

Why then not also account for the numerous libs using the for...in loop to iterate over arrays, another, quite common bad practice?

Again, it's possible, but it would mean abandoning literal notations for arrays.

On top of that, there's lots of other ways to break Prototype which don't even imply bad coding practices: you know very well what would happen if a lib choose to redefine the dollar function, or worse, String.prototoype.evalScript (for example). If namespacing Prototype could deal with the former, dealing with the latter would imply completely rethinking the whole library (i.e. stopping to extend native objects).

Potential conflicts with other libs will always exist. Doing as much as possible to play nice with libs that are correctly coded is one thing, but modifying Prototype to account for lousy coding practices is a route that we should not take in my opinion.

02/11/07 13:44:28 changed by savetheclocktower

Tobie, I see where you're coming from, but I'm in favor of an Object.each method for other reasons as well: I'd like to eventually extend all Enumerable methods onto Object to serve as generics. I think this would solve the problems we've had with finding a solid Hash implementation. It wouldn't interfere with literal Object syntax. Nor (in my opinion) does it add a lot of overhead.

I wonder if we have the moral authority to frown upon extending Object.prototype when we extend virtually all other prototypes ourselves. I'm not saying it's a great idea to extend Object.prototype; I'm saying that the people who don't extend *any* prototypes are a bit more justified in their opposition.

I don't think there is any good answer when it comes to the conflict between Object.prototype and for..in. Most people say that Object.prototype is broken; Doug Crockford says that {{{for..in}} is broken. He's entitled to his opinion.

I don't think we should tell people that for..in is dangerous and should no longer be used, but I do think we should code defensively.

(follow-up: ↓ 12 ) 02/13/07 19:40:41 changed by sam

  • status changed from new to closed.
  • resolution set to wontfix.

This is certainly a noble idea, but I'm of the opinion that we shouldn't take responsibility for extensions to Object.prototype as long as we aren't extending it ourselves.

(in reply to: ↑ 11 ; follow-up: ↓ 13 ) 02/19/07 15:54:09 changed by szoftos

Replying to sam:

This is certainly a noble idea, but I'm of the opinion that we shouldn't take responsibility for extensions to Object.prototype as long as we aren't extending it ourselves.

and how will you fix #7444 ?

(in reply to: ↑ 12 ) 02/24/07 17:22:13 changed by yanick.rochon

As Ssm and Tobie have said (and sorry if I'm not getting this right), Prototype may not be responsible for bad coding practices of other 3rd party libraries, but I've read people "bashing" Prototype because it is not compatible with other 3rd party libraries... The solution that I provided on this ticket #7566 doesn't raise any new issue ; it merely ensures that anything added to the Object.prototype is simply discarded. And if there is one case where this fix would still break (in this particular setting), I would like to know. (not assuming that someone directly modify the code inside Prototype from a third party script of course.)

Personnally, I prefer the Object.exend() over Object.prototype.extend ; "static" functions are better than adding custom properties to objects in Javascript. It might be pratical to do so, but if we (Javascript developpers) want to tell people to avoid extending the Object.prototype, we should not do it ourselves. In the long run, providing the right functions at the right place falls onto the developper if the code breaks. Then, it's only a matter of teaching people to code right (*sig*).

Finally, $() and such functions may be regarded as "unconventional" and may be conflictual (for example, JQuery uses the same function as initializer), but the compromise is clear ; 3rd party libs should be complementary, not a competitor. Which means that Prototype should be compatible with any other 3rd party library, as long as this library doesn't offer competitive functionality. Thus, for..in loops should not be discouraged, and should not depend on object.each() nor Object.each() to iterate safely either.