Kieran Barker

Never rely on the global event property in JavaScript

When writing event listeners in JavaScript, you should never rely on the global window.event property.

The MDN Web Docs explain:

You should avoid using this property in new code, and should instead use the Event passed into the event handler function. This property is not universally supported and even when supported introduces potential fragility to your code.

I was reviewing my password toggle script yesterday and I found that I had fallen into this trap.

The problem

I had the following function...

/**
 * Toggle the visibility of a single password field
 * @param {Object} password The password field
 */
function togglePassword (password) {
  password.type = event.target.checked ? "text" : "password";
}

...That I passed into the Array.forEach() method at the end of another function:

/**
 * Toggle all password fields inside a single form
 * @param {Object} event The Event object
 */
function togglePasswords (event) {

  // If this isn't a password toggle, do nothing
  if (!event.target.matches("[data-toggle]")) return;

  // Get the closest form
  // If it doesn't exist, do nothing
  var form = event.target.form;
  if (!form) return;

  // Get all password fields inside this form
  var passwords = Array.from(
    form.querySelectorAll("[data-password]")
  );

  // Toggle the password fields
  passwords.forEach(togglePassword);

}

The problem was that I didn't explicitly define the event as a parameter of the togglePassword() function. This means it was relying on the global window.event property.

The solution

To fix this, I modified the togglePassword() function to accept the event as a second argument:

/**
 * Toggle the visibility of a single password field
 * @param {Object} password The password field
 * @param {Object} event    The Event object
 */
function togglePassword (password, event) {
  password.type = event.target.checked ? "text" : "password";
}

Then I passed in the event explicitly:

// Toggle the password fields
passwords.forEach(function (password) {
  togglePassword(password, event);
});

Lexical scoping

If you know me, you'll know that I like named functions. Therefore, it's reasonable to ask why I invoked the togglePassword() function within an anonymous function.

This is because when you use the forEach() method, you pass in a callback function. The forEach() method automatically passes specific arguments into your callback function:

  1. The current element being processed
  2. The index of the current element being processed
  3. The array on which forEach() was called

You can name these arguments whatever you like. Therefore, if I did the following, as I did originally...

// Toggle the password fields
passwords.forEach(togglePassword);

...Then the forEach() method would call the togglePassword() function with the index of the current element as the second argument.

Although the togglePassword() function has event defined as its second parameter, it's just a name. The forEach() method would pass in the index of the current element.

This would throw a TypeError, because it would be trying to access the target property of a number, which doesn't exist and is therefore undefined:

Uncaught TypeError: Cannot read property 'checked' of undefined

Instead, by using the anonymous function...

// Toggle the password fields
passwords.forEach(function (password) {
  togglePassword(password, event);
});

...The call to forEach() works.

This is because the togglePassword() function can access the event from the lexical scope of the outer togglePasswords() function.

More information

For more information on event delegation and scoping, I recommend the following articles by Chris Ferdinandi and Todd Motto: