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

1. Inconsistent return types (completed)

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

2. Fragile A() URL resolution

Tries reverse(url) first, then falls back to literal string. Uses type(url) is str instead of isinstance(). Intentional but error-prone — a string matching a URL name will be reversed, while one that doesn't pass through as-is.

Fix: Add explicit parameter like url_name="view_game" vs href="/literal/path".

3. Toast XSS vulnerability

Custom string escaping for Alpine.js interpolation:

safe_message = message.replace("\\", "\\\\").replace("`", "\\`")

Doesn't protect against all injection vectors (e.g., }) could close the Alpine expression early).

Fix: Use proper HTML escaping + JSON serialization for safe template interpolation.

4. 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.

5. Default mutable arguments

attributes: list[HTMLAttribute] = [] is a classic Python gotcha (though harmless here since the list is only read, never mutated in place).

Fix: Use attributes: list[HTMLAttribute] | None = None and convert to [] inside the function body.

2.4 KiB Raw Blame History