Топ 10 ошибок в JavaScript

Знакома ли вам ситуация, когда код вроде написан правильно, но все равно не работает как ожидается? Я бы сказал, классика для JavaScript... Но только когда ты не знаешь про неочевидные нюансы языка 🤓

Давайте разберем топ 10 ошибок, которые я до сих пор часто встречаю как в своей работе, так и за ее пределами. Для простоты восприятия, постарался расположить их в порядке важности и / или частоты - от меньшего к большему

10. Поверхностное копирование spread-оператором

Мы знаем, что можно скопировать объект, просто развернув его в новый объект, что-то вроде const userCopy = { ...user }. Однако проблема появляется, когда нужно скопировать сложную структуру

Дело в том, что оператор spread создает лишь поверхностную копию - так называемую Shallow Copy. Взгляните на этот код:

const user = { name: 'Alice', settings: { theme: 'dark' } };
const userCopy = { ...user };

userCopy.name = 'John';
userCopy.settings.theme = 'light';

// Alice - здесь как и ожидается
console.log(user.name);
// light — а должно быть dark!
console.log(user.settings.theme);

Здесь наглядно видно, что первый уровень вложенности действительно скопировался, но в обоих объектах осталась та же самая ссылка на settings, поэтому изменяя значение темы в копии, мы получаем изменения и там, и там

Этот способ подходит, когда мы точно знаем, что объект "плоский" и у него нет сложных вложенных структур. Применяя такой способ копирования так же стоит помнить, что spread совершает операцию за O(n), то есть перебирает объект ключ за ключом

Итак, как сделать глубокое клонирование объекта? Кстати говоря, на собеседованиях об этом часто спрашивают. Есть несколько вариантов:

• Написать свой deepClone (Сомнительно)
• JSON.parse(JSON.stringify(obj))
• Какой-нибудь lodash.cloneDeep(obj)
• structuredClone(obj)

JSON.parse(JSON.stringify(obj)) - неплохой хак. Но:

• Непроизводительная операция
• Сериализует объекты Date в ISOString (1970-01-01T00:00:00.123Z)
• Отбрасывает функции, свойства со значением undefined и не умеет сериализовать Set, Map, Regex и подобные особые типы

structuredClone(obj) - великолепный выбор, если у вас современная среда и в данных нет тех же функций, Set, Map и подобных особых типов. Доступен в современных браузерах и Node.js с 17 версии (MDN)

const user = { name: 'Alice', settings: { theme: 'dark' } };
const userDeepCopy = structuredClone(user);

В ином случае придется прибегнуть к использованию сторонних библиотек, либо изобретению велосипеда с костылями вместо спиц 😊

9. Опасная деструктуризация

Допустим, мы хотим нарисовать имя пользователя где-нибудь на клиенте. Мы получаем пользователя тем или иным способом, берем с него имя и используем там, где нам нужно

const { name } = getUser();
console.log(name);

Но что если наш пользователь не найден? В случае, если getUser() вернет null или undefined, мы получим ошибку TypeError: cannot destruct property name of null

Чтобы такого не возникало, добавляйте фоллбек (Fallback) значение там, где разворачиваете потенциально несуществующий объект:

const { name } = getUser() ?? {};
console.log(name) // undefined, но ошибки нет

Или вот еще пример

const response = await fetch('http://example.domain/api/users');
// Упадем, если апи вернуло data: null
const { data: [{ name }] } = await response.json();

В случаях с апи лучше сделать явную проверку. Что-то такое:

const response = await fetch('http://example.domain/api/users');
const json = await response.json();

if (!json?.data?.length) {
  return;
}

const [user] = data;
console.log(user.name);

8. async коллбэк в forEach

Допустим, мы хотим последовательно сделать какие-то одинаковые запросы один за одним. Интуитивно можно прийти к такому варианту:

const stepIds = [1, 2, 3];

stepIds.forEach(async (id) => {
  const step = await loadStep(id);
  renderStep(step);
});

console.log('Все шаги загружены!');

Как вы думаете, запросы выполняются последовательно? Как бы не так:

const stepIds = [1, 2, 3];

stepIds.forEach(async (id) => {
  console.log(`Начало загрузки шага ${id}`);
  const step = await loadStep(id);
  console.log(`Шаг ${id} загружен`);
});

console.log('Все шаги загружены!');

// Начало загрузки шага 1
// Начало загрузки шага 2
// Начало загрузки шага 3
// Все шаги загружены!
// Шаг 2 загружен (самый быстрый ответ не обязательно первый)
// Шаг 1 загружен
// Шаг 3 загружен

Что происходит на самом деле:

1. forEach запускает все async функции сразу
2. await внутри каждой итерации не блокирует следующую итерацию forEach
3. Шаги покажутся в случайном порядке (какой загрузится первым)
4. Надпись Все шаги загружены! появится до загрузки

Дело в том, что forEach (как и другие методы массивов) не предназначен для работы с асинхронными коллбеками:

1. forEach синхронно вызывает переданную функцию для каждого элемента
2. При встрече await внутри async-функции, управление возвращается в Event Loop
3. forEach продолжает итерации, не дожидаясь промиса
4. Все асинхронные операции стартуют параллельно

Таким образом, если в нашем тестовом массиве тысяча идентификаторов - код сделает практически тысячу параллельных запросов. Чтобы исправить положение, можно использовать обычный цикл for...of:

const stepIds = [1, 2, 3];

for (const id of stepIds) {
  const step = await loadStep(id);
  renderStep(step);
}

// Сработает после загрузки всех шагов
console.log('Теперь точно все загружены!');

7. map + filter вместо reduce

Очень уж часто я встречаю комбинацию из map().filter() или filter().map()

const users = [
  {id: 1, age: 25, premium: false},
  {id: 2, age: 17, premium: true},
  // ... 1000+ записей
];

const premiumUsersIds = users
  .filter(({ premium }) => premium)
  .map(({ id }) => id);

Такой вариант выполняет работу за O(2n). Если мы используем reduce, снизим потенциальное количество итераций вдвое, выполняя ту же работу за O(n):

const users = [
  {id: 1, age: 25, premium: false},
  {id: 2, age: 17, premium: true},
  // ... 1000+ записей
];

const premiumUsersIds = users.reduce((result, { id, premium }) => {
  if (premium) result.push(id);
  return result;
}, []);

Там, где мы обрабатываем много данных, reduce покажет себя лучше, однако, на небольших массивах всегда нужно искать компромисс между читаемостью и эффективностью

Кстати, для примера выше есть еще одна альтернатива с повышенной читаемостью, но производительностью хуже reduce - flatMap:

const users = [
  {id: 1, age: 25, premium: false},
  {id: 2, age: 17, premium: true},
  // ... 1000+ записей
];

const premiumUsersIds = users.flatMap(user => {
  return user.premium ? [user.id] : [];
});

Можно обратиться к вот такой сравнительной таблице:

|--------------|--------------------|------------|-----------------------|
| Метод        | Производительность | Читаемость | Промежуточные массивы |
|--------------|--------------------|------------|-----------------------|
| filter + map | ❌ 2 прохода       | ✅ Высокая | ❌ Создаёт            |
|--------------|--------------------|------------|-----------------------|
| reduce       | ✅ 1 проход        | ⚠️ Средняя | ✅ Нет                |
|--------------|--------------------|------------|-----------------------|
| flatMap      | ⚠️ 1 проход*       | ✅ Высокая | ⚠️ Частично           |
|--------------|--------------------|------------|-----------------------|

* - flatMap всё же создаёт временные массивы [user.id]/[]

6. Неправильная работа с this

Допустим, у вас есть объект пользователя со свойствами и методами и вы хотите вызвать один из методов с задержкой. Утрировано, конечно, но для примера пойдет:

const user = {
  name: 'Alice',
  greet() {
    console.log(`Привет, я ${this.name}`);
  },
};

setTimeout(user.greet, 1000);

И, казалось бы, все вроде логично, но в логе мы увидим Hello, my name is undefined 🤓. В такие моменты можно задуматься о том, чтобы выместить ненависть ко всему живому на своей клавиатуре, например, но не спешите - давайте разберемся

this в JavaScript определяется во время вызова функции, а не во время её объявления. Ведь this сам по себе это ссылка на текущий контекст выполнения, а в JavaScript он может быть разным

