code/cl8y-dex-terraclassic

Fork 0

Trade page: unknown/invalid deep-link notices wiped by the Layout keyed-Outlet remount — unknown links silently land on the default pair #358

New issue

Closed

opened 2026-06-11 02:54:48 +00:00 by Brouie · 8 comments

Brouie commented

2026-06-11 02:54:48 +00:00

(Migrated from gitlab.com)

Real-browser behavior at 3169af0 (fresh tab, address bar): /trade/ ends at /trade/<pairs[0]> (EMBER/CORAL) with NO pair-not-found notice. Documented behavior (docs/frontend.md trade-page-unknown-pair-link, the #175 acceptance): notice + URL parked at bare /trade + auto-pick blocked.

Root cause — not the #357 fix, and not the route structure:

Layout.tsx:194 renders <Outlet key={location.pathname} /> (f07a71c, #138/#182). TradePage's own cleanup navigate('/trade', {replace:true}) in the unknown branch changes the pathname -> key flips -> React remounts the whole route subtree -> unknownPairNotice (plain useState, TradePage.tsx:112) is wiped.
Fresh mount at bare /trade: allPairs returns synchronously from the query cache, routePair is undefined, every guard is null -> default-pick navigates to pairs[0] (a third mount). Net: snap to default, no notice.
Proven with a Layout-parity vitest probe: 3 mounts, path history /trade/ -> /trade -> /trade/<pairs[0]>, notice absent — exact match of the browser run. Same shell with the key REMOVED parks at /trade with the notice visible, so the keyed Outlet is the sole trigger (the separate /trade + /trade/:pairAddr Route entries do not remount by themselves on react-router 7).

Timeline — silent regression from f07a71c, not a #175 implementation bug:

87b24d9 (#176 invalid notice), 3ff34a5/00ae799 (#175 unknown notice) — both worked in the browser when verified; f07a71c landed one day after #175 and broke both flows.
Unit tests stayed green throughout: TradePage.test.tsx mounts the trade routes flat with no Layout parent, so the remount cannot happen there. That router structurally cannot catch this class of bug.
The invalid-link flow (#176) uses the identical setState-then-navigate('/trade') path, so it is latently broken the same way. Today's browser walk did observe the invalid notice + URL clear, so that flow needs one focused re-check of the FINAL url; tracking it here either way.

#357 is unaffected: the 1aeaf2c guard evaluates identically pre/post on this path (probed at e198dcb and 3169af0), and the known-pair deep-link fix verified clean in the browser.

Fix direction, smallest diff first:

Carry the notice through navigation instead of component state: navigate('/trade', {replace:true, state:{unknownPair: addr}}) and derive the notice from location.state on mount (location.state survives the remount). Same treatment for invalidLinkNotice. A search-param variant (/trade?unknown=) also works and is refresh-proof — the Outlet keys on pathname only, so a search param does not retrigger the remount.
Do NOT drop the pathname key from Layout.tsx:194 (regresses #138/#182). A single optional-param route would NOT fix it either — /trade vs /trade/ still differ in pathname.
Regression test: mount the trade routes under a Layout-parity parent rendering <Outlet key={location.pathname} /> and assert the settled final pathname AND notice visibility. The flat-router #175/#176 tests cannot see this.

Not launch-class like #357 was — every pair stays tradeable — but it voids the #175/#176 notice acceptance in production builds.

Real-browser behavior at 3169af0 (fresh tab, address bar): /trade/<valid-format terra1 addr not in the factory list> ends at /trade/<pairs[0]> (EMBER/CORAL) with NO pair-not-found notice. Documented behavior (docs/frontend.md trade-page-unknown-pair-link, the #175 acceptance): notice + URL parked at bare /trade + auto-pick blocked. Root cause — not the #357 fix, and not the route structure: - Layout.tsx:194 renders `<Outlet key={location.pathname} />` (f07a71c, #138/#182). TradePage's own cleanup navigate('/trade', {replace:true}) in the unknown branch changes the pathname -> key flips -> React remounts the whole route subtree -> unknownPairNotice (plain useState, TradePage.tsx:112) is wiped. - Fresh mount at bare /trade: allPairs returns synchronously from the query cache, routePair is undefined, every guard is null -> default-pick navigates to pairs[0] (a third mount). Net: snap to default, no notice. - Proven with a Layout-parity vitest probe: 3 mounts, path history /trade/<unknown> -> /trade -> /trade/<pairs[0]>, notice absent — exact match of the browser run. Same shell with the key REMOVED parks at /trade with the notice visible, so the keyed Outlet is the sole trigger (the separate /trade + /trade/:pairAddr Route entries do not remount by themselves on react-router 7). Timeline — silent regression from f07a71c, not a #175 implementation bug: - 87b24d9 (#176 invalid notice), 3ff34a5/00ae799 (#175 unknown notice) — both worked in the browser when verified; f07a71c landed one day after #175 and broke both flows. - Unit tests stayed green throughout: TradePage.test.tsx mounts the trade routes flat with no Layout parent, so the remount cannot happen there. That router structurally cannot catch this class of bug. - The invalid-link flow (#176) uses the identical setState-then-navigate('/trade') path, so it is latently broken the same way. Today's browser walk did observe the invalid notice + URL clear, so that flow needs one focused re-check of the FINAL url; tracking it here either way. #357 is unaffected: the 1aeaf2c guard evaluates identically pre/post on this path (probed at e198dcb and 3169af0), and the known-pair deep-link fix verified clean in the browser. Fix direction, smallest diff first: - Carry the notice through navigation instead of component state: navigate('/trade', {replace:true, state:{unknownPair: addr}}) and derive the notice from location.state on mount (location.state survives the remount). Same treatment for invalidLinkNotice. A search-param variant (/trade?unknown=<addr>) also works and is refresh-proof — the Outlet keys on pathname only, so a search param does not retrigger the remount. - Do NOT drop the pathname key from Layout.tsx:194 (regresses #138/#182). A single optional-param route would NOT fix it either — /trade vs /trade/<addr> still differ in pathname. - Regression test: mount the trade routes under a Layout-parity parent rendering `<Outlet key={location.pathname} />` and assert the settled final pathname AND notice visibility. The flat-router #175/#176 tests cannot see this. Not launch-class like #357 was — every pair stays tradeable — but it voids the #175/#176 notice acceptance in production builds.

Brouie commented

2026-06-11 02:55:05 +00:00

(Migrated from gitlab.com)

mentioned in issue #357

PlasticDigits commented

2026-06-11 15:26:52 +00:00

(Migrated from gitlab.com)

mentioned in commit a52c9e2ec6

mentioned in commit a52c9e2ec6f76d281792a918b06b37734e53045a

PlasticDigits commented

2026-06-11 15:27:02 +00:00

(Migrated from gitlab.com)

mentioned in merge request !865

PlasticDigits commented

2026-06-11 15:34:01 +00:00

(Migrated from gitlab.com)

mentioned in commit cf275720ef

mentioned in commit cf275720eff7b4df98455fc25762fabd2d4cbeba

Brouie commented

2026-06-12 03:37:00 +00:00

(Migrated from gitlab.com)

Verified the fix (a52c9e2 / !865) at the source + unit layer on 16e0656.

Root cause matches the writeup: the notices were plain useState, wiped when TradePage's own navigate('/trade') flips Layout.tsx's <Outlet key={location.pathname}> and remounts the subtree. The fix carries them in router location.state via getTradePageInvalidLinkNotice / getTradePageUnknownPairNotice, so they survive the remount — and the auto-pick effect now bails while a notice is present, which blocks the third mount that used to snap to pairs[0].

Regression test is Layout-parity: it renders the trade routes under a parent doing <Outlet key={location.pathname}>, drives an unknown deep-link and an invalid one, and asserts the settled pathname stays /trade (not /trade/) AND the notice stays in the document. That's the structural gap the flat-router #175/#176 tests couldn't see. Both new tests time out against the pre-fix TradePage (useState wiped) and pass at HEAD — proper pre/post proof.

Covers the #175/#176 notice acceptance at the unit layer. Only thing left from my side is the real-browser final-URL recheck (incl. the invalid flow the issue called out) — that's on my laptop queue. Good to close on the src+unit layer once that browser pass rides along.

Verified the fix (a52c9e2 / !865) at the source + unit layer on 16e0656. Root cause matches the writeup: the notices were plain useState, wiped when TradePage's own navigate('/trade') flips Layout.tsx's `<Outlet key={location.pathname}>` and remounts the subtree. The fix carries them in router location.state via `getTradePageInvalidLinkNotice` / `getTradePageUnknownPairNotice`, so they survive the remount — and the auto-pick effect now bails while a notice is present, which blocks the third mount that used to snap to pairs[0]. Regression test is Layout-parity: it renders the trade routes under a parent doing `<Outlet key={location.pathname}>`, drives an unknown deep-link and an invalid one, and asserts the settled pathname stays /trade (not /trade/<default>) AND the notice stays in the document. That's the structural gap the flat-router #175/#176 tests couldn't see. Both new tests time out against the pre-fix TradePage (useState wiped) and pass at HEAD — proper pre/post proof. Covers the #175/#176 notice acceptance at the unit layer. Only thing left from my side is the real-browser final-URL recheck (incl. the invalid flow the issue called out) — that's on my laptop queue. Good to close on the src+unit layer once that browser pass rides along.

Brouie commented

2026-06-12 03:50:52 +00:00

(Migrated from gitlab.com)

mentioned in issue #337

PlasticDigits commented

2026-06-12 04:12:38 +00:00

(Migrated from gitlab.com)

Verification complete — PASS

Verified fix at 8408689 (merged via !865 / a52c9e2).

Root cause (confirmed)

TradePage stored invalid/unknown pair notices in component useState. Navigating to bare /trade changed location.pathname, which remounted the route subtree via Layout.tsx <Outlet key={location.pathname} />, wiping the notice. A fresh mount at /trade then auto-picked pairs[0].

Fix (confirmed)

Notices are carried in router location.state (invalidPair / unknownPair) via navigate('/trade', { replace: true, state: … }) and read with getTradePageInvalidLinkNotice / getTradePageUnknownPairNotice. Auto-pick bails while a notice is present.

Automated — PASS

cd frontend-dapp
npm test -- --run src/pages/TradePage.test.tsx src/utils/__tests__/tradePairRoute.test.ts
# 31 passed (incl. Layout-parity #358 tests + #175/#176/#357 regressions)

Criterion	Result
Unknown pair notice survives Layout keyed-Outlet remount	PASS — `keeps pair-not-found notice after Layout keyed-Outlet remount`
Invalid pair notice survives Layout keyed-Outlet remount	PASS — `keeps invalid pair link notice after Layout keyed-Outlet remount`
Settled pathname stays `/trade` (not `/trade/<default>`)	PASS — both Layout-parity tests
`tradePairRoute` location-state helpers	PASS — 9 tests
Docs/skills updated for location-state pattern	PASS — `docs/frontend.md`, `skills/AGENTS_FRONTEND_TRADE_INVALID_PAIR_LINK.md`

Browser (Playwright Chromium against `make dev` + LocalTerra deploy) — PASS

Stack: make setup-cloud-localterra → make dev on http://127.0.0.1:5173.

Step	Result
`/trade/terra1xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx` → URL `/trade`, Pair not found notice visible, no workspace	PASS
`/trade/lilwayne%20babyyy` → URL `/trade`, Invalid pair link notice visible	PASS
`/trade/terra1nc84knc0n7td5xqplwy0luh97zd8hv5mhvm9cdempc05xk0xvxyqjr6cyg` (non-default factory pair) stays on requested pair, workspace loads	PASS (#357 unaffected)

SKIP

None.

Closing — acceptance met at src + unit + browser layers.

## Verification complete — PASS Verified fix at `8408689` (merged via !865 / `a52c9e2`). ### Root cause (confirmed) `TradePage` stored invalid/unknown pair notices in component `useState`. Navigating to bare `/trade` changed `location.pathname`, which remounted the route subtree via `Layout.tsx` `<Outlet key={location.pathname} />`, wiping the notice. A fresh mount at `/trade` then auto-picked `pairs[0]`. ### Fix (confirmed) Notices are carried in router `location.state` (`invalidPair` / `unknownPair`) via `navigate('/trade', { replace: true, state: … })` and read with `getTradePageInvalidLinkNotice` / `getTradePageUnknownPairNotice`. Auto-pick bails while a notice is present. ### Automated — PASS ```bash cd frontend-dapp npm test -- --run src/pages/TradePage.test.tsx src/utils/__tests__/tradePairRoute.test.ts # 31 passed (incl. Layout-parity #358 tests + #175/#176/#357 regressions) ``` | Criterion | Result | |-----------|--------| | Unknown pair notice survives Layout keyed-Outlet remount | **PASS** — `keeps pair-not-found notice after Layout keyed-Outlet remount` | | Invalid pair notice survives Layout keyed-Outlet remount | **PASS** — `keeps invalid pair link notice after Layout keyed-Outlet remount` | | Settled pathname stays `/trade` (not `/trade/<default>`) | **PASS** — both Layout-parity tests | | `tradePairRoute` location-state helpers | **PASS** — 9 tests | | Docs/skills updated for location-state pattern | **PASS** — `docs/frontend.md`, `skills/AGENTS_FRONTEND_TRADE_INVALID_PAIR_LINK.md` | ### Browser (Playwright Chromium against `make dev` + LocalTerra deploy) — PASS Stack: `make setup-cloud-localterra` → `make dev` on `http://127.0.0.1:5173`. | Step | Result | |------|--------| | `/trade/terra1xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx` → URL `/trade`, **Pair not found** notice visible, no workspace | **PASS** | | `/trade/lilwayne%20babyyy` → URL `/trade`, **Invalid pair link** notice visible | **PASS** | | `/trade/terra1nc84knc0n7td5xqplwy0luh97zd8hv5mhvm9cdempc05xk0xvxyqjr6cyg` (non-default factory pair) stays on requested pair, workspace loads | **PASS** (#357 unaffected) | ### SKIP None. Closing — acceptance met at src + unit + browser layers.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-12 04:12:50 +00:00

PlasticDigits commented

2026-08-26 04:16:12 +00:00

(Migrated from gitlab.com)

mentioned in issue #665