60

Google JavaScript Style Guide advises against extending the Array.prototype. However, I used Array.prototype.filter = Array.prototype.filter || function(...) {...} as a way to have it (and similar methods) in browsers where they do not exist. MDN actually provides similar example.

I am aware about Object.prototype issues, but Array is not a hash table.

What issues may arise while extending Array.prototype that made Google advise against it?

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
Andrey Shchekin
  • 21,101
  • 19
  • 94
  • 162

11 Answers11

86

Most people missed the point on this one. Polyfilling or shimming standard functionality like Array.prototype.filter so that it works in older browsers is a good idea in my opinion. Don't listen to the haters. Mozilla even shows you how to do this on the MDN. Usually the advice for not extending Array.prototype or other native prototypes might come down to one of these:

  1. for..in might not work properly
  2. Someone else might also want to extend Array with the same function name
  3. It might not work properly in every browser, even with the shim.

Here are my responses:

  1. You don't need to use for..in on Array's usually. If you do you can use hasOwnProperty to make sure it's legit.
  2. Only extend natives when you know you're the only one doing it OR when it's standard stuff like Array.prototype.filter.
  3. This is annoying and has bit me. Old IE sometimes has problems with adding this kind of functionality. You'll just have to see if it works in a case by case basis. For me the problem I had was adding Object.keys to IE7. It seemed to stop working under certain circumstances. Your mileage may vary.

Check out these references:

Good luck!

Lucy Bain
  • 2,496
  • 7
  • 30
  • 45