В примере мы передаем сам метод в качестве коллбэка setTimeout, поэтому контекстом выполнения становится... window или undefined (в строгом режиме), а не наш объект! Чтобы получить ожидаемое поведение, можно явно назначить контекст, либо использовать стрелочную функцию, у которой своего контекста нет:

// Так сработает
setTimeout(user.greet.bind(user), 1000);

// И так тоже
setTimeout(() => user.greet(), 1000);

Давайте еще один, более практичный пример. Например, у нас есть объект корзины и мы хотим по нажатию на кнопку вызвать метод корзины:

const button = document.getElementById('add-btn');

const cart = {
  selectedIds: [],
  selectItem(event) {
    const targetItemId = event.target.dataset.id;
    this.selectedIds.push(targetItemId);
  },
};

button.addEventListener('click', cart.selectItem);

И когда мы нажмем на кнопку, мы получим ошибку, потому что selectedIds не существует в контексте метода selectItem. Чиним подходящим способом:

button.addEventListener('click', (event) => cart.selectItem(event));

Так как this определяется во время вызова, при передаче метода как коллбэка нужный контекст попросту отвязывается. Это как если бы мы сделали так:

const selectItem = cart.selectItem;
selectItem(); // this === window / undefined

5. try ... catch без await

Очень распространенная ошибка среди тех, кто не до конца понимает асинхронную природу в JavaScript. Предположим, у нас есть простой метод сервиса, который делает запрос тех или иных данных:

getData(options) {
  return sendRequest(options);
}

sendRequest - асинхронная функция, возвращающая промис, но мы его сразу возвращаем и можно сделать await по месту вызова getData. Пока все хорошо. А теперь мы хотим завернуть это в try...catch, чтобы обрабатывать ошибки на месте, не позволяя им вываливаться в глобальный обработчик:

getData(options) {
  try {
    return sendRequest(options);
  } catch (error) {
    this.logger.error('Failed to send request', error);
    return null;
  }
}

Вроде все красиво: try...catch есть, ошибки обрабатываются и даже логирование на месте. Но ничего не заработает - все равно ошибка попадет в глобальный обработчик

Почему? Функция возвращает промис, создаваемый синхронно, но он резолвится асинхронно за пределами try...catch, когда тот уже отработал!

Чтобы не пропустить ошибку и try...catch ее поймал, мы должны ждать промис внутри try...catch. Просто добавим воды await:

async getData(options) {
  try {
    return await sendRequest(options);
  } catch (error) {
    this.logger.error('Failed to send request', error);
    return null;
  }
}

4. Мутации данных

Предположим, мы получаем данные с сервера, но приходят они немного не в том формате, в котором нам удобно с ними работать. Тогда мы сразу их преобразуем:

const apiResponse = [
  { id: 1, name: 'Alice', contacts: { email: 'alice@example.com' } },
  { id: 2, name: 'Bob', contacts: { email: 'bob@example.com' } },
];

const normalizeUsers = (users) => {
  return users.map(user => {
    // Переносим email на верхний уровень
    user.email = user.contacts.email;
    // Удаляем старую структуру
    delete user.contacts;
    
    // ❌ Возвращаем мутированный объект!
    return user;
  });
}

const normalizedUsers = normalizeUsers(apiResponse);

// undefined 😱 Исходные данные испорчены!
console.log(apiResponse[0].contacts);

// true (Один и тот же объект)
console.log(normalizedUsers[0] === apiResponse[0]);

А что, если у нас уже есть или в будущем появится несколько потребителей этих данных? Изменяя объекты по ссылке, мы получим гораздо больше проблем, чем пользы - побочные эффекты и потеря исходных данных. Отладка при таких условиях - невыносимая пытка

Чтобы избежать нежелательных изменений, лучше использовать иммутабельный подход:

const normalizeUsers = (users) => {
  return users.map(user => {
    return {
      id: user.id,
      name: user.name,
      email: user.contacts.email,
    }
  });
}

Здесь мы получаем нужный нам формат, не изменяя исходные данные. Можно еще вот так, если полей много и прописывать каждый совсем не хочется:

const normalizeUsers = (users) => {
  return users.map(({ contacts, ...rest }) => {
    return { ...rest, email: contacts.email };
  });
}

Всегда старайтесь использовать иммутабельный подход - сэкономит нервы

