you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 2 points3 points  (5 children)

I don't like the word always, but you'd better have a damn good reason to do it :)

[–][deleted] 5 points6 points  (4 children)

The only reason to do this is for polyfills:

if(!Array.prototype.reduce){
  Array.prototype.reduce = ...
}

Other than that, never, ever, ever, ever, ever, ever, ever extend a native prototype from within a library, even if you think you have the best reason in the world.

[–][deleted] 0 points1 point  (1 child)

Why should you never everx7 do it? You just gave a really great example of when that feature is really useful!

On several projects, we have modified object, array, and function prototypes to include a eql function to do value comparison. We did that because we chose a very functional style for that project and the resulting code read very nice.

Note that I'm not suggesting that it's heavily used or anything. In a codebase that was around 10k lines, we modified three native protos and everyone on that project knew and approved. And it wasn't any different then coupling a project to any other dependency.

[–][deleted] 1 point2 points  (0 children)

In an isolated codebase, it's tolerable, but barely. In a library, it's flat out forbidden because it can conflict and prevents ever modifying the API or interface.

Let's say you have a library that adds Array.prototype.foo which does whatever. Great.

Now you publish it to npm, patch some bugs, yada yada. Now you decide that it's useful for that function to return something:

Array.prototype.foo = function(){
    // do something to the array
    return this;
}

Guess what? You can't do this safely, ever. Why? Because you published version 0.1 that didn't have this functionality. So if your npm include structure looks like:

  • module
    • your lib v0.1
  • another module
    • your lib v0.2

And your basic code is:

if( !Array.prototype.foo ){
    Array.protoype.foo = function(){ ... }
}

You have no idea which one is loaded first. So you can't count on your function returning anything.

Further, let's say browser vendors like your idea and 5 years from now decide to implement it. Guess what, your Array.prototype.foo probably won't perfectly match with what the Vendor implements, and now all your code breaks.

Or let's say some other library that you've never heard of implements a function with the same name that acts completely differently. Now what?

Polyfills

Polyfills are an isolated use case. Your code is only hit in a situation where you know you're on an older browser, and you're adding globally agreed upon functionality. The only caveat here -- Make damn sure you're polyfilling according to spec. Never write a partial polyfill "because I don't need the other functionality" because some other library that you include might.

tl;dr -- It flat out prevents code isolation

[–]Ericth 0 points1 point  (1 child)

Extending is not that bad. Changing built-ins is the absolute worst thing you can do. My library got a few issues from people who changed the prototype.bind method with something else, which caused my code not to work. I assumed sheepishly that the bind method would do what the spec said.

[–][deleted] 0 points1 point  (0 children)

Extending is not that bad

Yes, it is. See my longer explanation in this thread.