# Suggested Improvements to common/components.py

## Completed

### Caching on template rendering
- Added `functools.lru_cache` on `_render_cached()` wrapper around `render_to_string`
- Cache key: `(template_path, json.dumps(context, sort_keys=True))` — deterministic and unique
- `maxsize=4096` in production, disabled entirely in DEBUG mode (so template changes are reflected immediately)
- Only caches `template` path calls; `tag_name` calls are already nanosecond string ops
- Verified working: identical calls return identical output, different inputs produce separate cache entries

### Non-deterministic IDs
`randomid()` was replaced with `hashlib.sha1(content_hash.encode()).hexdigest()[:10]` for deterministic ID generation.
- `Popover()` passes content hash (`wrapped_content:popover_content:wrapped_classes`) so IDs are deterministic per unique content
- `games/templatetags/randomid.py` uses the same hash-based approach
- Fixes: caching (Popover output now cacheable), page consistency, thread safety

### Inconsistent return types
All component functions now return `SafeText` and are annotated accordingly. Redundant `mark_safe()` wrappers removed from `LinkedPurchase()` and `NameWithIcon()`.

### Fragile A() URL resolution
Replaced single `url` parameter with explicit `url_name` (URL pattern name resolved via `reverse()`) and `href` (literal path). Removed dead `Callable` type hint. `reverse()` now raises `NoReverseMatch` instead of silently falling back to literal text. Added mutual exclusion check — providing both parameters raises `ValueError`. Updated all 10 call sites across 6 view files and internal callers (`LinkedPurchase()`, `NameWithIcon()`).

### Toast XSS vulnerability
The vulnerable `Toast()` component (which used unsafe string escaping for
Alpine.js interpolation) had no callers and was deleted entirely. Toast display
is handled by the existing event-driven pipeline: middleware → `HX-Trigger`
headers → `show-toast` CustomEvent → Alpine store.

### Default mutable arguments
All functions with mutable defaults (`attributes` and `children`) changed from `= []` to `| None = None` with `or []` conversion in the body.

What was fixed: `attributes: list[HTMLAttribute] = []` and `children: list[HTMLTag] | HTMLTag = []` are a classic Python gotcha — the default is shared across all callers and could silently corrupt state if ever mutated in place. Changed 8 functions (`Component`, `Popover`, `A`, `Button`, `Div`, `Input`, `Form`, `Icon`) to use the `None` sentinel pattern, preventing future bugs and eliminating linter warnings.

## Incomplete

### No tests
Zero test coverage for the entire component system.

**Fix**: Add unit tests for each component function — basic rendering, edge cases,
and cache hit/miss verification.

**Done**: 96 unit tests covering all component functions (`Component`, `randomid`, `Popover`, `PopoverTruncated`, `A`, `Button`, `Div`, `Icon`, `Form`, `Input`, `NameWithIcon`, `LinkedPurchase`, `PurchasePrice`, `_render_cached`, `enable_cache`). Includes template rendering, deterministic ID generation, LRU cache behavior, HTML output validation, edge cases, error handling, and model-dependent integration tests.