3. Нестрогое сравнение

Нестрогое сравнение приводит к совсем неожиданным последствиям. Допустим, мы хотим написать простую функцию, которая определяет, инициализировано ли значение:

const isDefined = (value) => value != undefined; // ! =

console.log(isDefined(0)); // ✅ true
console.log(isDefined('')); // ✅ true
console.log(isDefined(undefined)); // ✅ false
console.log(isDefined(null)); // ❌ false

Как это false? Ведь null - это пустое значение. Да, оно буквально значит "Ничего", но оно определено и функция для такого вызова должна вернуть true!

Дело в том, что нестрогое сравнение неявно приводит типы вместо того, чтобы учитывать их при сравнении. А при нестрогом сравнении null == undefined:

const isDefined = (value) => value !== undefined; // ! = =

console.log(isDefined(0)); // ✅ true
console.log(isDefined('')); // ✅ true
console.log(isDefined(undefined)); // ✅ false
console.log(isDefined(null)); // ✅ true

Вот еще пример, когда нестрогое сравнение может сломать нам логику:

// value здесь - строка
const value = prompt('Введите число:');

// = =
if (value == 42) {
  alert('Вы угадали!');
}

Если пользователь введёт '042', код всё равно сработает, потому что '042' == 42. Решаем явным приведением типов и строгим сравнением:

const value = prompt('Введите число:');

// = = =
if (Number(value) === 42) {
  alert('Вы угадали!');
}

true == 1, 'false' == false - ну и так далее

2. Использование setInterval

Если нам нужно выполнять какое-то действие раз в секунду, интуитивно можно прийти к setInterval - вроде бы готовая функция, решающая задачу. Но на самом деле setInterval не лучший способ, так как порождает множество проблем:

• Пересечение вызовов (так называемый Race Condition)
• Нельзя менять интервал между итерациями
• Утечки памяти
• Различная реализация в браузерах

Давайте по порядку разберемся, что именно не так с setInterval и чем его заменить

Пересечение вызовов

setInterval не учитывает длительность выполнения функции. Если код внутри обработчика выполняется дольше, чем установленный интервал, могут начаться наложения вызовов. Это может привести к совершенно неожиданным последствиям и увеличенной нагрузке

Например, лонгпулл (Longpoll):

setInterval(() => {
  console.log('Начало');

  // Долгий асинхронный процесс
  fetch('https://api.example.com/data')
    .then(response => response.json())
    .then(data => console.log('Данные получены', data));

  console.log('Конец');
}, 1000);

Если запрос займёт больше 1 секунды, новый вызов начнётся, пока старый ещё не завершился! Это создаст лавинообразную нагрузку на сервер и может положить API

Если мы перепишем этот код на async / await - ситуация не изменится. Здесь прямо как с асинхронными колбэками в методах массивов - асинхронные функции передаются на выполнение в EventLoop, а setInterval ставит следующие вызовы в очередь, не дожидаясь завершения предыдущих

Нельзя менять интервал между итерациями

Например, мы хотим сделать инкрементальное увеличение задержки для каждой итерации, чтобы делать, например, запрос сначала через 1 секунду, потом через 2, потом через 3 и так далее:

let delay = 1000;

setInterval(() => {
  console.log('Новая итерация');

  // Это не повлияет на интервал
  delay += 1000;
}, delay);

Ничего не выйдет - интервал останется прежним, так как setInterval фиксирует задержку при первом вызове и не позволяет её менять

Утечки памяти

Если мы используем замыкания на тяжелые объекты внутри setInterval или создаем такие объекты на итерации, они будут накапливаться в памяти. Они будут очищены из памяти только если завершить setInterval (функция clearInterval)

Думаете, это не критично? Представим, что у нас есть массив из 10 000 простеньких объектов, которые мы получаем из апи каждую секунду:

const obj = {
  id: 'user_1234567890',
  profile: {
    name: 'Иван Иванов',
    age: 30,
    isPremium: true,
  },
  orders: [1001, 1002, 1003, 1004, 1005],
  lastActivity: new Date(),
  metadata: {
    tags: ['важный', 'постоянный'],
    rating: 4.5,
  }
};

Массив из 10 000 таких объектов будет занимать в памяти в районе 2 МБ. Через 5 минут (То есть всего 300 итераций) зависимости setInterval займут уже 600 МБ

