#44 open
Jim Higson

Failure with augmented Object.prototype not documented

Reported by Jim Higson | March 9th, 2009 @ 11:54 AM

It is really easy to break Prototype when adding to Object.prototype. For example, I put a test page here:

http://users.ox.ac.uk/~admn2094/...

A reasonable proportion of js coders consider extending Object to be evil so If Prototype doesn't work under these conditions that seems fair enough. Mostly this is a problem where Prototype uses objects as generic hashes but doesn't check hasOwnValue etc.

Despite being discouraged by a lot of people it is still quite common to extend Object so IMO Prototype should either be patched to work with extended Object or it should be documented as known not to work under these conditions.

Comments and changes to this ticket

  • Juriy Zaytsev

    Juriy Zaytsev March 9th, 2009 @ 05:52 PM

    Making Prototype fail-safe with augmented Object.prototype is a quite significant performance hit. Same hit as if, for example, we would fix getElementById in IE (to ignore same-named elements).

    It doesn't mean we have to forget about it, though. We can simply make it an opt-in.

    What do others think?

  • Samuel Lebeau

    Samuel Lebeau March 9th, 2009 @ 07:58 PM

    It should at least be documented as Jim suggested. I think this is related to #382 and how Prototype handles object properties in general.

    This requires a real discussion...

  • Tobie Langel

    Tobie Langel March 10th, 2009 @ 08:29 AM

    • Assigned user set to “T.J. Crowder”
    • State changed from “new” to “doc”
  • Jim Higson

    Jim Higson March 10th, 2009 @ 02:34 PM

    As I'm sure we know, this can be made safe using:

    for( a in o ){
       if( !o.hasOwnProperty(a) ) continue;
    
       // code as usual
    }
    

    For speed-critical loops that are expected to have lots of iterations maybe one big test is better than lots of little ones:

    if( /* test if Object.prototype has been added to */ ){
       for( a in o ){
          if( !o.hasOwnProperty(a) ) continue;
    
          // code as usual
       }
    }else{
       for( a in o ){
          // duplicated code - maybe bad or move to function?
       }
    }
    

    This would need a hasOwnProperty implementation for old Safaris and IE5 (maybe in the Prototype object?)

    Do we have a set of standard Prototype benchmarks? I could add this into all of Prototype's for-in loops and experiment to see how much difference it actually makes.

    Incidentally, this is being discussed the Prototype-Scripty list:
    http://groups.google.co.uk/group/prototype-scriptaculous/browse_thr...

    And I blogged (slightly in frustration) about this here when my apps started to break:
    http://jimhigson.blogspot.com/2009/03/prototype-can-be-brittle.html

  • Juriy Zaytsev

    Juriy Zaytsev March 10th, 2009 @ 02:50 PM

    in would need to be replaced too.

  • Tobie Langel

    Tobie Langel May 12th, 2009 @ 04:02 PM

    • Milestone cleared.

    [responsible:ID#32948 milestone:ID#9627 bulk edit command]

  • Tobie Langel
  • T.J. Crowder

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

    • Assigned user cleared.

    [responsible:none bulk edit command]

  • Tobie Langel

    Tobie Langel November 29th, 2009 @ 07:18 PM

    • Milestone cleared.
    • Assigned user set to “Samuel Lebeau”
  • Tobie Langel

    Tobie Langel November 29th, 2009 @ 08:11 PM

    • State changed from “doc” to “open”
  • rus1804

    rus1804 May 5th, 2010 @ 02:59 PM

    It seems that using a namespace to augment Object.prototype will solve the problem:

    Object.prototype.test=function(){
      return 42;
    };
    

    will cause nasty errors,

    Object.prototype.javacream = {};
    Object.prototype.javacream.test=function(){
     return 42;
    };
    

    will work.

    Using namespaces is recommended in JavaScript, so imo there is no need to change prototype code.

  • T.J. Crowder

    T.J. Crowder May 5th, 2010 @ 04:33 PM

    It seems that using a namespace to augment Object.prototype will solve the problem.

    No. Adding any property to Object.prototype will break any for..in loop that assumes that the only properties on Object.prototype are not enumerable. The vast majority of for..in loops make that assumption. It doesn't matter whether you call the proprty test or javacream or hippy_hippy_mojo, it will still show up in people's for..in loops and make them behave unexpectedly.

    Adding to Object.prototype is a Bad Thing(tm). Don't do it. I don't think Prototype is any more fragile in this regard than any other library and the vast majority of non-library scripts out there (not that that means it shouldn't be documented, just that doing so is a hyper-low priority).

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 ยป

Shared Ticket Bins

Pages