# Frontend Code Review – Explorer Monorepo

**Scope:** `explorer-monorepo/frontend/`  
**Reviewed:** Vanilla JS SPA (`public/index.html`), React/Next.js app (`src/`), services, config.  
**Date:** 2025-02-09

**Update (post-review):** All "Improve" items from this review were implemented per [FRONTEND_TASKS_AND_REVIEW.md](FRONTEND_TASKS_AND_REVIEW.md): response.ok checks (C1–C3), blocks API normalizer (C4/M4), escapeHtml/safe URLs/onclick (H1–H3), getRpcUrl in rpcCall (H4), cancel blocks rAF (H5), block number validation (M1), stable list keys (M2), named constants (M3), shared block card helper (L1), DEBUG console gating (L2), aria-live for errors (L3), API modules (L4). See that file for the full task list and status.

---

## 1. Overview

The frontend has two delivery paths:

| Asset | Purpose | Deployed |
|-------|--------|----------|
| **`public/index.html`** | Single-page explorer (Blocks, Transactions, Bridge, WETH, search, wallet connect). Vanilla JS, ~4.2k lines. | Yes (VMID 5000, https://explorer.d-bis.org) |
| **`src/`** (Next.js) | React app (blocks, transactions, addresses, search, wallet). Uses Blockscout-style API. | No (dev/build only) |

---

## 2. Vanilla JS SPA (`public/index.html`)

### 2.1 Security

**Good:**
- **`escapeHtml()`** is used for error messages, revert reasons, method names, ABI/bytecode, token names/symbols, NFT metadata, and other user/API-derived strings before `innerHTML`. Reduces XSS from API or user input.
- **CSP** in `<meta>` restricts script/style/font/connect sources; comment documents `unsafe-eval` for ethers v5 UMD.
- **Credentials:** `fetchAPI` uses `credentials: 'omit'` for API calls.
- **Wallet:** No private keys or secrets in code; MetaMask/ethers used for signing.

**Improve:**
- **Defense in depth for hashes/addresses:** Any API-derived string (e.g. `hash`, `from`, `to`, `address`) that is interpolated into `innerHTML` should be escaped. Currently:
  - Block cards: `shortenHash(hash)` and block number are injected without `escapeHtml`. Block numbers are numeric; hashes are usually hex from a trusted API, but escaping would harden against a compromised or malicious API.
  - Breadcrumbs: `shortenHash(identifier)` and `identifier` in `href="#/address/' + identifier + '"` – if `identifier` can contain `'` or `"`, it could break attributes or enable injection. Recommend `escapeHtml(shortenHash(hash))` for display and sanitize/validate for attributes.
- **`shortenHash`:** Only truncates; does not strip HTML. Use `escapeHtml(shortenHash(hash))` wherever the result is used in HTML.

### 2.2 Correctness & Robustness

**Good:**
- **Navigation:** Re-entrancy guard (`_inNavHandler`) and “showView first, then set hash” prevent the previous infinite recursion from hashchange.
- **`applyHashRoute`:** Uses `currentView` and `currentDetailKey` so the same view/detail is not re-applied unnecessarily.
- **`fetchAPI`:** Uses `AbortController` and 15s timeout; `AbortError` is handled in retry logic.
- **`fetchAPIWithRetry`:** Exponential backoff; retries on timeout/5xx/network; does not retry on non-retryable errors.
- **RPC fallback:** When Blockscout API fails, blocks/transactions can be loaded via `rpcCall` (e.g. `eth_blockNumber`, `eth_getBlockByNumber`).
- **Validation:** `safeBlockNumber`, `safeTxHash`, `safeAddress` used before detail views and in retry buttons.
- **Wrap/Unwrap:** Balance checks and `callStatic` simulation before deposit/withdraw; user rejection vs contract error distinguished in messages.

**Improve:**
- **`rpcCall`:** Uses a single RPC URL; does not use `getRpcUrl()` for failover. Consider using `getRpcUrl()` for critical RPC calls so the app benefits from the same failover as elsewhere.
- **Memory/cleanup:** `requestAnimationFrame(animateScroll)` in the blocks scroll runs indefinitely. On view change, the loop is not cancelled; consider storing the frame id and cancelling in a cleanup when leaving the blocks view.
- **Breadcrumb `identifier`:** In `updateBreadcrumb`, `identifier` is used in `href="#/address/' + identifier + '"`. If `identifier` contains `'`, the attribute can break. Prefer escaping or using `encodeURIComponent` for path segments.

### 2.3 Structure & Maintainability

- **Single file:** ~4.2k lines in one HTML file makes navigation and testing harder. Consider splitting script into logical modules (e.g. API, nav, views, wallet) and bundling, or at least grouping related functions and marking sections.
- **Duplicate logic:** Block card HTML is built in multiple places (home stats area, blocks list, block detail). A single `createBlockCard`-style helper (or shared template) would reduce drift and bugs.
- **Magic numbers:** Timeouts (15s, 5s), retry counts (3), and delays are literal; consider named constants at the top of the script.
- **Console:** Several `console.log`/`console.warn`/`console.error` calls are useful for debugging but could be gated by a `DEBUG` flag or removed for production if desired.

### 2.4 Accessibility & UX

- Nav links use `onclick` and `aria-label` where checked; focus and keyboard flow should be verified (e.g. tab order, Enter to activate).
- Error messages and retry buttons are visible; consider ensuring they are announced (e.g. live region) for screen readers.
- Dark theme is supported and persisted in `localStorage`.

---

## 3. React/Next.js App (`src/`)

### 3.1 Security

- **React escaping:** Components render props as text by default, so no raw `dangerouslySetInnerHTML` was found; Address, Card, Table, etc. are safe from XSS in normal use.
- **API client:** Uses `localStorage.getItem('api_key')` for `X-API-Key`; ensure key is not exposed in logs or error messages.
- **Env:** `NEXT_PUBLIC_*` is appropriate for client-side config; no secrets in frontend code.

### 3.2 Data Fetching & API Shape

**Issues:**
- **`addresses/[address].tsx`:** Uses `fetch()` then `response.json()` without checking `response.ok`. On 4xx/5xx, `data` may be an error body and `setAddressInfo(data.data)` can set invalid state. Recommend: check `response.ok`, handle errors, and only set state on success.
- **`blocksApi.list`:** Returns `ApiResponse<Block[]>` (expects `{ data: Block[] }`). If the backend returns a different shape (e.g. `{ items: [] }`), `response.data` may be `undefined` and `setBlocks(response.data)` can lead to runtime errors in `blocks.map()`. Align client with actual API response or normalize in the client.
- **Home page:** `loadRecentBlocks()` uses `blocksApi.list`; same shape assumption as above. `loadStats()` is a placeholder (no real API call).

### 3.3 Correctness

- **useEffect deps:** In `page.tsx`, `useEffect(..., [])` calls `loadStats` and `loadRecentBlocks` which are recreated each render. This is fine with empty deps (run once). In `blocks/index.tsx` and `transactions/index.tsx`, deps are `[page]`; `loadBlocks`/`loadTransactions` are not in the dependency array, which is intentional to avoid unnecessary runs; no bug found.
- **Block detail:** `blocks/[number].tsx` uses `parseInt((params?.number as string) ?? '0')` – invalid or missing number becomes `NaN`/`0`; the API may return 404. Consider validating and showing a clear “Invalid block number” message.
- **Table key:** `Table.tsx` uses `key={rowIndex}`; if the list is reordered or filtered, prefer a stable key (e.g. `block.number`, `tx.hash`).

### 3.4 Consistency & Gaps

- **Routing:** Next.js app uses file-based routes (`/blocks`, `/transactions`, `/addresses/[address]`). The deployed SPA uses hash routing (`#/blocks`, `#/address/0x...`). They are separate; no conflict, but be aware that deep links differ between the two frontends.
- **API base:** React app uses `NEXT_PUBLIC_API_URL` (default `http://localhost:8080`). The vanilla SPA uses same-origin `/api` or Blockscout API. Ensure backend and env are aligned when running the Next app against a real API.
- **blocks API:** Only blocks are implemented in `services/api/`; transactions and addresses use raw `fetch` in pages. Consider moving to shared API modules and the same client for consistency and error handling.

---

## 4. Services & Config

### 4.1 API Client (`src/services/api/client.ts`)

- Axios instance with timeout (30s), JSON headers, and optional `X-API-Key` from `localStorage`.
- Response interceptor rejects with `error.response.data`; type is `ApiError`. Callers should handle both API error shape and network errors.
- **Note:** Used by blocks API only; other pages use `fetch` directly.

### 4.2 Blocks API (`src/services/api/blocks.ts`)

- Builds query params correctly; uses `apiClient.get<Block[]>`.
- Return type `ApiResponse<Block[]>` assumes backend returns `{ data: T }`. If backend is Blockscout-style (`items`, etc.), either add an adapter or document the expected backend contract.

### 4.3 Next.js Config

- `reactStrictMode: true`, `output: 'standalone'`.
- `NEXT_PUBLIC_API_URL` and `NEXT_PUBLIC_CHAIN_ID` defaulted in `next.config.js`; can be overridden by env.

---

## 5. Recommendations Summary

| Priority | Item | Action |
|----------|------|--------|
| **P1** | Address page fetch | In `addresses/[address].tsx`, check `response.ok` and handle non-2xx before parsing JSON and setting state. |
| **P1** | API response shape | Confirm backend response shape for blocks (and any shared APIs). Normalize to `{ data }` in client or document and handle `items`/other shapes. |
| **P2** | XSS hardening (SPA) | Use `escapeHtml(shortenHash(hash))` (and escape other API-derived strings) wherever content is written with `innerHTML`. Escape or encode `identifier` in breadcrumb `href`. |
| **P2** | RPC failover | Use `getRpcUrl()` inside `rpcCall()` (or for critical paths) so RPC failover is consistent. |
| **P2** | Blocks scroll animation | Cancel `requestAnimationFrame` when leaving the blocks view (e.g. in a cleanup or when switching view). |
| **P3** | SPA structure | Split script into modules or clearly grouped sections; extract shared constants and block-card markup. |
| **P3** | Table keys | Use stable keys (e.g. `block.number`, `tx.hash`) in list components instead of index. |
| **P3** | Block number validation | In `blocks/[number].tsx`, validate `params.number` and show a clear message for invalid or missing block number. |

---

## 6. Files Reviewed

- `public/index.html` – full read and grep for escapeHtml, innerHTML, fetch, navigation, wallet.
- `src/app/layout.tsx`, `src/app/page.tsx`, `src/app/wallet/page.tsx`
- `src/pages/_app.tsx`, `src/pages/blocks/index.tsx`, `src/pages/blocks/[number].tsx`, `src/pages/transactions/index.tsx`, `src/pages/transactions/[hash].tsx`, `src/pages/addresses/[address].tsx`, `src/pages/search/index.tsx`
- `src/components/common/Card.tsx`, `Button.tsx`, `Table.tsx`
- `src/components/blockchain/Address.tsx`, `src/components/wallet/AddToMetaMask.tsx`
- `src/services/api/client.ts`, `src/services/api/blocks.ts`
- `package.json`, `next.config.js`