Jamund Ferguson
  • 16,721
  • 3
  • 42
  • 50
  • 12
    "You don't need to use for..in" -- don't use it at all. Even with `hasOwnProperty`, you'll still step over `length`, which makes no sense in most cases. – Michael Lorton Jan 15 '12 at 18:19
  • 2
    @Malvolio Disagree: You cannot easily know / control what is in your 3rd party libraries. For instance, `for..in` breaks in the SEA3D loader for Three.js, with my additions to `Array.prototype`. In fact, much of three.js uses `for..in`. Really, BEWARE. These are not pleasant bugs to have to find. – Engineer Jul 02 '14 at 08:57
  • @mrdoob Could this be worth looking into? – Engineer Jul 02 '14 at 09:00
  • 2
    @NickWiggill, for .. in is not designed for arrays, it is designed for arraylike objects (read associative arrays). You should never use for .. in for arrays. – David Jan 18 '15 at 14:21
  • If you control all your application's code this is great. If you work on a platform (ahem, SharePoint) you risk breaking platform code with issues that will manifest in strange ways on completely unrelated functionality. And since the actual error will occur (usually introduced via a for..in) in minifed platform code it's really difficult to diagnose. – Derek Gusoff Apr 12 '17 at 15:25
  • 2
    you can avoid enumeration problems extending the Array.prototype object with [`Object.defineProperty()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty) – Soldeplata Saketos Apr 03 '18 at 10:43
  • This is 2018 now. We have `for..of` now. So i do this: `Array.prototype.RemoveElement = function (el) { this.splice(this.indexOf(el), 1); };` – Fandi Susanto Jun 11 '18 at 14:21
9

I'll give you the bullet points, with key sentences, from Nicholas Zakas' excellent article Maintainable JavaScript: Don’t modify objects you don’t own:

  • Dependability: "The simple explanation is that an enterprise software product needs a consistent and dependable execution environment to be maintainable."
  • Incompatible implementations: "Another peril of modifying objects that you don’t own is the possibility of naming collisions and incompatible implementations."
  • What if everyone did it?: "Simply put: if everyone on your team modified objects that they didn’t own, you’d quickly run into naming collisions, incompatible implementations, and maintenance nightmares."

Basically, don't do it. Even if your project is never going to be used by anyone else, and you're never going to import third party code, don't do it. You'll establish a horrible habit that could be hard to break when you start trying to play nice with others.

James Sumners
  • 14,485
  • 10
  • 59
  • 77
  • 2
    There's nothing wrong with extending host objects to make them standards compliant if the team agrees to it. Extending host objects with non-standard properties is a different game, and I would agree "don't do it" – Raynos Jan 14 '12 at 03:26
  • 1
    Any reason you can come up with to modify the host objects can easily be overcome by implementing the feature on your own objects. The documentation for the host environment doesn't include your modifications. This can cause confusion no matter what the "team" agrees upon. What about the new guy who comes in and doesn't know all of your host modifications but you've told him it's okay to change host objects? Things can break in a hurry. – James Sumners Jan 14 '12 at 03:34
  • 3
    This reasoning in the fact advises against OOP – OnTheFly Jan 14 '12 at 03:39
  • _"The documentation for the host environment doesn't include your modifications."_ - What documentation? We're mostly talking here about supporting unknown web browsers that may or may be standards compliant, and just adding in missing features. If you are extending host objects to make them standards compliant (as Raynos mentioned) then presumably as in the question you'd first test if the function already existed and only add your own version if needed. – nnnnnn Jan 14 '12 at 03:54
  • @user539484, no it doesn't. It advises being responsible and not thrashing your standard library. If you want to add to it, feel free, but do so on your own objects (subclasses if you will). – James Sumners Jan 14 '12 at 04:11
  • @nnnnnn That doesn't make it good, or recommended, practice. The reasons stated in the article are why the Google advisement exists. – James Sumners Jan 14 '12 at 04:15
  • 2
    @jsumners, what if John or Jane already wrote `VeryUsefulObject` but it still missing `exactlyWhatINeed` method? – OnTheFly Jan 14 '12 at 04:19
  • But it _is_ recommended for that purpose. It's not _universally_ recommended, and obviously some people recommend _against_ it, but still... – nnnnnn Jan 14 '12 at 04:25
  • @jsumners so your saying that [ES5-shim](https://github.com/kriskowal/es5-shim) and [DOM-shim](https://github.com/Raynos/DOM-shim) are not useful or correct in their editing of natives with _well-defined_ behaviour? – Raynos Jan 14 '12 at 11:23
  • @Raynos from that first link: "In many cases, this means that these shims cause many ES5 methods to silently fail." – James Sumners Jan 14 '12 at 15:59
  • @jsumners so what? That's a bonus, You either get well defined behaviour, or as close as you can to it. It's better then using these methods and they don't work at all. The case here is that you need to extend natives with missing features if you want to code to modern standards. You've got no choice and there's nothing wrong with it. – Raynos Jan 14 '12 at 16:11
  • Breaking standard methods that already exist is a bonus? Okay. – James Sumners Jan 14 '12 at 16:17
  • @jsumners `VeryUsefulObject.prototype.exactlyWhatINeed = VeryUsefulObject.prototype.exactlyWhatINeed || function(){ … }`. Bam. Nothing's being broken here. – vzwick May 30 '12 at 20:31
  • @jsumners I gotta give to you, though, that Raynos and I had a pretty special case of messing with others' objects in mind. Shimming, if done right, is incredibly useful. Generally though, messing with others' stuff is most likely a bad idea for the exact reasons you gave. On a sidenote: Much of the beauty of JS lies in the fact that you *can* mess with others' stuff in a way that is quite unlikely to ever cause any of the above problems if done right. – vzwick May 30 '12 at 20:37
  • "The simple explanation is that an enterprise software product needs a consistent and dependable execution environment to be maintainable." reads to me like: "leverage good tools that bring your execution environment to a known and documented state". I think polyfills are an acceptable compromise here, unless you would rather just not use array at all. – Adam Tolley May 02 '14 at 14:31
5

As a modern update to Jamund Ferguson's answer:

Usually the advice for not extending Array.prototype or other native prototypes might come down to one of these:

  1. for..in might not work properly
  2. Someone else might also want to extend Array with the same function name
  3. It might not work properly in every browser, even with the shim.

Points 1. and 2. can now be mitigated in ES6 by using a Symbol to add your method.

It makes for a slightly more clumsy call structure, but adds a property that isn't iterated over and can't be easily duplicated.

// Any string works but a namespace may make library code easier to debug. 
var myMethod = Symbol('MyNamespace::myMethod');

Array.prototype[ myMethod ] = function(){ /* ... */ };

var arr = [];

// slightly clumsier call syntax
arr[myMethod]();

// Also works for objects
Object.prototype[ myMethod ] = function(){ /* ... */ };

Pros:

  • For..in works as expected, symbols aren't iterated over.
  • No clash of method names as symbols are local to scope and take effort to retrieve.

Cons:

Community
  • 1
  • 1
thelastshadow
  • 3,406
  • 3
  • 33
  • 36
  • arr[myMethod]() -- Cantcha wrap that in a simpler call? – johny why Apr 25 '20 at 06:25
  • @johnywhy yes, but if you do that (say by adding `Array.prototype.f = Array.prototype[ myMethod ]`) then that method is available to (and might clash with) other library code. – thelastshadow May 17 '20 at 21:08
3

Prototype does this. It's evil. The following snippet demonstrates how doing so can produce unexpected results:

<script language="javascript" src="https://ajax.googleapis.com/ajax/libs/prototype/1.7.0.0/prototype.js"></script>
<script language="javascript">
  a = ["not", "only", "four", "elements"];
  for (var i in a)
    document.writeln(a[i]);
</script>

The result:

not only four elements function each(iterator, context) { var index = 0; . . .

and about 5000 characters more.

wadim
  • 197
  • 2
  • 6
  • 4
    they're not evil, the evil part is your code, if you wrap this in .hasOwnProperty only it's own properties show up, no offense – Steel Brain Dec 01 '14 at 11:25
  • 1
    I think it's not syntactically elegant to use .hasOwnProperty for iterating over an array. – wadim Dec 02 '14 at 12:08
  • 1
    Why would you use for ... in instead of forEach? No need for has ownproperty or messing with scope variables – CervEd Oct 17 '16 at 00:25
3

I want to add an additional answer that allows extending the Array prototype without breaking for .. in loops, and without requiring use of hasOwnPropery:

Don't use this bad approach which causes prototype values to appear in for .. in:

Array.prototype.foo = function() { return 'foo'; };
Array.prototype.bar = function() { return 'bar'; };

let a = [ 1, 2, 3, 4 ];
console.log(`Foo: ${a.foo()}`);
console.log(`Bar: ${a.bar()}`);
console.log('==== Enumerate: ====');
for (let v in a) console.log(v);

Instead use Object.defineProperty, with enumerable: false - it exists for pretty much exactly this reason!

Object.defineProperty(Array.prototype, 'foo', {
  value: function() { return 'foo'; },
  enumerable: false
});
Object.defineProperty(Array.prototype, 'bar', {
  value: function() { return 'bar'; },
  enumerable: false
});

let a = [ 1, 2, 3, 4 ];
console.log(`Foo: ${a.foo()}`);
console.log(`Bar: ${a.bar()}`);
console.log('==== Enumerate: ====');
for (let v in a) console.log(v);

Note: Overall, I recommend avoiding enumerating Arrays using for .. in. But this knowledge is still useful for extending prototypes of classes where enumeration is appropriate!

Gershom Maes
  • 7,358
  • 2
  • 35
  • 55
3

Extending Array.prototype in your own application code is safe (unless you use for .. in on arrays, in which case you need to pay for that and have fun refactoring them).

Extending native host objects in libraries you intend others to use is not cool. You have no right to corrupt the environment of other people in your own library.

Either do this behind an optional method like lib.extendNatives() or have [].filter as a requirement.

Extending Natives and Host Objects

Raynos
  • 166,823
  • 56
  • 351
  • 396
2

Some people use for ... in loops to iterate through arrays. If you add a method to the prototype, the loop will also try to iterate over that key. Of course, you shouldn't use it for this, but some people do anyway.

Tikhon Jelvis
  • 67,485
  • 18
  • 177
  • 214
  • They think what they are *iterating*, but in reality they are *enumerating* :-) Such code is broken already, there is no point to support it anyway. – OnTheFly Jan 14 '12 at 03:53
  • It's stupid code, sure, but it's still something to be aware of: if you add something to the prototype and start having weird bugs, it could be because of this. – Tikhon Jelvis Jan 14 '12 at 04:04
  • My point is what this pitfall assumes one has fallen into the pit already :) – OnTheFly Jan 14 '12 at 04:23
1

You can easily create somekind of sandbox with poser library.

Take a look on https://github.com/bevacqua/poser

var Array2 = require('poser').Array();
// <- Array

Array2.prototype.eat = function () {
  var r = this[0];
  delete this[0];
  console.log('Y U NO .shift()?');
  return r;
};

var a = new Array2(3, 5, 7);

console.log(Object.keys(Array2.prototype), Object.keys(Array.prototype))
test30
  • 3,496
  • 34
  • 26
1

I believe this question deserves an updated ES6 answer.

ES5

First of all, as many people have already stated. Extending the native prototypes to shim or polyfill new standards or fix bugs is standard practice and not harmful. For example if a browser doesn't support the .filter method if (!Array.prototype.filter) you are free to add this functionality on your own. In-fact, the language is designed to do exactly this to manage backwards compatibility.

Now, you'd be forgving for thinking that since JavaScript object use prototypal inheritance, extending a native object like Array.prototype without interfering should be easy, but up until ES6 it's not been feasible.

Unlike objects for example, you had to rely and modifying the Array.prototype to add your own custom methods. As others have pointed out, this is bad because it pollutes the Global namespace, can interfere with other code in an unexpected way, has potential security issues, is a cardinal sin etc.

In ES5 you can try hacking this but the implementations aren't really practically useful. For more in depth information, I recommend you check out this very informative post: http://perfectionkills.com/how-ecmascript-5-still-does-not-allow-to-subclass-an-array/

You can add a method to an array, or even an array constructor but you run into issues trying to work with the native array methods that rely on the length property. Worst of all, these methods are going to return a native Array.prototype and not your shiny new sub-class array, ie: subClassArray.slice(0) instanceof subClassArray === false.

ES6

However, now with ES6 you can subclass builtins using class combined with extends Array that overcomes all these issues. It leaves the Array.prototype intact, creates a new sub-class and the array methods it inherits will be of the same sub-class! https://hacks.mozilla.org/2015/08/es6-in-depth-subclassing/

See the fiddle below for a demonstration: https://jsfiddle.net/dmq8o0q4/1/

CervEd
  • 3,306
  • 28
  • 25
0

Extending the prototype is a trick that only works once. You do and you use a library that also does it (in an incompatible way) and boom!

Michael Lorton
  • 43,060
  • 26
  • 103
  • 144
  • That's why we only use well written compatible libraries. – Raynos Jan 14 '12 at 03:20
  • 1
    A rule that only works if you define "well-written" as including "does not extend Array.prototype"! – Michael Lorton Jan 15 '12 at 18:17
  • 3
    Of course... That's kind of the definition of well written, well written libraries don't corrupt the environment they don't own. – Raynos Jan 15 '12 at 18:22
  • 4
    That's a good answer to the original question: "don't corrupt the environment you don't own". You aren't the only one using Array -- don't mess with it. – Michael Lorton Jan 16 '12 at 00:03
0

The function you are overriding could be used by the internal javascript calls and that could lead to unexpected results. Thats one of the reasons for the guideline

For example I overrode indexOf function of array and it messed up accessing array using [].

Schu
  • 1,124
  • 3
  • 11
  • 23
  • 1
    I doubt it messes up `[]`. Unless you did something horribly wrong – Raynos Jan 14 '12 at 03:20
  • Yup it did because [] access now returns a indexOf override function instead of data. – Schu Jan 14 '12 at 03:23
  • 2
    When you define your own `indexOf` surely you first test that the function doesn't already exist? Add your own only when the browser doesn't already support it. – nnnnnn Jan 14 '12 at 03:47
  • 1
    This doesn't seem like an issue of extending a native prototype, but rather an issue of clobbering standard methods with non standard ones. –  Jan 14 '12 at 04:23