Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Native prototypes are modified (unnecessarily) #518

Closed
cowboy opened this Issue · 2 comments

3 participants

Ben Alman Josh Perez Anton Kovalyov
Ben Alman

@scottgonzalez noticed some strange behavior in grunt today. Apparently, when accessing the .name property of a string (accidentally, it should've been an object with a name property but a string was used instead), this output was observed:

/*! Amplify v1.1.0 - http://amplifyjs.com
 * Copyright (c) 2012 function () {

// If the string looks like an identifier, then we can return it as is.
// If the string contains no control characters, no quote characters, and no
// backslash characters, then we can simply slap some quotes around it.
// Otherwise we must also replace the offending characters with safe
// sequences.

            if (ix.test(this)) {
                return this;
            }
            if (nx.test(this)) {
                return '"' + this.replace(nxg, function (a) {
                    var c = escapes[a];
                    if (c) {
                        return c;
                    }
                    return '\\u' + ('0000' + a.charCodeAt().toString(16)).slice(-4);
                }) + '"';
            }
            return '"' + this + '"';
        }; Licensed 
 * http://appendto.com/open-source-licenses */

I tracked it down to this code in JSHint: https://github.com/jshint/jshint/blob/master/jshint.js#L892-913

It's probably a bad idea for any lib that wasn't designed specifically to modify or augment native prototypes to do so.

Ben Alman

(there are at least 5 methods in JSHint that extend native prototypes)

Josh Perez

This we inherited from jslint. I'll check it out further a bit later.

Anton Kovalyov valueof referenced this issue from a commit
Anton Kovalyov valueof Don't extend String.prototype with non-standard methods.
Extending String.prototype is generally a not very wise idea
that leads to embarassing bugs such as GH-518. This patch
removes all instances of JSHint extending String.prototype
with non-standard methods and removes two of them completely
because they were not used.

Closes GH-518.
c581b43
Anton Kovalyov valueof closed this issue from a commit
Anton Kovalyov valueof Don't extend String.prototype with non-standard methods.
Extending String.prototype is generally a not very wise idea
that leads to embarassing bugs such as GH-518. This patch
removes all instances of JSHint extending String.prototype
with non-standard methods and removes two of them completely
because they were not used.

Closes GH-518.
c581b43
Anton Kovalyov valueof closed this in c581b43
jugglinmike jugglinmike referenced this issue from a commit in jugglinmike/jshint
Anton Kovalyov valueof Don't extend String.prototype with non-standard methods.
Extending String.prototype is generally a not very wise idea
that leads to embarassing bugs such as GH-518. This patch
removes all instances of JSHint extending String.prototype
with non-standard methods and removes two of them completely
because they were not used.

Closes GH-518.
7cf5807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.