Ревью миграции JS-проекта на TypeScript: что смотреть

Команда включила TypeScript, переименовала файлы в .ts, сборка проходит зелёной — а в проде всё те же undefined is not a function. Типичная история недоделанной миграции: тип-чекер настроен так, что молчит, а автор PR честно думает, что типизация закрыла проблему. На ревью такой миграции ваша задача не «проверить качество кода», а найти места, где типы врут или отсутствуют, и где рефакторинг под видом типизации незаметно поменял поведение.

Статья про то, что конкретно смотреть в PR с миграцией на TypeScript 5.x: не про то, как включить компилятор, а про то, где ревьюер должен остановиться и потребовать переделать. Углубляться в устройство самого компилятора не будем — только то, что видно в диффе.

С чего начинается ревью: сначала конфиг, потом код

Прежде чем смотреть .ts-файлы, откройте tsconfig.json. Половина проблем миграции спрятана именно там, а не в коде. Если strict выключен, любой файл, который «типизирован», на деле проверен наполовину.

Ключевые флаги, которые определяют, ловит ли компилятор ошибки вообще:

Миграция без strictNullChecks — это не миграция на TypeScript, а переименование файлов. Компилятор не видит самый частый класс ошибок в JS-коде.

Если команда мигрирует большой проект и не может включить strict сразу, это нормально — но тогда в PR должен быть виден план: какие флаги включены сейчас, какие следующим шагом. Отдельный флаг useUnknownInCatchVariables (по умолчанию входит в strict с TS 4.4) стоит проверить: без него переменная в catch имеет тип any и снова обходит проверки.

Главный враг ревью — тихий any

В мигрированном коде any появляется тремя путями, и каждый требует разной реакции.

Явный any как затычка

Автор поставил : any, чтобы компилятор замолчал. Это видно в диффе и легко комментируется. Опаснее не сам факт, а места, где any расползается: значение с типом any, переданное в функцию, отключает проверку и внутри неё.

// Было в JS, стало в TS — тип не восстановлен
function parseConfig(raw: any) {
  return raw.settings.timeout; // никакой проверки: settings может быть undefined
}

Замечание на ревью формулируйте конкретно: не «тут any», а «raw приходит из JSON.parse, тип неизвестен — используйте unknown и провалидируйте settings перед доступом». unknown заставляет автора сузить тип явно, any — нет.

Неявный any из вывода

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

any через приведение as

Конструкция value as SomeType говорит компилятору «поверь мне» и отключает проверку в этой точке. В миграции такие приведения плодятся, чтобы обойти несовпадения. Особенно опасна двойная форма value as unknown as SomeType — она пробивает любую несовместимость типов.

// Опасно: приведение скрывает, что форма объекта другая
const user = response.data as unknown as User;

// Лучше: рантайм-проверка формы данных
function isUser(x: unknown): x is User {
  return typeof x === 'object' && x !== null && 'id' in x && 'email' in x;
}
if (!isUser(response.data)) throw new Error('Неверный формат пользователя');
const user = response.data; // здесь тип уже User

Каждое as на границе с внешними данными (ответ API, парсинг JSON, чтение из localStorage) — повод потребовать вместо приведения проверку в рантайме через type guard. Внутри кода приведение иногда оправдано, на границе — почти никогда.

Где рефакторинг под видом типизации меняет поведение

Самая коварная часть миграции: автор «просто добавляет типы», но по пути правит логику, часто не замечая. Эти места ищите прицельно.

• Опциональная цепочка вместо ручной проверки. a && a.b && a.b.c заменили на a?.b?.c. Обычно эквивалентно, но && останавливается и на пустой строке или нуле, а ?. — только на null/undefined. Поведение расходится, если в цепочке бывают 0 или ''.
• Нулевое слияние вместо ||. x || defaultValue заменили на x ?? defaultValue. Первое подставит дефолт для 0, '', false; второе — только для null/undefined. Для флага или счётчика это меняет логику.
• Инициализация полей класса. При включённом strictPropertyInitialization автор мог добавить ! (definite assignment) к полю, чтобы компилятор замолчал, вместо того чтобы честно инициализировать его. declare-поле, которое реально пустое в рантайме, — источник ошибки.
• Приведение перечислений. Строковые константы превратили в enum. Проверьте, что нигде это значение не сравнивается со строкой напрямую и не сериализуется — enum и строка при === дают false.

Практическое правило для автора и ревьюера: миграция файла и изменение его логики — два разных PR. Если в одном диффе смешаны переименование, добавление типов и «заодно поправил условие», просите разделить. В смешанном PR регрессию не видно.

Что смотреть в типах, а не в логике

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

• Объектные типы вместо object и {}. Тип {} в TS означает «что угодно, кроме null и undefined», а не «пустой объект». Почти всегда это ошибка автора.
• Функциональные типы без возврата. Callback затипизирован как Function — это тот же any для вызова. Нужен конкретный (arg: T) => R.
• Дублирование форм вместо переиспользования. Одну и ту же структуру описали трижды в разных файлах. Просите вынести в общий interface или type — иначе при изменении API правки разъедутся.
• Игнор-комментарии. // @ts-ignore отключает проверку строки вслепую, // @ts-expect-error — тоже, но упадёт, если ошибка исчезнет. Требуйте второй вариант там, где подавление всё же нужно: он не протухнет молча.

Как проверить, что миграция реально что-то ловит

Зелёная сборка ничего не доказывает, если проверок нет. Быстрый способ убедиться, что тип-чекер работает:

1. Запустите tsc --noEmit с включённым strict на ветке PR. Число ошибок больше нуля — миграция ещё идёт, это честно; ноль при выключенном strict — подозрительно.
2. Посчитайте any в диффе. Грубо: grep -rn ": any" src/ | wc -l до и после. Рост на десятки — миграция сделана затычками.
3. Проверьте, что линтер настроен на запрет any и @ts-ignore хотя бы предупреждением (правила @typescript-eslint/no-explicit-any, @typescript-eslint/ban-ts-comment). Без этого следующий PR снова принесёт any.

Что делать дальше

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

В проде смотрите на количество ошибок вида «cannot read property of undefined» в логах до и после включения strictNullChecks — это прямой показатель, что миграция реально закрыла баги, а не переставила их. Инвалидация типов внешних библиотек через @types/* — отдельная тема, здесь её не разбираем.