Good JavaScript practices

as defined by the community

Project maintained by Phoenix35 Hosted on GitHub Pages — Theme by mattgraham

Feel free to join the SpeakJS discord server to discuss the matter.

Preface

This is a collection updated to fit the latest spec and practices in the community.
Relevant ESLint rules are linked within each section.
If you use a proper ESLint configuration file, you should already be notified for a lot of them.

Correct use of JS in the browser
Strict mode
No var
Casing
No globals
No more forEach
No prototype builtins

Correct use of JS in the browser

First, your <script> tag MUST be in the <head> part, and MUST NOT be inline. Ideally, make it an ES module.
This allows:

earliest possible download by the browser (<head>)
download in parallel with the rest of the page.
If inline, the HTML bytes are downloaded and rendered later, because the browser has to parse JS in the same thread before
parsing at the correct time (after your DOM is loaded)

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title>Amazing title</title>
  <link rel=stylesheet href="path/to/sheet.css">

  <!-- IDEALLY -->
  <script type=module src="path/to/module.js"></script>

  <!-- OR -->
  <script defer src="path/to/script.js"></script>

</head>
<body>

<!-- DO NOT -->
<script src="path/to/script.js"></script>

<!-- NEVER -->
<script>
/* JS code inside HTML */
</script>

</body>
</html>

Second, your HTML MUST NOT contain on* attributes. Simply because if you follow point #1, it doesn’t work.
Modules have their own scope, so the listener would be invisible to HTML

<!-- BAD -->
<button onclick="submitForm();">Submit</button>

// module.js

const myButton = document.querySelector("button");

// Not in the scope of HTML.
// An error will trigger on button click to say it's undefined
function submitForm (evt) {

}

IF you use regular scripts with the defer attribute in the <script> tag, the best practice is to prevent globals with an II(A)FE.

// script.js
(/* async */ () => {
"use strict";

const myButton = document.querySelector("button");

// Same problem. Not in scope.
function submitForm (evt) {

}
})();

Use .addEventListener in JS instead

<!-- GOOD -->
<button class="submit-button">Submit</button>
<!-- or any other way to identify the element -->

const myButton = document.querySelector(".submit-button"); // change the selector accordingly

function submitForm (evt) {

}

myButton.addEventListener("click", submitForm, options);

// OR an anonymous function works too if you don't intend to reuse it
myButton.addEventListener("click", (evt) => {

}, options);

The options argument is detailed here.
There are several reasons to use .addEventListener.

Strict mode

This is not arguable! Code in strict mode, ALWAYS!
What is strict mode?

Every JS file in the browser that is not an ES6 module MUST be written like this.

// Use async if you intend to use top-level await
(/* async */ () => {
"use strict";

// rest of your code here
})();

ECMAScript Modules are already in strict mode so there is no need for the II(A)FE nor this directive.

In node.js, the II(A)FE is not needed but strict mode still is.
For a file to be considered a module it must have the .mjs extension (note the m), or, project-wide, the package.json must contain "type": "module".
In any other case, you MUST indicate strict mode.

"use strict";

// your requires

// rest of your code here

See the ESM doc on node.js for more info.

No var

Ban var. Default to const instead of var.

/* BAD */
var n = 1;

/* BETTER */
let n = 1;

/* BEST */
const n = 1;

/* BAD */
for (var i = 0; i < n; i++)
  //

/* GOOD */
for (let i = 0; i < n; ++i)
  //

You can update your code by replacing every var with const and replace const with let when it fails (your linter will tell you).

Casing

Use camelCase convention for all variables that are not constructors.
Use PascalCase convention for all constructors.
You may UPPERCASE some constants.

function Item (category) { // this is a constructor
  this.category = category;
  return this;
}
function createItem (category = "") { // this is not
  return new Item(category); 
}

const someItem1 = createItem("cat1");
const someItem2 = new Item("cat2");

No globals

Do not needlessly pollute the global namespace.
https://eslint.org/docs/rules/no-global-assign
https://eslint.org/docs/rules/no-implicit-globals

In browsers

/* BAD */
"use strict";
const nItems = 10;
// rest of your code here