Различная реализация в браузерах

setInterval (как, впрочем, и setTimeout) не входит в стандарт языка JavaScript - это часть Web API (Ну или Node API). Разные браузеры могут по-разному реализовывать setInterval, из-за чего возможны проблемы с точностью интервалов и временем срабатывания

Решение - рекурсивный setTimeout

Безусловно, setTimeout имеет схожие болезни, но в виду своей природы он надежнее и предлагает больше контроля. В отличии от setInterval, рекурсивный setTimeout меньше подвержен утечкам памяти, так как позволяет сборщику мусора очищать зависимости каждую итерацию, а еще:

• Не будет пересечения вызовов
• Позволяет изменять интервал
• Можно остановить прямо из коллбэка

const startTick = (delay) => {
  setTimeout(async () => {
    console.log('Начало итерации');

    // Дождёмся завершения
    await fetch('https://api.example.com/data');
    console.log('Конец итерации');
    
    /**
     * Цикл остановится, если мы сделаем здесь
     * обычный return
     */

    // Изменяем интервал
    startTick(delay + 500);
  }, delay);
}

startTick(1000);

1. Смешение типов в переменных

Вот мы и добрались до первого места. Самый главный прародитель скверны, корень зла, бессменный директор всего этого хаоса - подход, в котором мы смешиваем типы в переменных, параметрах и где-бы то ни было еще

Динамическая типизация - прямо таки цифровой змей искуситель нашего виртуального Эдема. Ведь зачем создавать новую переменную, если можно переиспользовать старую? Зачем явно приводить типы, если язык как-то сам с этим справляется?

Но на деле, если мы сами не следим за типами данных, можно по неопытности прийти к неожиданным последствиям:

let balance = '100';

const addFunds = (amount) => {
  balance += amount;
}

addFunds(50);
console.log(balance); // 10050

А как насчет ситуации, когда объект становится массивом объектов? Ну казалось бы, здесь уже есть один такой - почему бы не сложить здесь же несколько похожих?

let settings = {
  theme: 'dark',
  language: 'ru',
};

console.log(settings.theme); // dark

settings = [
  { theme: "light" },
  { theme: "dark" },
];

console.log(settings.theme); // undefined
console.log(settings[0].theme); // light

Разработчики, отлаживающие математические операции, скажут вам большое человеческое спасибо за то, что вы сделали число объектом!

let score = 10;

console.log(score + 5); // 15

score = { points: 10, history: [5, 3, 2] };

// Ожидание: 15, Реальность: "[object Object]5"
console.log(score + 5);

Может показаться, что это какая-то нереалистичная претензия, но вот такие, например, ошибки можно встретить не только в коде учащегося:

// Функция возвращает либо число, либо массив чисел
const getDiscount = (user) => {
  if (user.vip) {
    return 20; // Скидка в процентах
  } else {
    return [5, 10]; // Возможные скидки
  }
}

let discount = getDiscount({ vip: true });
console.log(discount + 5); // 25

discount = getDiscount({ vip: false });
console.log(discount + 5); // "[5,10]5"

Вывод

Переменная, в которую записано число, всегда должна хранить число. Всегда создавайте новые переменные, если этого требует ваша логика. Смешение типов безоговорочно приведет к невероятному количеству проблем

Да, JavaScript - язык с динамической типизацией и на первый взгляд глупо не пользоваться его особенностями, но именно эта свобода часто становится ловушкой, из которой выход придется искать часами, днями, неделями

Здесь же поможет подход, о котором я рассказывал в этой статье - не перезаписывайте переменные, всегда старайтесь использовать const вместо let. А еще перед началом проекта подумайте над тем, чтобы сразу интегрировать TypeScript. Только осторожнее, пути назад не будет - вы перестанете воспринимать проекты без TypeScript как что-то серьезное 😊

Бонус

Я не придумал, как органично расположить эту информацию в топе. Тем более, что это не совсем ошибки - скорее, неоптимальные подходы к написанию кода. Но я не мог не включить эти темы в статью 🤓

Антипаттерн if true

Такие ошибки порождает наш образ мышления. Например, когда мы хотим обновить данные администратора, в общении мы описываем процесс так:

