Code review на JavaScript: что проверять, чтобы код не рассыпался

Code review на JavaScript: что проверять, чтобы код не рассыпался

Вы когда-нибудь открывали чужой pull request и чувствовали, как внутри всё сжимается? Словно смотрите на запутанный клубок проводов, где каждый узел — это потенциальный баг. Code review на JavaScript — это не просто галочка в таск-трекере. Это ваш последний рубеж защиты от хаоса. Особенно в динамичном мире JS, где типизация — это иллюзия, а «волшебное» поведение — норма.

В этой статье я расскажу, на что реально обращать внимание при ревью JavaScript-кода. Забудьте про бессмысленные споры о табах vs пробелах. Мы поговорим о вещах, которые напрямую влияют на стабильность, читаемость и производительность. Это тот самый чеклист code review, который я использую сам.

1. Чистота функций и отсутствие side-эффектов

Первый и самый важный пункт. Функция должна делать ровно одну вещь и делать её хорошо. Если вы видите функцию, которая одновременно валидирует данные, отправляет запрос на сервер и обновляет глобальный стейт — это тревожный звоночек.

Что проверять:

• Количество аргументов: Идеально — 0, максимум — 3. Если больше — выносите в объект-конфиг.
• Мутация входных данных: Функция не должна менять переданный массив или объект. Это хардкорный баг, который сложно отловить.
• Побочные эффекты: Любое изменение внешнего мира (console.log, запись в файл, изменение DOM) должно быть явным и оправданным.

Плохой пример:

let totalUsers = 0;

function processUsers(users) {
    for (let i = 0; i < users.length; i++) {
        users[i].active = true; // Мутация входных данных!
        totalUsers++;           // Побочный эффект!
        console.log(users[i]);  // Ещё один побочный эффект
    }
    return users;
}

Хороший пример:

function activateUsers(users) {
    return users.map(user => ({ ...user, active: true }));
}

// Использование:
const activeUsers = activateUsers(users);
console.log(`Активировано: ${activeUsers.length}`);

Помните: чистые функции — это основа предсказуемого кода. Если на code review вы видите мутацию — смело пишите комментарий. Это одна из главных причин трудноуловимых багов в JavaScript.

2. Асинхронность: промисы и async/await

JS — король асинхронности, но также и король коллбэк-ада. При ревью обращайте внимание на то, как обрабатываются асинхронные операции. Проверка кода JavaScript без анализа цепочек промисов — это профанация.

Красные флаги:

• Отсутствие обработки ошибок: Нет .catch() или try/catch. Рано или поздно это уронит приложение.
• Вложенные промисы: Если вы видите .then() внутри .then() — это повод для рефакторинга.
• Забытый await: Функция помечена как async, но внутри вызовы без await. Результат — undefined вместо данных.

Плохой пример (отсутствие обработки ошибок):

async function fetchUserData(userId) {
    const response = await fetch(`/api/users/${userId}`);
    const data = await response.json();
    return data;
}

Хороший пример:

async function fetchUserData(userId) {
    try {
        const response = await fetch(`/api/users/${userId}`);
        if (!response.ok) {
            throw new Error(`HTTP error! status: ${response.status}`);
        }
        return await response.json();
    } catch (error) {
        console.error('Ошибка загрузки пользователя:', error);
        throw error; // Пробрасываем дальше, но логируем
    }
}

Совет для ревьюера: всегда требуйте объяснения, почему try/catch не используется. «Потому что это никогда не падает» — не аргумент. Качество кода определяется тем, как он ведёт себя в нештатных ситуациях.

3. Работа с состояниями: null, undefined и falsy-значения

JavaScript известен своими «сюрпризами» с типами. null vs undefined, 0 как falsy-значение, сравнение через == вместо ===. Это классические точки отказа.

Что ищем:

• Неявное приведение типов: Используйте === всегда. Исключение — проверка на null и undefined одновременно (value == null).
• Проверка на наличие: if (arr.length) — плохо. if (arr.length > 0) — хорошо. Почему? Потому что 0 — это falsy, и код не провалится в условие, если длина ноль.
• Опциональная цепочка: Используется ли ?. для доступа к глубоко вложенным свойствам?

Плохой пример:

function getUserName(user) {
    if (user.name) { // Может быть пустая строка ""
        return user.name;
    }
    return 'Anonymous';
}

Хороший пример:

function getUserName(user) {
    return user?.name?.trim() || 'Anonymous';
}

Также обратите внимание на ранний выход (early return). Если функция имеет много вложенных if, это усложняет её чтение и повышает риск ошибки.

4. Производительность: что бросается в глаза

Code review — не место для микро-оптимизаций (спасибо, не надо менять for на forEach). Но есть вещи, которые реально тормозят приложение. SEO-оптимизация кода — это не про ключевые слова, а про скорость загрузки.

Основные проблемы:

• Избыточные перерендеры в React/Vue: Нет useMemo или React.memo для тяжёлых вычислений.
• Циклы внутри циклов (O(n²)): Если можно решить через Map или Set — делайте так.
• Запросы в цикле: for с fetch внутри — это ад для бэкенда. Используйте Promise.all.

Плохой пример (запросы в цикле):

async function loadUsers(ids) {
    const users = [];
    for (const id of ids) {
        const user = await fetch(`/api/users/${id}`);
        users.push(await user.json());
    }