How reliable are the polyfill/shim implementations on MDN

I have been looking through the polyfill implementations on the Mozilla Developer Network (MDN) as I require a few of these for a library. I know shim.js exists, but I'm not using that.

It seems that the polyfills are not consistent in code styling. It almost appears that they are written by the community in an almost "wiki" style.

Take for example String.prototype.contains

if(!('contains' in String.prototype)) {
    String.prototype.contains = function(str, startIndex) {
        return -1 !== String.prototype.indexOf.call(this, str, startIndex);
    }
}

it seems more logical to me to implement this as such:

if(!String.prototype.contains) {
    String.prototype.contains = function(str, startIndex) {
        return this.indexOf(str, startIndex) !== -1;
    }
}

Given that JavaScript is a size critical language (in that everything should be as small as possible for network transmission), my example should be favourable to the example on MDN as this saves a few bytes.

As the title suggests, I want to know how reliable the code is on MDN, and should I modify this as necessary to provide really clean, tiny implementations where possible?

Solution

It seems that your question refers to the article on String.contains().

Yes, MDN is a wiki so the quality of its content (including code examples) can vary. However, the content on general web topics (as opposed to extension development for example) is usually pretty good. Still, you shouldn't forget to use common sense.

The polyfill suggested on MDN and your version differ in three points:

!('contains' in String.prototype) vs. !String.prototype.contains to check whether a property exists: The former is clearly preferable. The in operator merely looks up a property, there are no side-effects. !String.prototype.contains on the other hand will actually retrieve the value of that property and convert it to a boolean value. Not only is this marginally slower, some property values like 0 will be wrongly coerced to false. You probably won't notice the difference with functions but this might become a real issue when polyfilling other property types.
-1 !== foo vs. foo !== -1 for comparisons: This is a matter of taste but some people prefer the former variant. The advantage of always putting the constant first in comparisons is that you won't unintentionally turn a comparison into an assignment: writing -1 = foo when you meant -1 == foo will cause an error. On the other hand, foo = -1 instead of foo == -1 will succeed and noticing that issue in your code might take a while. Obviously, if you choose to adapt that style you need to use it consistently throughout all your code.
String.prototype.indexOf.call vs. this.indexOf: The former guards against the situation that the indexOf method on the this object is overwritten. As a result, it is closer to the behavior of the native String.contains() function. Consider this example:

var a = "foo";
a.indexOf = function() {something_weird};
alert(a.contains("f"));

The native implementation of String.contains and a polyfill using String.prototype.indexOf.call will work even if this.indexOf is overwritten - a polyfill using this.indexOf however will fail.

Altogether, the code provided on MDN has a few more fail-safes. Whether these are required in your individual scenario is not given of course. However, dropping them to save a few bytes is the wrong approach to optimization ("premature optimization is the root of all evil"). Personally, I prefer good style over efficiency unless the difference in performance is known to be relevant.