feat(client): adopt Idiomorph morph swap in component-client.js (B1, #22)

[fsecada01]

Scope

Issue #22 is Epic B's tracking issue (checklist B1-B5), not a single leaf phase. Per the epic's own critical-path note ("B1 unblocks Epics C, D, G") and the dev-plan doc's dependency table (B2/B3/B4 all depend on B1; B5 is an explicit post-1.0 spike), this PR scopes to B1 only:

B1 Adopt morph swap (idiomorph/morphdom) in component-client.js instead of innerHTML replace — enabler

B2 (focus/scroll/in-flight input preservation), B3 (stable list reconciliation key), B4 (JS-owned region escape hatch), and B5 (server-side diff spike) are unaddressed here and should each be their own follow-on phase.

What changed

• Vendored Idiomorph v0.7.4 (0BSD license), unmodified, at static/component_framework/js/vendor/idiomorph.js. No build tooling was introduced — this project ships hand-written vanilla ES modules with no bundler, and idiomorph's ESM dist file is a self-contained, dependency-free import, consistent with that.
• update() now calls Idiomorph.morph(element, html, { morphStyle: 'outerHTML' }) in place, instead of parsing the server HTML into a detached node and element.replaceWith()-ing it. Nodes idiomorph can match up (same tag/id) keep their identity — and therefore their already-bound event listeners, focus, and (per idiomorph's default restoreFocus: true) input selection.
• rollback() does the same against the snapshot's outerHTML. Side effect worth flagging: the old replaceWith()-based rollback silently lost all event listeners on the restored node (parsed-from-string nodes have none), so a component would go dead after a failed request until the next full update. Morphing fixes that as a natural consequence of the same technique swap — not claimed as a separate deliverable.
• tests/js/morph.test.mjs (5 new tests) verifies the integration seam — that update()/rollback() delegate to Idiomorph.morph() with the right arguments instead of doing a full replace — rather than re-testing idiomorph's own morphing algorithm, which has its own upstream test suite. This matches the existing no-jsdom, hand-stub philosophy documented in tests/js/optimistic.test.mjs.

Verification

• node --test "tests/js/**/*.test.mjs" — 13 passed (5 new + 8 existing, no regressions)
• pytest tests/ — 496 passed
• ruff check . / ruff format --check . — clean
• ty check src/ — 17 diagnostics, unchanged from baseline (this PR touches no Python source)

Out-of-scope follow-up noticed (not fixed here)

just test-js reports 0 tests locally (a cmd.exe-glob-expansion quirk in the justfile recipe, pre-existing on master before this PR — verified via git stash). CI's ci.yml test-js job calls node --test "tests/js/**/*.test.mjs" directly (not through just), so this doesn't affect CI, only local just test-js runs on Windows.

[fsecada01]

Review: PR #43 — Adopt Idiomorph morph swap (B1, #22)

Overview

Replaces component-client.js's two full-DOM-replace call sites (update(), rollback()) with in-place Idiomorph.morph() calls, vendoring Idiomorph v0.7.4 (0BSD) unmodified as a static ES module. Scope is correctly limited to B1 out of Epic B's B1-B5 checklist — B2-B4 explicitly depend on B1, B5 is an out-of-scope spike, and the PR body states this clearly. 5 new tests, 496 pytest + 13 JS tests all green, no Python files touched.

Code quality / style

• Clean, minimal diff against component-client.js — the two methods get smaller (rollback() drops the wrapper-parsing dance entirely), not larger. Good sign for a "swap the technique" change.
• Docstrings were updated to describe the new behavior accurately, not just left stale.
• The vendored file has a clear attribution header (source, pinned version, license, "do not hand-edit"). Good practice for a vendored dependency.
• Test file follows the existing optimistic.test.mjs conventions closely (same makeEl stub shape, same DOM-stub style) — consistent, easy to read side-by-side.

Specific suggestions (non-blocking)

1. Vendoring isn't reproducible. The idiomorph file was hand-copied (per the PR description, via curl from jsdelivr) with no recorded checksum or exact fetch command anywhere in the repo. A future contributor upgrading the pinned version has no way to verify they got an unmodified, correct copy short of a manual line-by-line diff against upstream. Consider adding a one-line comment with the exact fetch command + expected sha256, or a small vendor/README.md note, so re-vendoring is verifiable rather than "trust the last committer."
2. bind(element) re-scans the whole subtree on every update, even though morph's entire point is that most nodes didn't change. _bindTrigger's existing remove-then-reattach logic makes this correct (the same trigger node object survives the morph, so its _cfHandler expando persists and the old listener is properly removed before the new one attaches) — but it's now doing a full querySelectorAll('[data-event]') + rebind pass on every single dispatch response, which partially undercuts the efficiency angle of adopting morph in the first place. Not a bug, but worth a follow-up (e.g. an Idiomorph afterNodeAdded callback to bind only genuinely new trigger nodes) — probably natural scope for B2/B4 rather than this PR.
3. Emptiness-check semantics changed slightly. Old code: wrapper.innerHTML = html; !wrapper.firstElementChild (i.e., "did parsing yield a real Element?"). New code: !html || !html.trim() (i.e., "is the string non-whitespace?"). These diverge for a comment-only or non-Element HTML string (e.g. <!-- x -->): old code would treat it as empty and skip the update; new code treats it as non-empty and calls Idiomorph.morph() against content with no Element node, whose behavior in that case isn't covered by the new tests. Low-probability in practice (server always renders a real root element or a genuinely empty string), but worth either a quick test/assertion of intended behavior here, or a one-line note that this is intentionally a looser check.
4. Morphing can now insert extra top-level siblings that the old code silently dropped. wrapper.firstElementChild used to take only the first parsed element and discard everything else in the HTML string. Idiomorph.morph(..., {morphStyle: 'outerHTML'}) operates on the full parsed content and will insert all of newContent's top-level nodes as siblings if the server HTML ever contains more than one (stray whitespace/comment nodes from template rendering, or a genuine multi-root bug in a component template). Almost certainly a non-issue given components render a single root today, but it's a real behavioral change from "silently truncate to first element" to "faithfully reproduce everything," worth being aware of if a future component template ever emits incidental extra top-level content.

Potential issues / risks

• No new security surface. html was already treated as trusted, pre-rendered server output before this change (wrapper.innerHTML = html); morphing it via Idiomorph doesn't change that trust boundary — same server-controlled-HTML model as before, no new XSS vector introduced.
• Static-file serving of the new vendor/ subdirectory wasn't explicitly verified against all three adapters' static-file configuration (Django collectstatic, FastAPI StaticFiles mount, Litestar's static handler) in this review — these typically walk the static directory tree recursively so a new subdirectory should Just Work, but it'd be worth a quick manual smoke-check before relying on it in a real deployed app, since this is the first nested subdirectory under static/component_framework/js/.
• Test coverage is intentionally seam-level, not algorithm-level — by design (idiomorph has its own upstream tests, and this repo has no jsdom), so a regression in idiomorph's own morphing behavior across a future version bump wouldn't be caught by this repo's suite. Acceptable given the pinned, unmodified-vendor approach, but worth remembering if the pinned version is ever bumped.

Test coverage

Good: the 5 new tests correctly target the integration seam (that update()/rollback() call Idiomorph.morph() with the right target/content/config instead of replaceWith()), matching the PR's own stated testing philosophy. The existing 8 tests all pass unmodified, confirming no regression to the dispatch/optimistic-UI paths that don't touch update()/rollback() internals directly (they stub around them).

Verdict

No blocking findings. The four "specific suggestions" above are worth a look but none require changes before merge — they're either follow-up-phase material (B2-B4) or low-probability edge cases with no test regression today.