Compare commits

...

4 commits

Author SHA1 Message Date
bot-backend
62e2a7ba85 style(tradein/v2): prettier (репозиторный pre-commit не покрывает tradein-mvp/frontend)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 10s
CI / changes (pull_request) Successful in 10s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m13s
2026-07-04 17:44:48 +03:00
bot-backend
10f4f6adac fix(tradein/v2): 3-state pendingRestoreId sentinel — null и undefined разные вещи
Живая проверка в браузере (клик по строке "Предыдущие оценки" на проде)
подтвердила: router.replace() — асинхронный client-nav (RSC round-trip),
window.location.search не обновляется синхронно, а readUrlId() читает
window.location.search напрямую (useSearchParams намеренно не используется —
ломает next build через Suspense). Единственный надёжный источник urlId
в синхронном ре-рендере — override-state, выставленный в том же тике,
что и клик.

Предыдущая версия (`pendingRestoreId ?? readUrlId()`) использовала null
для двух разных смыслов сразу: "нет override" и "принудительно пусто".
handleNew() выставлял null именно во втором смысле (сброс после 404),
но ?? тут же трактовал null как "нет override" и падал обратно на
readUrlId() — который на тот момент ещё возвращал старый плохой id
(router.replace("/v2") ещё не успел домотать). Итог: "НОВАЯ ОЦЕНКА"
после битой ссылки могла не сбрасывать ошибку.

Три состояния убирают двусмысленность: undefined = нет override (readUrlId
рулит), null = принудительно пусто, string = override на конкретный id.
2026-07-04 17:43:57 +03:00
bot-backend
d3d8d15f67 fix(tradein/v2): handleNew() тоже должен сбрасывать pendingRestoreId
Иначе клик "НОВАЯ ОЦЕНКА" после ошибки восстановления (напр. 404 на битый history-id) навсегда
залипает на старом pendingRestoreId — единственный effect, который его чистит, требует
restored.data, которого в error-состоянии не будет никогда. Только F5 разлипал бы.
2026-07-04 17:25:18 +03:00
bot-backend
535c2d3855 fix(tradein/v2): гонка при клике по строке "Предыдущие оценки" — оценка не загружалась
router.replace(`/v2?id=`) — асинхронный soft-nav, а setNav(0) в том же тике вызывает
синхронный re-render ДО того как window.location реально обновится → readUrlId() читает
устаревшее значение навсегда (никто больше не форсит re-render). Добавлен pendingRestoreId —
синхронный override, аналогично тому как freshResult уже обходит эту же гонку для
post-submit пути.
2026-07-04 17:05:51 +03:00

View file

@ -401,8 +401,30 @@ export default function TradeInV2Page() {
const [freshResult, setFreshResult] = useState<AggregatedEstimate | null>(
null,
);
// #2424 fix — router.replace(`?id=`) is an async client-navigation (RSC
// round-trip) that does NOT update window.location.search synchronously, but
// the onSelectEstimate handler below also calls setNav(0) in the same tick,
// which DOES force an immediate synchronous re-render. Without an override,
// that re-render's readUrlId() call reads the stale (pre-navigation) URL —
// and since this page has no other reactive subscription to the URL, the
// stale value is locked in forever (mirrors how `freshResult` already
// overrides the same race for the post-submit path below). Cleared once
// `restored.data` actually reflects this id (see effect below).
//
// Three states, not two: `undefined` means "no override, defer to
// readUrlId()"; `null` means "override to no-id" (handleNew's reset);
// a string means "override to this id" (a row was just clicked). Using
// plain `null` for both "no override" and "force no-id" was tried first and
// reintroduced this exact race for handleNew(): forcing pendingRestoreId
// from a bad id to `null` would, under `pendingRestoreId ?? readUrlId()`,
// fall through to readUrlId() on the very next render — which is still
// stale until router.replace("/v2") finishes — so the bad id came right
// back. The two intents need distinguishable values.
const [pendingRestoreId, setPendingRestoreId] = useState<
string | null | undefined
>(undefined);
const urlId = readUrlId();
const urlId = pendingRestoreId !== undefined ? pendingRestoreId : readUrlId();
// Post-mount flag: readUrlId() is null on SSR but a UUID on the client's first
// render, so any structure that depends on the id (e.g. the PDF <a> vs disabled
@ -459,6 +481,30 @@ export default function TradeInV2Page() {
? null
: (freshResult?.estimate_id ?? urlId);
// Clear the pendingRestoreId override once BOTH (a) the restore query has
// resolved data for that same id AND (b) window.location.search has
// genuinely caught up to it — only then is readUrlId() alone guaranteed to
// keep returning the right value, so the override is no longer needed.
// The (b) check guards a narrow edge case: if the query cache already held
// a fresh (within staleTime) response for the clicked id — e.g. it was the
// just-submitted/just-restored estimate earlier in the same SPA session —
// `restored.data` can resolve synchronously, before router.replace's async
// navigation has actually updated the URL. Clearing on (a) alone would then
// drop the override a beat too early and briefly re-expose the very race
// this state exists to prevent. If (b) isn't true yet, we simply leave the
// (still-correct) override in place — harmless, since it already matches
// what the URL will become — rather than risk reverting to a stale read.
useEffect(() => {
if (
pendingRestoreId !== undefined &&
pendingRestoreId !== null &&
restored.data?.estimate_id === pendingRestoreId &&
readUrlId() === pendingRestoreId
) {
setPendingRestoreId(undefined);
}
}, [pendingRestoreId, restored.data]);
// Per-user side data, independent of the current estimate. Both fail-soft
// (retry:false): history feeds the «Предыдущие оценки» overlay (04), quota
// feeds the TopNav «Мои отчёты» badge. Refreshed after a successful estimate
@ -637,6 +683,10 @@ export default function TradeInV2Page() {
mutation.mutate(input, {
onSuccess: (est) => {
setFreshResult(est);
// A fresh submission always supersedes any pending restore-by-id
// override (#2424 fix) — drop it so it can't shadow a later restore.
// `undefined` (not `null`): there is no override to force, just none.
setPendingRestoreId(undefined);
// A new estimate consumed quota and added a history row — refresh both
// so the TopNav badge + «Предыдущие оценки» overlay reflect it.
void queryClient.invalidateQueries({
@ -657,6 +707,10 @@ export default function TradeInV2Page() {
function handleNew() {
setFreshResult(null);
// `null`, not `undefined`: forces urlId to no-id immediately, synchronously
// clearing restoreActive/restoreError even before router.replace below
// finishes and readUrlId() would otherwise still read the stale id.
setPendingRestoreId(null);
router.replace("/v2", { scroll: false });
}
@ -925,6 +979,19 @@ export default function TradeInV2Page() {
// above (line ~649); this file uses replace() consistently for
// every /v2?id= transition, so we follow that convention here
// too rather than introducing the file's only push() call.
//
// #2424 fix: router.replace() is an async client-navigation —
// window.location.search does NOT update synchronously — but
// setNav(0) below DOES force an immediate synchronous
// re-render in the same tick. Without pendingRestoreId,
// readUrlId() on that re-render would read the stale
// (pre-navigation) URL and — since this page has no other
// reactive subscription to the URL — that stale value would
// be locked in forever. setPendingRestoreId synchronously
// hands `urlId` the correct target id, mirroring how
// `freshResult` already overrides this same race for the
// post-submit path (handleSubmit above).
setPendingRestoreId(id);
router.replace(`/v2?id=${id}`, { scroll: false });
setNav(0);
}}