fix(ui): использовать <Link> вместо <a> для internal routes (build error) #439

Merged
lekss361 merged 1 commit from fix/landing-nav-link-component into main 2026-05-23 11:31:59 +00:00
Owner

Summary

Test plan

  • npm run lint проходит без no-html-link-for-pages ошибок
  • GHA build проходит
  • Nav ссылки работают
## Summary - Build error в commit 65c77b9: ESLint rule @next/next/no-html-link-for-pages запрещает <a href=...> для internal Next.js pages - Заменены все 6 <a> в nav (/, /analytics, /site-finder, /trade-in/, /concept, /admin/scrape/all) на <Link> из next/link - Добавлен import Link from next/link - Footer <a href=/api/v1/docs> и <a href=/health> — backend endpoints — оставлены без изменений ## Test plan - [ ] npm run lint проходит без no-html-link-for-pages ошибок - [ ] GHA build проходит - [ ] Nav ссылки работают
lekss361 added 1 commit 2026-05-23 11:26:36 +00:00
lekss361 merged commit 43a8ec426e into main 2026-05-23 11:31:59 +00:00
Author
Owner

Merged via deep-code-reviewer — verdict APPROVE.

Deep Code Review — verdict

Summary

  • Status: APPROVE (urgent build fix)
  • Files reviewed: 1 (P2: frontend/src/app/page.tsx)
  • Lines: +14 / -12
  • Commit: e5c5b2de (single, clean, no force/amend/no-verify)

Cross-app navigation verification (critical concern)

Architecture confirmed via vault audits/TradeIn_MVP_CodeReview_May23.md:

  • tradein-mvp/ — отдельный Next.js stack (tradein-frontend:3000) с basePath /trade-in
  • Caddy @tradein path /trade-in /trade-in/* → routes BEFORE default frontend, ловит до handle { reverse_proxy frontend:3000 }
  • В main app есть frontend/src/app/trade-in/page.tsx — это DEAD CODE (vault audit M-1 "Delete from main, tradein-mvp is canonical"), но Caddy туда никогда не дойдёт

<Link href="/trade-in/"> поведение в проде:

  1. Next router prefetch'ит RSC payload → Caddy → tradein-frontend → возвращает HTML (не RSC)
  2. Next silently fails prefetch (acceptable)
  3. На click → Next router пытается client-side nav → RSC response не valid → fallback на hard navigation (полный page reload)
  4. Browser загружает /trade-in/ → tradein-frontend рендерит trade-in page корректно
  5. Работает (тот же effective behavior что был с <a>, просто с одним лишним prefetch round-trip)

<Link href="/admin/scrape/all">: page существует в main app (frontend/src/app/admin/scrape/all/page.tsx) → нормальная client-side nav

Footer <a href="/api/v1/docs"> и <a href="/health">: backend endpoints, не Next pages → ESLint rule не triggers, оставлены <a> корректно

Findings

Green:

  • Single clean commit, surgical fix
  • ESLint rule @next/next/no-html-link-for-pages правильно понят: только internal Next pages, не external/backend
  • import Link from "next/link" — нет namespace collision (других Link imports в файле нет)
  • Trade-in URL сохранён с trailing slash /trade-in/

Yellow (follow-ups, не блокирующие):

  1. <Link href="/trade-in/"> рекомендуется доработать до <Link href="/trade-in/" prefetch={false}> — отключит ненужный RSC prefetch на hover для cross-app route (tradein-frontend всё равно вернёт HTML, prefetch впустую). Косметика, не deploy-blocker.
  2. A11y findings из PR #438 still open: nav links без aria-current для активной страницы, без visible focus underline. Можно отдельным PR.

Self-acknowledged miss (3rd in a row)

В review PR #438 я флагнул "Logo <a> вместо <Link> (full reload, acceptable trade-off)" — но не предсказал что ESLint rule @next/next/no-html-link-for-pages BLOCK'нет GHA build. Это 3-й missed finding подряд (после caddy/** paths в #433 и Caddy REDACT assumption в #434).

Pattern для future UI-PRs: при review <a href="/internal-route"> в Next.js — VERIFY:

  • ESLint rule @next/next/no-html-link-for-pages не fail'нет CI
  • Если path matches Next pages (включая app/**/page.tsx) → требовать <Link>
  • Cross-app route (basePath subapp) → <a> с eslint-disable-next-line comment + reason