• Получаем пользователя
• Если пользователь найден
• И если у того, кто сделал запрос, достаточно прав
• Обновляем данные
• Иначе выбрасываем ошибку
• Иначе выбрасываем ошибку
• Возвращаем результат

Если мы переложим это на код, то можно совершенно автоматически прийти к такому варианту:

const updateUser = async (req, res) => {
  let result;

  const targetId = Number(req.query.id);
  const payload = req.body;

  if (targetId) {
    const user = await getUser(targetId);

    if (user) {
      if (req.hasPermission(Permission.UPDATE_ADMINS)) {
        result = await updateUserInDatabase(targetId, payload);
      } else {
        throw new ForbiddenError('You do not have permission');
      }
    } else {
      throw new NotFoundError('User not found');
    }
  } else {
    throw new BadRequestError('User id required');
  }
}

Получается весьма неказистая структура, которую не то, что поддерживать - даже прочитать сложно. Такой себе if-hell. Но что, если мы изменим подход и перепишем код по-другому:

const updateUser = async (req, res) => {
  const targetId = Number(req.query.id);
  const payload = req.body;

  if (!targetId) {
    throw new BadRequestError('User id required');
  }

  const user = await getUser(targetId);

  if (!user) {
    throw new NotFoundError('User not found');
  }

  if (!req.hasPermission(Permission.UPDATE_ADMINS)) {
    throw new ForbiddenError('You do not have permission');
  }

  return updateUserInDatabase(targetId, payload);
}

✨ Вуаля! Мало того, что мы избавляемся от лишней вложенности, так еще и очищаем код от множества ненужных else. Получаем упрощение поддержки кода, лучшее восприятие и простоту отладки и тестирования. Ключ к успеху здесь - инвертирование условия и ранний выход. Если переложить этот код на текстовое описание, то получится:

• Получаем пользователя
• Если пользователь НЕ найден
• Выбрасываем ошибку
• Если у того, кто сделал запрос, НЕТ прав
• Выбрасываем ошибку
• Обновляем данные и возвращаем результат

Булевый избыток

Назовем это так. Я часто встречаю, особенно в коде новичков, явные сравнения с булевыми значениями:

if (isActive === true) { ... }
if (hasPermission === false) { ... }

Это лишнее, так делать не нужно. Упрощаем:

if (isActive) { ... }
if (!hasPermission) { ... }

Помимо условий, встречается еще и такое:

const checkAccess = (user) => {
  if (user.role === 'admin') {
    return true;
  } else {
    return false;
  }
}

Это тоже можно легко упростить до одной строчки:

const checkAccess = (user) => user.role === 'admin';

Избыточные условия

Многие забывают про то, что в JavaScript существует специальный оператор, позволяющий сократить длину условий и сильно улучшить восприятие. Предположим, мы хотим сделать что-то с настройками пользователя, но мы не знаем точно, доступны ли они:

const user = await getUser();

if (user && user.profile && user.profile.settings) { ... }

Легким движением курсора и пальцев на клавиатуре можно упростить это условие до:

const user = await getUser();

if (user?.profile?.settings) { ... }

Подобное можно встретить и когда мы проверяем наличие элементов в массиве, который может быть null:

const users = await getUsers();

if (users && users.length > 0) { ... }

Упрощаем до

const users = await getUsers();

if (users?.length) { ... }

А что если мы хотим установить значение по умолчанию, если глубоко вложенного поля нет? Возьмем для примера те же настройки профиля:

let settings;

if (user && user.profile && user.profile.settings) {
  settings = user.profile.settings;
} else {
  settings = DEFAULT_SETTINGS;
}

Оператор опциональной цепочки вкупе с оператором Coalesce сильно упростит нам жизнь и здесь:

const settings = user?.profile?.settings ?? DEFAULT_SETTINGS;

Итоги

Вот мы и подошли к завершению. Думаю, мне удалось дать рекоммендации по качеству кода, рассказать об основных "болях" тимлидов и проблемах, на которых застревают новички - от коварной динамической типизации до асинхронных граблей

Делитесь в комментариях своим мнением насчет топа и на скольких из этих ошибок вы спотыкались. А, может, у вас есть примеры из реального кода, которые стоит разобрать? Буду рад обсудить!