silvermine/eslint-config-silvermine

Consider rule to disallow global use of `event`

jimjenkins5 opened this issue · 0 comments

The problem:

Using the variable name event can hide errors in javascript.

Most browsers (IE, Chrome, and Safari) declare a global window.event before running the handlers, so running your code without declaring the variable could look like it works great. However, Firefox, at least, does not declare the window.event variable, so your code will error in FF.

Examples:

"Bad:"

// Works in all browsers except Firefox
$('a').click(function() {
   ... do something with event
});

// Works in all browsers, but is dangerous, because if you 
// forget to add `event` to the function declaration, you will 
// break Firefox and not be given an error
$('a').click(function(event) {
   ... do something with event
});

"Good"

// Works across the board and doesn't cause confusion
$('a').click(function(evt) {
   ... do something with evt
});

Recommended solution:

ESLint has 2 rules that can help:

  1. no-restricted-globals
    • Adding event to this list will prevent the usage of event as a global (i.e. without declaring it)
  2. no-restricted-properties
    • Adding { object: "event" } to this list will prevent declaring a variable named event.

I recommend that we add at least the first rule to our base config. Adding the second rule is optional, but would avoid ambiguity in our code

References

https://stackoverflow.com/questions/16306486/unexpected-access-to-event-variable-across-browsers