/* GOOD */
"use strict";
(function () {
  const nItems = 10;
  // rest of your code here
})();

/* GOOD alternative */
"use strict";
{
const nItems = 10;
// rest of your code here
}

In modules or userscripts

const nItems = 10;  // modules have their own scope so it's fine
// rest of your code here

No more forEach

Ban .forEach()! It only comes with downsides.

Lack of control flow.
continue; can be simulated by return;, but break is not possible

Not async friendly.

let counter = 0;
const nums = [ 2, 8, 10 ];
nums.forEach(async (num) => {
  await something();
  counter++;
});
console.log(counter); // 0

const urls = [ "https://remote.com/1", "https://remote.com/2", "https://remote.com/3", ... ];
urls.forEach(async (url) => {
  const response = await fetch(url); // This is doing N CONCURRENT requests, you are spamming the server
  doStuff(response);
});

Looks like functional programming, while it is anything but.
The purpose of .forEach is to have side-effects
Doesn’t get to the point.
All the other Higher-Order Functions (HOFs) tell you what they do in their names. .forEach does everything so nothing
Lends itself to misuse.
- return something; doesn’t work.
- People may want to chain it, like the other HOFs.
- asyncing the callback will bite you
The only time you would have a reason to use .forEach – when you want a side-effect – it is superseded by for-of.
It is compatible with any iterable instead of being a method of a select few, and has none of the downsides listed above

This feeling is backed up by an authority

Act on value only

/* BAD */
iterable.forEach((value) => {
  
});

/* GOOD */
for (const value of iterable) {
  
}
// or
for (const value of iterable.values()) {
  
}
// if values of an object
for (const value of Object.values(obj)) {
  
}

Act on index/key only

/* BAD */
iterable.forEach((_, key) => {
  
});

/* GOOD */
for (const key of iterable.keys()) {
  
}
// if keys of an object
for (const key of Object.keys(obj)) {
  
}

Act on both key and value

/* BAD */
iterable.forEach((value, key) => {
  
});

/* GOOD */
for (const [ key, value ] of iterable.entries()) {
  
}
// if an object
for (const [ key, value ] of Object.entries(obj)) {
  
}

The whole functionality

/* BAD */
iterable.forEach(cb, thisArg);

/* GOOD */
for (const [key, value] of iterable.entries()) {  // adapt to whatever you actually need
  cb.call(thisArg, value, key, iterable);
}

Higher-order functions

Eloquent JavaScript devoted an entire chapter to them.
We’re going to see what can be used instead of the dreaded forEach.

Output an array the same length as the starting array

.map Also called a transform, or a mapping operation.

/* BAD */
const oldPlus2 = [];

oldArr.forEach((number) => {
  oldPlus2.push(number + 2);
});

/* GOOD */
const oldPlus2 = oldArr.map((number) => number + 2);

Select entries

.filter() If you want a new array with entries selected by a predicate.

/* BAD */
const onlyEvens = [];

oldArr.forEach((integer) => {
  if (integer % 2 === 0)
    onlyEvens.push(onlyEvens);
});

/* GOOD */
const onlyEvens = oldArr.filter((integer) => integer % 2 === 0);

Check if a condition is met by at least one entry

.some() To check if whether at least one element (early exit) in the array passes the predicate.
/!\ This example is a tad more complicated /!\

const authzCreds = [
  {
    id: 11,
    pubKey: "3a",
  },
  {
    id: 16,
    pubKey: "b0",
  },
  {
    id: 99,
    pubKey: "66",
  }
];

const group = [
  {
    id: 11,
    name: "Jack",
    pubKey: "66",  // pubKey will not pass
  },
  {
    id: 99,
    name: "James",
    pubKey: "66",  // You good
  },
  {
    id: 13,
    name: "Rose",
    pubKey: "3a",  // ID will not pass
  },
];

/* BAD */
// This is executed for EVERY element!
// We have to have a supplementary state in case we only need to proceed once
let hasExecuted = false;
group.forEach(({ id: personId, pubKey: personPubKey }) => {
  if (hasExecuted)
    return;

  let isAllowed = false;
  authzCreds.forEach((cred) => {
    if (personId === cred.id && personPubKey === cred.pubKey)
      isAllowed = true;
  });

  if (isAllowed) {  // Proceed only once.
    proceed();
    hasExecuted = true;
  }
});

