Na co naprawdę patrzę w frontendowym code review

W ostatnim tekście pisałem, że nowym bottleneckiem jest czytanie kodu.

To teraz konkretnie: na co patrzę, kiedy czytam.

Robię frontendowe code review w projekcie, który operuje na milionach rekordów. Wydajność nie jest tu tematem teoretycznym. Zły algorytm nie tylko wygląda mniej elegancko — on realnie spowalnia aplikację w sposób, który użytkownicy widzą i czują.

Z czasem wyrobiłem sobie w głowie checklistę. Nie formalny dokument. Nie zestaw zasad, które przykładam do każdego PR-a bez zastanowienia. Raczej sygnały, przy których zwalniam i zadaję jeszcze jedno pytanie.

To jest właśnie ta checklista.

Niepotrzebne iteracje

Najczęstsza rzecz, którą wyłapuję, to przechodzenie po tych samych danych dwa razy, mimo że dałoby się zrobić to w jednym przebiegu.

// ❌ dwie iteracje
const activeValues = data.filter(item => item.active).map(item => item.value);

// ✅ jedna iteracja
const activeValues = data.reduce<number[]>((acc, item) => {
  if (item.active) acc.push(item.value);
  return acc;
}, []);

Przy tysiącu rekordów prawdopodobnie nie zobaczymy żadnej różnicy.

Przy milionie rekordów łańcuch .filter().map() nie jest już czymś, co automatycznie pomijam wzrokiem. Nadal może być w porządku. Ale chcę, żeby autor kodu wiedział, że robimy dodatkowe przejście po danych i tworzymy tablicę pośrednią.

Sama poprawka zwykle nie jest trudna. Trudniejsze jest wyrobienie sobie nawyku, żeby w ogóle zadać to pytanie.

Operacje, które ukrywają intencję

Czasem kod na pierwszy rzut oka wygląda czysto, ale pod spodem chowa niepotrzebną pracę albo niejasną decyzję.

// ❌ działa, ale ukrywa intencję
const id = Math.random().toString().slice(2);

// ✅ jeśli potrzebujesz losowego ID, powiedz to wprost
const id = crypto.randomUUID();

Większy problem nie polega tu tylko na liczbie operacji. Chodzi o intencję.

Math.random().toString().slice(2) wygląda jak obejście. Bierzemy losową liczbę, zamieniamy ją na string, odcinamy kawałek i traktujemy wynik jak identyfikator.

crypto.randomUUID() mówi od razu, jakiej wartości potrzebujemy.

Ten wzorzec widzę często w kodzie generowanym przez AI: coś działa, ale nie komunikuje decyzji. Kod zostaje zaakceptowany, bo wynik wygląda sensownie, a nie dlatego, że ktoś naprawdę zakwestionował implementację.

Agregacja powinna być jawna

Kiedy widzę w review coś takiego, zwykle sugeruję prostszy zapis:

// ❌ imperatywny akumulator
let totalSize = 0;
cols.forEach(col => {
  totalSize += col.size ?? 0;
});

// ✅ jawna agregacja
const totalSize = (cols: CustomColumnDef<Row>[]): number =>
  cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

Nie chodzi o udawanie, że każde let jest złe. Chodzi o to, żeby kształt operacji był od razu widoczny.

Wersja z reduce mówi: weź te wartości, zsumuj je i zwróć wynik.

Wersja imperatywna wymaga od czytelnika przejścia przez kilka kroków, zanim nazwie sobie w głowie, co ten kod właściwie robi.

Czytelność nie jest luksusem. To ona sprawia, że kolejne code review idzie szybciej.

Logika, która powinna wyjść poza komponent

Czasem widzę komponenty, które robią za dużo. Logika biznesowa, wartości wyliczane ze stanu, efekty uboczne — wszystko wymieszane w jednej funkcji renderującej.

Kiedy widzę wartości liczone na podstawie propsów albo state’u, nie proszę automatycznie o custom hooka. Ale pytam, czy ta logika stała się już na tyle ważna, że warto ją nazwać, przetestować albo wykorzystać ponownie.

// ❌ wartość wyliczana inline przy każdym renderze
function DataTable({ rows }: Props) {
  const visibleCount = rows.filter(r => !r.hidden).length;
  // ...
}

// ✅ wyciągnięte, nazwane, testowalne i możliwe do ponownego użycia
function useVisibleCount(rows: Row[]) {
  return useMemo(() => rows.filter(r => !r.hidden).length, [rows]);
}

