Lazy node-tree component system + onSwap widget lifecycle#15
Merged
Conversation
Serve alpinejs 3.15.12, @alpinejs/mask 3.15.12, flowbite 2.4.1 and flowbite-datepicker 2.0.0 from games/static/js/ instead of jsdelivr, so pages (and browser tests) work without network access. Adds the StaticScript primitive for vendored UMD bundles, which cannot be loaded as ES modules. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
Port FastHTML's proc_htmx as onSwap(selector, initializeElement) in utils.js, built on htmx.onLoad: it runs an initializer once per matching element, on initial page load and inside every htmx-swapped fragment. Migrate search_select.js, range_slider.js, filter_bar.js and add_purchase.js to it, removing the hand-rolled DOMContentLoaded + htmx:afterSwap listeners and per-element guard flags. This also fixes a latent bug: both events passed the Event object as range_slider's "force" parameter, so every htmx swap force-re-initialized all sliders and stacked duplicate listeners. The collapse button's window.initRangeSliders() call was a no-op (handles are positioned in percentages, so hidden-init is safe) and is removed with the global. Add e2e/test_widgets_e2e.py covering the onSwap lifecycle (initial-load init, htmx-swap init, single-fire toggles) plus FilterSelect pills and the add-purchase type toggle. The synthetic page in test_search_select_e2e.py now loads htmx and search_select.js as a module, matching the new initialization path. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
greenlet (pulled in by pytest-playwright) ships a manylinux wheel whose C extension links against libstdc++.so.6, which the nixpkgs Python cannot resolve, breaking pytest at plugin-load time. Expose it via an LD_LIBRARY_PATH scoped to the dev shell. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
KucharczykL
force-pushed
the
claude/kind-gauss-vj2wyp
branch
from
171cc06 to
e7db7eb
Compare
Introduce a FastHTML-style component model alongside the existing function-based one, purely additive: - Node: base renderable; __html__/__str__ render lazily so str()/f-string composition keeps working during migration. - Element: the single class for any HTML element (tag + attrs + children), rendering via the existing memoized _render_element. - Safe: wraps pre-rendered HTML (migration bridge for f-string components). - Fragment: ordered children with no wrapper tag (replaces str(a)+str(b)). - BaseComponent: base for higher-level components; render() returns a subtree, media declared via a Media attribute. - Media: declarative JS deps with order-preserving dedup merge. - collect_media()/render() helpers walk the tree. The legacy Component() function now builds an Element and is Node-aware in its child handling, so a tree mixing string- and node-returning components renders correctly with byte-identical output. No call sites changed yet. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
Generic leaf builders (Div, Span, Td, Tr, Th, Ul, Li, Strong, Label, Template, P) are now generated from one _html_element factory over the single Element class — the tag name is data, not a per-tag body. Only elements that add classes/behaviour (Button, Pill, Checkbox, Radio, Input, A, SearchField, H1, Modal, AddForm, tables) stay hand-written. All primitives now return Node objects; string-built widgets (Icon, SimpleTable, YearPicker) return Safe, and YearPicker declares its datepicker media. Raw concatenation (_popover_html, Popover slot) uses Fragment. Node.__str__/__html__ now return a SafeString: a node's rendered output is safe HTML by construction, so str(node) stays safe when fed back into a child list or template (matching the old SafeText behaviour and preventing double-escaping). Consumers adapted: the form widgets (SearchSelectWidget, PrimitiveCheckboxWidget) return render(component) so Django gets a safe string; the session form's manual field markup joins via str(row). Component tests render nodes to HTML before asserting. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
The JS-bearing widgets now declare their script dependencies, so a view no longer needs to know which scripts a component requires: - SearchSelect / FilterSelect → search_select.js - RangeSlider → range_slider.js - DateRangePicker → date_range_picker.js - YearPicker → datepicker.umd.js (external, from Phase 2) - FilterBar chrome → filter_bar.js Because the filter-bar internals now build a node tree (the legacy Component() string-builder calls became Element/Div), each bar's collect_media() returns its own filter_bar.js merged with the scripts that bubble up from the FilterSelect / RangeSlider / DateRangePicker widgets it contains — exactly the set the views thread by hand today. Adds Node.with_media() so a function-built node can declare media without a full BaseComponent subclass, and tests proving the bubbling. Note: the six *FilterBar functions still share the _filter_bar chrome helper rather than a BaseComponent class hierarchy; folding them into one is a follow-up that does not affect media collection (Phase 4). https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
Page() now calls collect_media(content) and emits the ModuleScript / StaticScript tags itself, so views no longer thread scripts= for component-owned JS. The list views (game/session/purchase/device/ platform/playevent) compose with Fragment(filter_bar, content) instead of mark_safe(str(filter_bar) + str(content)) — keeping the node tree intact so the filter bar's media (filter_bar.js + search_select.js + range_slider.js, and date_range_picker.js on purchases) reaches Page(). The stats views drop _STATS_SCRIPTS; YearPicker's datepicker.umd.js is collected from its declared media. The scripts= argument remains for page-specific glue not owned by a component (the add-form helpers add_game.js / add_purchase.js / add_session.js, alongside search_select.js for their form widgets). Adds regression tests asserting the list and stats pages auto-load their widget scripts with no scripts= in the view, and documents the node/ media model in CLAUDE.md. https://claude.ai/code/session_01BKurBhE3Qj25p7Bfsg7EeK
The onSwap migration turned filter_bar.js, range_slider.js, and
search_select.js into ES modules that register via htmx.onLoad. The five
filter synthetic e2e pages still loaded them as classic `<script defer>`
with no htmx present, so the `import { onSwap }` line was a SyntaxError and
no widget ever initialized — 18 failing tests.
Load htmx.min.js first (classic) and the three widgets as `type="module"`,
mirroring how Page() serves them in the real app. date_range_picker.js
stays a classic defer script (it is an IIFE, not a module).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The *FilterBar family (FilterBar / SessionFilterBar / PurchaseFilterBar / DeviceFilterBar / PlatformFilterBar / PlayEventFilterBar) previously shared the collapsible chrome through a free `_filter_bar(fields, ...)` helper that each function called at the end. Replace that with a `_FilterBarBase` BaseComponent: it owns the chrome render() and declares `media = _FILTER_BAR_MEDIA`, and each bar is now a subclass implementing `build_fields()`. The per-entity field-building bodies move verbatim into module-level `_<entity>_fields(existing, ...)` functions that each subclass delegates to, so the large bodies are untouched (no reindentation) and the diff stays reviewable. Media still bubbles: BaseComponent.collect_media() merges the bar's own filter_bar.js with the search_select.js / range_slider.js / date_range_picker.js declared by the contained widgets. Call sites are unchanged — `FilterBar(filter_json=..., preset_list_url=...)` now instantiates a Node instead of calling a function, and both `str(bar)` and `collect_media(bar)` behave as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cleanup of hacky leftovers from the node-tree migration (no behaviour
change):
- Return annotations: the component builders return Node subtrees, not
SafeText strings, but ~40 functions still declared `-> SafeText`. Correct
them to `-> Node` across filters / search_select / date_range_picker /
domain. The genuine string returners keep `-> SafeText`: the Alpine
selectors (GameStatusSelector / SessionDeviceSelector, which build f-string
markup) and the script-tag helpers (CsrfInput / ModuleScript /
ExternalScript / StaticScript).
- layout.render_page / layout.Page / AddForm now accept `Node` in their
`content` / `scripts` / `fields` parameters (TYPE_CHECKING import in
layout to avoid the components import cycle), matching what views already
pass.
- session._session_fields builds a `Fragment(*rows, separator="\n")` instead
of `mark_safe("\n".join(str(row) ...))` — keeps the tree intact so media
could bubble, per the Fragment convention.
- Inline SVG icon children use `Safe(...)` nodes instead of `mark_safe(...)`
strings (filters mode-toggle + collapse icons, date_range_picker calendar
icon).
- _filter_field reads the widget's own id from its node `.attributes`
(`_widget_id`) for the label's `for`, dropping the superfluous `for_widget`
argument that always rendered `for="None"`. Removes the two TODOs whose
premise ("the Component function can't expose the id") the class/node
refactor retired, plus RangeSlider's dead commented-out Label block.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The component tests rendered lazy nodes to HTML through two competing pieces of scaffolding: a magic ``_RenderingComponents.__getattr__`` proxy that auto-str()'d any capitalized builder, plus separate ``str()`` wrapper functions for Checkbox / Radio (test_components) and SearchSelect / FilterSelect / Pill (test_search_select). Replace both with one explicit convention: import the real components and wrap node-returning calls in ``str(...)`` at the call site. ``Node.__str__`` returns a ``SafeText``, so the ``assertIsInstance(..., SafeText)`` checks stay meaningful and every string assertion is unchanged. Non-node helpers (``randomid``, ``_resolve_name_with_icon``, ``_render_element``, the legacy string ``Component()``) are called directly. No production code touched; 141 component/search-select tests and the full 444-test suite pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The legacy back-compat ``Component(tag_name=...)`` function (a thin
string-returning wrapper over ``Element``) was the last piece of the
pre-node-tree API. Migrate its ~18 call sites across the views to the node
builders and remove it:
- stats_content.py: the table helpers now use the whitelisted ``Td`` / ``Th``
/ ``Tr`` builders and ``Element`` for table/tbody/thead/h1; helper return
types are ``Node``.
- auth.py / statuschange.py / game.py / purchase.py: the hand-built
``<form>`` / ``<button>`` / ``<h1>`` / ``<h2>`` / ``<table>`` markup now uses
``Element("tag", ...)``.
- core.py: drop the ``Component()`` function and its back-compat note;
``common/components/__init__`` no longer exports it.
- Tests that exercised the shim now target ``Element`` directly
(test_components cache/escaping/edge-case classes; test_node_tree drops the
legacy-parity and legacy-bridge cases, which ``Element`` coverage subsumes).
- CLAUDE.md: drop the "legacy Component retained for back-compat" notes.
Full suite green (443; one obsolete legacy-bridge test removed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KucharczykL
changed the title
The builders annotated their ``children`` parameter as
``list[HTMLTag] | HTMLTag | None`` where ``HTMLTag = str``. ``list[str]`` is
invariant, so passing ``list[Element]`` / ``list[Node]`` — the normal case —
was a type error everywhere a component nested children.
Introduce a proper child type in core:
Child = Node | str
Children = Sequence[Child] | str | None
``Sequence`` is covariant, so ``list[Element]`` / ``list[Node]`` are accepted;
``Child`` includes ``Node`` so node children are no longer rejected. ``Element``
itself also accepts a bare ``Node`` (it wraps one), typed ``Children | Node``.
Replace the ``list[HTMLTag] | HTMLTag | None`` annotations across primitives /
domain with ``Children``, and add ``as_children()`` to normalise a ``children``
argument to a ``list[Child]`` — retiring the repeated
``children if isinstance(children, list) else [children]`` dance that defeated
type narrowing. Inline ``mark_safe(...)`` SVG/markup children become ``Safe(...)``
nodes (a ``Node`` child instead of a stub-typed string).
Pyright on the component package drops from 43 to 22 errors; the remaining 22
are pre-existing and unrelated (django-stubs model access, the ``mark_safe``
``_Wrapped`` return type, and ``list[HTMLAttribute]`` attribute invariance).
Full suite green (443).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Twin of the children fix: builders annotated ``attributes`` as ``list[HTMLAttribute] | None``, and ``list`` is invariant, so passing the ``list[tuple[str, str]]`` a caller naturally writes was a type error. Add ``Attributes = Sequence[HTMLAttribute]`` (covariant) and use it for the ``attributes`` parameter of every builder. Locals that get appended/concatenated stay a concrete ``list[HTMLAttribute]`` via the new ``as_attributes()`` normaliser, mirroring ``as_children()`` — builders call it once up front so ``attributes + [...]`` keeps working on a real list. Pyright on common/components drops 45 → 42; the remaining errors are all pre-existing and unrelated (django-stubs model access, the ``mark_safe`` ``_Wrapped`` return type, and the separate ``FilterSelect`` options-list invariance). Full suite green (443). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tightens the child model so the type is honest end to end. Previously a ``SafeText``/``mark_safe`` string passed as a child rendered unescaped — a trusted-HTML-as-string backdoor that ``Child = Node | str`` couldn't express (every ``SafeText`` is a ``str``). Now ``_child_key`` escapes *every* string child; the only way to put trusted pre-rendered HTML into the tree is a ``Safe`` node. So a ``str`` child is always untrusted text — which is exactly what the renderer escapes. Converted the trusted-HTML children that relied on the old passthrough: - ``CsrfInput`` and the Alpine selectors (``GameStatusSelector`` / ``SessionDeviceSelector``) now return ``Safe`` nodes instead of ``mark_safe`` strings — they are always tree children. - ``popover_content`` is now a ``Child`` (it is rendered as a child); the one HTML caller (``LinkedPurchase``) passes ``Safe(...)``. - View-side children that were ``mark_safe`` strings → ``Safe(...)``: ``_played_row`` (game detail), the stat SVGs and `` `` spacer (game), the login table (auth), the manual session-form field/label markup (session), and ``_purchase_name`` (stats). - ``SimpleTable.header_action`` typed ``Child``. The script-tag string helpers (``ModuleScript`` / ``StaticScript`` / ``ExternalScript``) stay ``SafeText`` strings: they are only ever joined into the ``scripts=`` string, never used as tree children. ``Children`` regains a bare ``Node`` member (a single node child is valid); the one ``*children`` site (``Popover``) normalises via ``as_children`` first. Tests that asserted the old SafeText-passthrough now assert the new rule (mark_safe child escaped; ``Safe`` node passes through). Full suite green (445; +2 new escaping tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Navbar is static chrome (a few reverse() URLs in otherwise-fixed markup), so it now returns a single Safe node wrapping that markup instead of a mark_safe string — consistent with "trusted HTML is a Safe node," and a full element tree would be ~80 lines of nesting for no gain (it owns no component JS). Page() interpolates it via str() exactly as before. filter_presets.list_presets returned HttpResponse(mark_safe(...)); HttpResponse never escapes its body, so the mark_safe was pure noise — dropped. The mark_safe calls that remain are all load-bearing and not tree children: the node engine itself (core: how a node emits its SafeString), the script-tag / scripts= string helpers, and Page()'s final document string. Full suite green (445). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brainstormed design for replacing the trusted HTML/JS f-strings (Alpine selectors, @@token@@ played-row) with three composing layers: - htpy-style sugar on the existing Element (kwargs attrs + [] children), additive, keeps Media/collect_media — no build step. - Custom Elements (light DOM, TypeScript) for behavior, with the native connectedCallback lifecycle replacing the onSwap shim. - A typed contract: one Python Props type per component, codegen'd into a TS interface + attribute reader, so server↔client drift fails `tsc`. Toolchain: tsc per-module (no bundler, preserves per-component Media), build-only/gitignored output, wired into make + Docker. Exemplars: GameStatusSelector, SessionDeviceSelector, played-row. Alpine retired for those three; existing .js migrated later. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bite-sized TDD plan for the design spec: TS toolchain scaffold, htpy-style Element sugar, custom-element registry + codegen, then the three exemplar conversions (GameStatusSelector, SessionDeviceSelector, played-row) retiring their inline Alpine/@@token@@ f-strings, plus CI/Docker/docs wiring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks the component system into a FastHTML-style lazy node tree and the widget JS into an onSwap initialization lifecycle, then clears out the migration leftovers. Two tracks, landed as ordered commits.
Track A —
onSwapwidget lifecycleWidget JS (search_select, range_slider, filter_bar, add_purchase) was hand-rolling
DOMContentLoaded+htmx:afterSwap+ per-element guard flags. Replaced with a singleonSwap(selector, init)helper (a port of FastHTML'sproc_htmx, ridinghtmx.onLoad) inutils.js: runs the initializer once per matching element, on first load and inside every htmx-swapped fragment.onSwaphelper + migration of the four widget scripts.e2e/test_widgets_e2e.py: Playwright coverage of the onSwap lifecycle (initial load + htmx swap, once-per-element guard).Track B — lazy node-tree component system
Components were function builders returning
SafeTextstrings. They're nowNodeobjects rendered only when asked, soPage()can walk a finished tree and collect each component's declared JS.Node/Element(one class for any element) /Safe/Fragment/BaseComponent/Media.Div = _html_element("div"), …) over the singleElementclass.Mediathat bubbles through the tree (collect_media);SearchSelect/FilterSelect,RangeSlider,DateRangePicker,YearPicker, the filter bars.Page()collects media and emits the<script>tags itself; list views compose withFragment(...)and dropscripts=threading._FilterBarBase(BaseComponent)hierarchy (six*FilterBarsubclasses implementingbuild_fields()), so the chrome script and the contained widgets' scripts collect through one path.Cleanups (post-migration leftovers)
-> SafeTextactually returnNode→ corrected; genuine string returners (Alpine selectors, script-tag helpers) kept.render_page/Page/AddFormacceptNode.Fragment/Safereplacemark_safe("\n".join(...))andmark_safe(svg)composition._filter_fieldreads the widget's own id from its node attributes — fixes a latentfor="None"on every filter label and retires the TODOs whose blocker the node refactor removed.Component()deleted: all ~18 call sites migrated toElement/Td/Th/Tr; removed fromcore.pyand the package exports. The component API is now node-tree-only.str(...)at the call site) instead of a magic proxy + duplicatestr()shims.Infra / fixes
libstdc++.so.6to manylinux wheels (greenlet, for pytest-playwright on NixOS).type="module", matching howPage()serves them (the onSwap migration made those scripts ES modules).Testing
make testgreen: 443 unit + 37 e2e. Ruff lint + format clean on all touched files.🤖 Generated with Claude Code