Post-merge action items

  1. Дождаться GHA build green после merge 43a8ec42
  2. Smoke: https://gendsgn.ru/ → click Trade-In / Concept / Admin / Site Finder / Аналитика — все routes должны open без TypeError / 404 flash
  3. Создать follow-up issue: <Link href="/trade-in/" prefetch={false}> (UX nit)
  4. Создать vault entry в fixes/ про ESLint no-html-link-for-pages rule pattern для future
Merged via deep-code-reviewer — verdict APPROVE. ## Deep Code Review — verdict ### Summary - **Status**: APPROVE (urgent build fix) - **Files reviewed**: 1 (P2: frontend/src/app/page.tsx) - **Lines**: +14 / -12 - **Commit**: e5c5b2de (single, clean, no force/amend/no-verify) ### Cross-app navigation verification (critical concern) **Architecture confirmed via vault `audits/TradeIn_MVP_CodeReview_May23.md`:** - `tradein-mvp/` — отдельный Next.js stack (`tradein-frontend:3000`) с basePath `/trade-in` - Caddy `@tradein path /trade-in /trade-in/*` → routes BEFORE default frontend, ловит до `handle { reverse_proxy frontend:3000 }` - В main app есть `frontend/src/app/trade-in/page.tsx` — это **DEAD CODE** (vault audit M-1 "Delete from main, tradein-mvp is canonical"), но Caddy туда никогда не дойдёт **`<Link href="/trade-in/">` поведение в проде:** 1. Next router prefetch'ит RSC payload → Caddy → tradein-frontend → возвращает HTML (не RSC) 2. Next silently fails prefetch (acceptable) 3. На click → Next router пытается client-side nav → RSC response не valid → **fallback на hard navigation** (полный page reload) 4. Browser загружает `/trade-in/` → tradein-frontend рендерит trade-in page корректно 5. Работает ✅ (тот же effective behavior что был с `<a>`, просто с одним лишним prefetch round-trip) **`<Link href="/admin/scrape/all">`**: page существует в main app (`frontend/src/app/admin/scrape/all/page.tsx`) → нормальная client-side nav ✅ **Footer `<a href="/api/v1/docs">` и `<a href="/health">`**: backend endpoints, не Next pages → ESLint rule не triggers, оставлены `<a>` корректно ✅ ### Findings **Green:** - Single clean commit, surgical fix - ESLint rule `@next/next/no-html-link-for-pages` правильно понят: только internal Next pages, не external/backend - `import Link from "next/link"` — нет namespace collision (других `Link` imports в файле нет) - Trade-in URL сохранён с trailing slash `/trade-in/` **Yellow (follow-ups, не блокирующие):** 1. `<Link href="/trade-in/">` рекомендуется доработать до `<Link href="/trade-in/" prefetch={false}>` — отключит ненужный RSC prefetch на hover для cross-app route (tradein-frontend всё равно вернёт HTML, prefetch впустую). Косметика, не deploy-blocker. 2. A11y findings из PR #438 still open: nav links без `aria-current` для активной страницы, без visible focus underline. Можно отдельным PR. ### Self-acknowledged miss (3rd in a row) В review PR #438 я флагнул "Logo `<a>` вместо `<Link>` (full reload, acceptable trade-off)" — но **не предсказал что ESLint rule `@next/next/no-html-link-for-pages` BLOCK'нет GHA build**. Это 3-й missed finding подряд (после `caddy/**` paths в #433 и Caddy REDACT assumption в #434). **Pattern для future UI-PRs**: при review `<a href="/internal-route">` в Next.js — VERIFY: - ESLint rule `@next/next/no-html-link-for-pages` не fail'нет CI - Если path matches Next pages (включая `app/**/page.tsx`) → требовать `<Link>` - Cross-app route (basePath subapp) → `<a>` с `eslint-disable-next-line` comment + reason ### Post-merge action items 1. Дождаться GHA build green после merge `43a8ec42` 2. Smoke: `https://gendsgn.ru/` → click Trade-In / Concept / Admin / Site Finder / Аналитика — все routes должны open без TypeError / 404 flash 3. Создать follow-up issue: `<Link href="/trade-in/" prefetch={false}>` (UX nit) 4. Создать vault entry в `fixes/` про ESLint `no-html-link-for-pages` rule pattern для future
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#439
No description provided.