To nie zawsze jest temat wydajności. I zdecydowanie nie jest to zasada, że każda wartość pochodna musi od razu trafić do custom hooka.

Chodzi o separację odpowiedzialności.

Jeśli jakaś kalkulacja jest na tyle istotna, że rozmawiamy o niej w review, to być może jest też na tyle istotna, żeby dostała nazwę. Logikę w hooku łatwiej przetestować w izolacji. Logikę zakopaną w komponencie trudniej zrozumieć i łatwiej zdublować.

useEffect, którego nie powinno być

To jest wzorzec, który najczęściej widzę w kodzie wygenerowanym przez AI.

Model zna useEffect. Widział go w ogromnej liczbie przykładów. Więc kiedy developer prosi o pomoc przy stanie pochodnym albo transformacji danych, model bardzo często sięga po useEffect z automatu — nawet wtedy, kiedy nie ma żadnego efektu ubocznego.

// ❌ useEffect do czegoś, co jest tylko wartością pochodną
const [fullName, setFullName] = useState('');

useEffect(() => {
  setFullName(`${firstName} ${lastName}`);
}, [firstName, lastName]);

// ✅ to jest po prostu zmienna
const fullName = `${firstName} ${lastName}`;

Trzy dodatkowe linie, dodatkowy state, dodatkowy render — wszystko po to, żeby zrobić coś, co jest dosłownie jedną linią JavaScriptu.

Jeśli nie synchronizujemy niczego ze światem zewnętrznym, najpierw pytam: czy ten efekt w ogóle jest potrzebny?

Często problemem nie jest developer. Problemem jest prompt.

Jeśli poprosisz AI, żeby coś „obsłużyło”, ale nie podasz ograniczeń, model wybierze najczęstszy wzorzec, jaki zna. A useEffect jest wszędzie.

Czasem wystarczy zapytać inaczej: „rozwiąż to bez useEffect” albo „jaki jest najprostszy sposób, żeby wyliczyć tę wartość?” — i wynik od razu robi się czystszy.

Komentarze, które opisują „co”, zamiast tłumaczyć „dlaczego”

Ostatnia rzecz jest trudniejsza do zmierzenia, ale łatwa do zauważenia.

// ❌ opisuje to, co kod już pokazuje
// iterate over columns and sum their sizes
const totalSize = cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

// ✅ tłumaczy, dlaczego taka decyzja została podjęta
// col.size can be undefined for dynamic columns — default to 0 to avoid NaN
const totalSize = cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

Komentarze generowane przez AI prawie zawsze opisują co robi kod. Bo to najłatwiejsza rzecz, jaką można o kodzie powiedzieć.

Komentarz w stylu „przejdź po kolumnach i zsumuj ich rozmiary” nie wnosi nic. Kod już to mówi.

Dobry komentarz tłumaczy dlaczego — ograniczenie, nieoczywistą decyzję, edge case albo coś, co będzie miało znaczenie za pół roku.

Kiedy widzę blok kodu z komentarzami, które powtarzają logikę linia po linii, proszę o ich usunięcie albo zmianę. Nie dlatego, że komentarze są złe. Dlatego, że taki komentarz jest szumem. A szum utrudnia review.

Co łączy te wszystkie przykłady

Te sześć rzeczy wygląda jak osobne problemy. Ale pod spodem często chodzi o to samo: kod został wyprodukowany, zanim został naprawdę zrozumiany.

Algorytm działa, ale robi więcej pracy niż trzeba. Helper zwraca wartość, ale ukrywa intencję. Hook jest poprawny, ale niepotrzebny. Komentarz istnieje, ale niczego nie wyjaśnia.

AI potrafi wygenerować każdy z tych przypadków. A developer, który pracuje w trybie: Enter, wynik, kopiuj, dalej — bardzo łatwo je przepuści.

Dlatego w code review ciągle wracam do jednej rzeczy: zwolnij dokładnie w tym momencie, w którym narzędzie chce cię przyspieszyć.

Przeczytaj to, co zostało wygenerowane. Zapytaj, czy kod robi więcej, niż musi. Zapytaj, czy decyzja jest widoczna w implementacji. Zapytaj, czy kolejna osoba zrozumie, dlaczego ten kod wygląda właśnie tak.

Przy dużych wolumenach danych koszt braku tych pytań jest mierzalny.

Przy każdej skali koszt nieczytania kodu jest realny.