/* GOOD */
// Loop can stop as soon as the condition is met
for (const { id, pubKey } of group) {
  if (
    authzCreds.some((cred) => id === cred.id && pubKey === cred.pubKey)
  ) {
    proceed();
    break;
  }
}

/* GOOD alternative */
for (const { id, pubKey } of group) {
  if (
    !authzCreds.some((cred) => id === cred.id && pubKey === cred.pubKey)
  )
    continue;
  proceed();
  break;
}

Check if a condition is met by all entries

.every() Early exit if one does not meet.

/* WHAT EVEN? */
let canContinue = true;
onlyEvens.forEach((n) => {
  if (n % 2 !== 0)
    canContinue = false;
});
if (canContinue)
  proceed();

/* GOOD */
if (onlyEvens.every((n) => n % 2 === 0))
  proceed();

Produce a single value out of an array

.reduce()

const someBytes = new Uint8Array([0x10, 0xFF, 0x30, 0x00, 0x10]);
const bLength = BigInt(someBytes.BYTES_PER_ELEMENT * 8);

/* BAD */
let newBInt = 0n;
someBytes.forEach((currUint) => {
  newBInt = (newBInt << bLength) + BigInt(currUint);
});


/* GOOD */
const newBInt = someBytes.reduce(
  (accBigInt, currUint) => (accBigInt << bLength) + BigInt(currUint),
  0n
);
newBInt; // 0x10FF300010n === 73000812560n;

Find the first index meeting a certain condition

.findIndex() returns -1 if not found, like its brother indexOf

const fibNumbers = [1, 1, 2, 3, 5, 8, 13, 21];

console.log(`The first Fibonacci number above 10 is #${
  fibNumbers.findIndex((number) => number > 10) + 1  // 7
}`);

Find the first value meeting a certain condition

.find() returns undefined if the test is not passed. It returns a reference to the object, not a copy!

const remoteDB = [
  {
    name: "B",
    version: 3,
  },
  {
    name: "A",
    version: 0,
  },
];

const localDB = [
  {
    name: "A",
    description: "Perspiciatis saepe sunt qui quo molestiae rerum.",
    version: 1,
  },
  {
    name: "B",
    description: "Blanditiis architecto sed est facilis qui.",
    version: 1,
  },
];

function updateLocalVersionFrom (db) {
  for (const newEntry of db) {
    const localEntry = localDB.find(({ name }) => name === newEntry.name);  // By reference!
    if (localEntry.version < newEntry.version)
      localEntry.version = newEntry.version;
  }
}

updateLocalVersionFrom(remoteDB);
// 0: Object { name: "A", ..., version: 1 }
// 1: Object { name: "B", ..., version: 3 }

Summary

.forEach is useless

Sharp minds might have noticed that all higher-order functions can actually be written with for...of inside blocks instead.
In my opinion, intent can usually be better expressed with higher-order functions than with loops.

No prototype builtins

Do not assume a safe prototype. Do not create an intermediate object.
https://eslint.org/docs/rules/no-prototype-builtins

/* BAD */
someObj.hasOwnProperty("prop");  // Not safe
({}).hasOwnProperty.call(someObj, "prop");  // Intermediate empty object

/* GOOD */
Object.prototype.hasOwnProperty.call(someObj, "prop");  // Safe, one time use
// or
const _hasOwnProperty = Object.prototype.hasOwnProperty;  // Safe, several times use
_hasOwnProperty.call(someObj, "prop");

/* BAD */
const arr = ([]).slice.call(iterable);  // Intermediate empty array

/* GOOD */
// If you don't map
const arr = [...iterable];
// If you map
const arr = Array.from(iterable, mapperFn);

// if array-like and not iterable
const arrayLikeObjLength = arrayLikeObj.length;
const arr = [];

for (let i = 0; i < arrayLikeObjLength; ++i)
  arr.push(arrayLikeObj[i]);