Skip to content

Astro migration phase 1: follow-up cleanups + pre-existing EIP cross-link 404 #313

@dionysuzx

Description

@dionysuzx

Follow-ups from the review of #308 (Astro 6 migration, phase 1). None of these blocked the migration — grouping them here to address together in a later pass. Most are low-risk nits; one is a pre-existing link-quality issue the migration makes more visible.

Worth doing

1. Guard EIP cross-references against 404s (pre-existing) — src/components/eip/EipPage.tsx (~L216–235)

The ReactMarkdown <a> renderer rewrites any ./eip-N / EIPS/eip-N / eips.ethereum.org/EIPS/eip-N reference to an internal /eips/N link without checking that N is a tracked, emitted page. Forkcast tracks 576 EIPs, but spec bodies routinely cross-reference untracked ones — ERC-20, ERC-721, ERC-1155, ERC-2612, ERC-4337 (e.g. [ERC-4337](./eip-4337.md) in public/eips/5792.md and 5003.md). Those become internal links to pages that don't exist.

The logic is pre-existing, but the symptom changed with the migration: in the SPA an untracked /eips/N hit <Navigate to="/"> (silent redirect home); now it's a real navigation to the 404 page. Broken→broken, not working→broken, so it didn't gate #308 — but worth fixing for crawler-friendliness and UX.

Fix: only internalize when eipById.get(n) exists and isn't pending; otherwise link out to the canonical https://eips.ethereum.org/EIPS/eip-${n}. Build the URL from the matched number — don't reuse the raw href, which is often relative (./eip-4337.md) and would itself 404. eipById is already imported in this file.

if (match) {
  const n = Number(match[1]);
  const target = eipById.get(n);
  if (target && !target.pendingPullRequest) {
    return <a href={`/eips/${n}`} onClick={e => { e.preventDefault(); navigate(`/eips/${n}`); }} {...rest}>{linkChildren}</a>;
  }
  return <a href={`https://eips.ethereum.org/EIPS/eip-${n}`} target="_blank" rel="noopener noreferrer" {...rest}>{linkChildren}</a>;
}

2. Deterministic sort tiebreakers (avoid generated-data churn on unspecified upstream order)

  • scripts/snapshot-runtime-routes.mjs (~L108) — upcoming-calls sort is date-only; the committed snapshot already has a same-date pair (2026-06-10). Add || a.issueNumber - b.issueNumber.
  • src/domain/calls/upcomingCalls.ts (~L118) — apply the same tiebreak to the parallel runtime sort so the two paths stay consistent.
  • src/domain/eips/stageChanges.ts (~L69) — stage-change feed sort is date-only; add || a.eip.id - b.eip.id.

(Use issue number / id, not startTimeUtc — the existing 2026-06-10 tie has identical 15:00:00Z times, so a time key wouldn't break it.)

3. Mirror the one-off guard — src/pages/calls/[type].astro (~L15)

getStaticPaths filters one-off types out of protocolCalls but adds every upcomingCalls type unconditionally. Type-safe today (UpcomingCall.type is the closed CallType union with no one-off-* member, so no orphan /calls/one-off-XXXX route can be emitted), but mirroring if (!isOneOffCall(call.type)) on the upcoming loop removes the asymmetry and matches the file's own comment.

4. Stale deploy comment — .github/workflows/deploy.yml (L58)

# Upload dist directory (Vite build output) → it's Astro output now.

Decisions to confirm (not mechanical)

5. Snapshot YouTube-fetch swallows transient errors — scripts/snapshot-runtime-routes.mjs (L63–75)

fetchYouTubeUrl swallows non-200/network errors and returns undefined. Since hasUpcomingWatchPage = Boolean(youtubeUrl) gates route emission, a transient 403/5xx during build:fresh silently drops a watch page and churns the snapshot (no broken link — the index link and route emission stay in lockstep on the same field). Either distinguish a transient failure (rethrow so it aborts the refresh) from a genuine "no video yet" (swallow), or just fold this into the already-planned move of snapshot-routes to a scheduled commit workflow (Phase 2 doc), which makes any churn visible in a reviewed commit instead of at deploy time.

6. Matomo now reports full URLs incl. query string — src/layouts/Layout.astro (L74–79)

The old React tracker used setCustomUrl(pathname) (path-only); the new bare trackPageView defaults to location.href, so e.g. /eips/7702?tab=spec now reports as a distinct URL. Arguably more correct and consistent with the plan's intent, but it's a metrics-continuity shift worth a conscious call: keep full-URL (add a one-line documenting comment) or restore path-only (_paq.push(['setCustomUrl', location.pathname]) before the trackPageView push).


Context: surfaced by the multi-dimensional review of #308. Items 1–4 are safe mechanical changes; 5–6 are behavioral decisions.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions