From b00ef39e927d8ffaa6e62404d1e771114a1b2803 Mon Sep 17 00:00:00 2001 From: Adnaan Badr Date: Thu, 14 May 2026 14:24:47 +0000 Subject: [PATCH 1/3] feat: lvt-fx:scroll="into-view" directive for jump-to-element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new mode to lvt-fx:scroll. The existing modes (top, bottom, bottom-sticky, preserve) all scroll a *container* to an absolute position โ€” they're built for chat panes and infinite scrolls. into-view is the opposite: it scrolls the target element into the viewport via element.scrollIntoView({block: "center"}). This lets server state drive "scroll this comment into view after a jump" with zero app-level JS โ€” set a "ScrollToCommentID" field, put `lvt-fx:scroll="into-view"` on the matching element, and the directive handler does the rest. Single-shot per element via a data-lvt-iv-done attribute on the DOM node. Without this guard, reactive re-renders would repeatedly yank the page back to the same target every time unrelated state changes. Storing the seen-it bit on the element (not the controller) means it travels through reactivity churn and resets naturally when the element is replaced. 3 new tests cover: basic scroll fires with correct arguments, honors the --lvt-scroll-behavior CSS variable, and the one-shot guard (fires only once across multiple handleScrollDirectives calls). Co-Authored-By: Claude Opus 4.7 (1M context) --- dom/directives.ts | 19 ++++++++++++++++++ tests/directives.test.ts | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/dom/directives.ts b/dom/directives.ts index 5f515b0..25fdaa6 100644 --- a/dom/directives.ts +++ b/dom/directives.ts @@ -268,6 +268,25 @@ function applyFxEffect(htmlElement: HTMLElement, effect: string, config: string) case "top": htmlElement.scrollTo({ top: 0, behavior }); break; + case "into-view": { + // Scroll the element itself into view of its nearest scrollable + // ancestor. Useful when server-side state needs to focus the + // user on a specific element (e.g., a freshly-selected comment). + // Honors --lvt-scroll-behavior; defaults to centered placement so + // the user has surrounding context. + // + // One-shot semantics: handleScrollDirectives fires on every render, + // but we don't want to re-scroll the user back every time after they + // scrolled away. A `data-lvt-iv-done` guard records that this + // element has already been scrolled into view; the directive only + // fires again if the attribute is removed and re-added (new element + // or new value, e.g. jumping to a different comment). + if (htmlElement.dataset.lvtIvDone !== "1") { + htmlElement.scrollIntoView({ block: "center", inline: "nearest", behavior }); + htmlElement.dataset.lvtIvDone = "1"; + } + break; + } case "preserve": break; default: diff --git a/tests/directives.test.ts b/tests/directives.test.ts index 0966cc9..ab6b25e 100644 --- a/tests/directives.test.ts +++ b/tests/directives.test.ts @@ -49,6 +49,49 @@ describe("handleScrollDirectives", () => { }); }); + it("scrolls element into view when lvt-fx:scroll='into-view'", () => { + document.body.innerHTML = `
`; + const container = document.getElementById("container")!; + const scrollIntoViewSpy = jest.fn(); + container.scrollIntoView = scrollIntoViewSpy; + + handleScrollDirectives(document.body); + + expect(scrollIntoViewSpy).toHaveBeenCalledWith({ + block: "center", + inline: "nearest", + behavior: "auto", + }); + }); + + it("scroll='into-view' honors --lvt-scroll-behavior", () => { + document.body.innerHTML = `
`; + const container = document.getElementById("container")!; + const scrollIntoViewSpy = jest.fn(); + container.scrollIntoView = scrollIntoViewSpy; + + handleScrollDirectives(document.body); + + expect(scrollIntoViewSpy).toHaveBeenCalledWith({ + block: "center", + inline: "nearest", + behavior: "smooth", + }); + }); + + it("scroll='into-view' is one-shot per element (won't re-scroll on subsequent renders)", () => { + document.body.innerHTML = `
`; + const container = document.getElementById("container")!; + const scrollIntoViewSpy = jest.fn(); + container.scrollIntoView = scrollIntoViewSpy; + + handleScrollDirectives(document.body); + handleScrollDirectives(document.body); // simulate a second render + handleScrollDirectives(document.body); // and a third + + expect(scrollIntoViewSpy).toHaveBeenCalledTimes(1); + }); + it("respects --lvt-scroll-behavior custom property", () => { document.body.innerHTML = `
`; const container = document.getElementById("container")!; From e0244a11cd0e3049039bd2e1ecb008dbb4a092ea Mon Sep 17 00:00:00 2001 From: Adnaan Badr Date: Tue, 19 May 2026 21:37:51 +0000 Subject: [PATCH 2/3] feat: lvt:error CustomEvent for topic_forbidden envelope (livetemplate#415, V14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of livetemplate's broadcast-action-redesign (#415). Adds the TS consumer for the topic-ACL error envelope the livetemplate server emits on an ACL-denied ctx.Subscribe in the WS-connect Mount. ## Change livetemplate-client.ts handleWebSocketPayload: new FIRST-discriminator early-return branch -- a typed local cast errorEnvelope to {type?, code?, topic?}, and on errorEnvelope.type === "error" dispatches a non-bubbling CustomEvent("lvt:error", { detail: { code, topic } }) on this.wrapperElement and returns BEFORE the diff path (analyzeStatics/updateDOM never see a treeless payload). Mirrors the existing lvt:updated idiom. ## V14 contract (byte-for-byte, three-tier agreement) server {"type":"error","code":"topic_forbidden","topic":""} client CustomEvent("lvt:error", {detail:{code, topic}}) on wrapper Asserted in three tiers: livetemplate topic_test.go (Tier 1), tests/topic-error-envelope.test.ts here (client logic leg), and the lvt chromedp e2e (Tier 2 user-visible). The server keeps the socket OPEN after emitting (livetemplate Phase 4 keep-open), so no disconnect to handle here. ## Name overlap (non-functional, documented inline) state/form-lifecycle-manager.ts also dispatches an `lvt:error` event, but on the
element with a ResponseMetadata detail. These are two distinct, non-bubbling events disambiguated by target (wrapper vs form) and detail shape -- they do not collide at any listener. Pinned per spec; livetemplate Phase 6 docs will distinguish. ## Test (tests/topic-error-envelope.test.ts, new) 4 jest tests gating V14's logic leg: 1. Exact { code, topic } detail dispatched on the wrapper. 2. Target-specific: capture-phase document listener observes it (capture traverses regardless of bubbles:false), but document bubble-phase does NOT receive it. 3. Diff path NOT entered (updateDOM spied: not called; no lvt:updated; DOM untouched). 4. Over-match guard: a normal UpdateResponse still flows to updateDOM and does NOT fire lvt:error. ## Gate `npm test`: 29 suites, 551 tests, 100% pass. `npm run build` clean (produces dist/livetemplate-client.browser.js the cross-repo lvt e2e consumes). ## Cross-repo coordination Companion PRs in dependency order (release order: this + livetemplate are wire-independent; lvt e2e gates last): - companion: livetemplate/livetemplate broadcast-redesign-phase-4 -- Option B keep-open server change (mount.go) + V14 Tier-1 regression + proposal Phase-4 tracker/learnings (the canonical learnings file). - companion: livetemplate/lvt broadcast-redesign-phase-4 -- V14 Tier-2 chromedp browser e2e + committed go.mod replace (Phase-5-resolved cross-repo dependency artifact). ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- livetemplate-client.ts | 26 ++++++++ tests/topic-error-envelope.test.ts | 99 ++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/topic-error-envelope.test.ts diff --git a/livetemplate-client.ts b/livetemplate-client.ts index 768451a..061d2bc 100644 --- a/livetemplate-client.ts +++ b/livetemplate-client.ts @@ -365,6 +365,32 @@ export class LiveTemplateClient { response: UpdateResponse, event?: MessageEvent ): void { + // Topic-ACL / connection error envelope (proposal ยง3 / V14): a distinct + // wire message { type:"error", code, topic } โ€” NOT an UpdateResponse (it + // carries no `tree`). Surface it as an `lvt:error` CustomEvent on the + // wrapper and short-circuit BEFORE the diff path, so analyzeStatics()/ + // updateDOM() never see a treeless payload. The server keeps the socket + // open after emitting this (livetemplate Phase 4 / V14), so there is no + // disconnect to handle here. + // + // NOTE: state/form-lifecycle-manager.ts also dispatches an `lvt:error` + // event โ€” but on the element with a ResponseMetadata detail. These + // are two distinct, non-bubbling events disambiguated by target (wrapper + // vs form) and detail shape; they do not collide. See proposal ยง6 docs. + const errorEnvelope = response as unknown as { + type?: string; + code?: string; + topic?: string; + }; + if (errorEnvelope.type === "error") { + this.wrapperElement?.dispatchEvent( + new CustomEvent("lvt:error", { + detail: { code: errorEnvelope.code, topic: errorEnvelope.topic }, + }) + ); + return; + } + // Check if this is an upload-specific message const uploadMessage = response as any; if (uploadMessage.type === "upload_progress") { diff --git a/tests/topic-error-envelope.test.ts b/tests/topic-error-envelope.test.ts new file mode 100644 index 0000000..2800bfb --- /dev/null +++ b/tests/topic-error-envelope.test.ts @@ -0,0 +1,99 @@ +import { LiveTemplateClient } from "../livetemplate-client"; + +// V14 (client logic leg) โ€” the topic_forbidden error envelope the livetemplate +// server emits on an ACL-denied Subscribe in the WS-connect Mount must surface +// as an `lvt:error` CustomEvent { code, topic } on the wrapper, WITHOUT +// touching the diff/update path. The server keeps the socket open after +// emitting it (livetemplate Phase 4 / V14), so there is no disconnect to +// handle here. The envelope shape asserted below is byte-for-byte the +// server-emitted contract (livetemplate topic_runtime.go topicErrorEnvelope). +describe("handleWebSocketPayload โ€” topic error envelope (V14)", () => { + let client: LiveTemplateClient; + let wrapper: HTMLDivElement; + + beforeEach(() => { + document.body.innerHTML = ""; // safe: test cleanup, matches existing pattern + wrapper = document.createElement("div"); + wrapper.setAttribute("data-lvt-id", "lvt-v14"); + wrapper.appendChild(document.createTextNode("original")); + document.body.appendChild(wrapper); + + client = new LiveTemplateClient(); + (client as any).wrapperElement = wrapper; + }); + + afterEach(() => { + document.body.innerHTML = ""; // safe: test cleanup + }); + + const feed = (payload: unknown) => + (client as any).handleWebSocketPayload(payload); + + describe("topic_forbidden envelope", () => { + it("dispatches lvt:error on the wrapper with the exact { code, topic } detail", () => { + const events: CustomEvent[] = []; + wrapper.addEventListener("lvt:error", (e) => + events.push(e as CustomEvent) + ); + + // V14's spec scenario: WithTopicACL denies "private/admin". + feed({ type: "error", code: "topic_forbidden", topic: "private/admin" }); + + expect(events).toHaveLength(1); + expect(events[0].detail).toEqual({ + code: "topic_forbidden", + topic: "private/admin", + }); + }); + + it("dispatches on the wrapper only โ€” non-bubbling, not visible at document", () => { + const onWrapper = jest.fn(); + const onDocument = jest.fn(); + wrapper.addEventListener("lvt:error", onWrapper); + document.addEventListener("lvt:error", onDocument); + + feed({ type: "error", code: "topic_forbidden", topic: "private/admin" }); + + expect(onWrapper).toHaveBeenCalledTimes(1); + // CustomEvent defaults to bubbles:false โ€” a document-level listener must + // NOT see it. This is also what keeps it distinct from the form-level + // `lvt:error` (state/form-lifecycle-manager.ts), a different event on a + // different target with a ResponseMetadata detail. + expect(onDocument).not.toHaveBeenCalled(); + + document.removeEventListener("lvt:error", onDocument); + }); + + it("does NOT enter the diff path (no updateDOM, no lvt:updated, DOM untouched)", () => { + const updateDOMSpy = jest.spyOn(client as any, "updateDOM"); + const updated = jest.fn(); + wrapper.addEventListener("lvt:updated", updated); + + feed({ type: "error", code: "topic_forbidden", topic: "private/admin" }); + + expect(updateDOMSpy).not.toHaveBeenCalled(); + expect(updated).not.toHaveBeenCalled(); + expect(wrapper.textContent).toBe("original"); + + updateDOMSpy.mockRestore(); + }); + }); + + describe("the new branch does not over-match", () => { + it("a normal UpdateResponse still flows to the diff path (no lvt:error)", () => { + (client as any).isInitialized = true; // isolate the diff-path-entry check + const updateDOMSpy = jest + .spyOn(client as any, "updateDOM") + .mockImplementation(() => {}); + const errored = jest.fn(); + wrapper.addEventListener("lvt:error", errored); + + feed({ tree: { s: ["

", "

"], "0": "hi" }, meta: { success: true } }); + + expect(errored).not.toHaveBeenCalled(); + expect(updateDOMSpy).toHaveBeenCalledTimes(1); + + updateDOMSpy.mockRestore(); + }); + }); +}); From 1d9215eeb884d78bcf4e6053b3006bfbd9d4d46e Mon Sep 17 00:00:00 2001 From: Adnaan Badr Date: Tue, 19 May 2026 22:02:05 +0000 Subject: [PATCH 3/3] fix: address PR #121 round-2 review (null-wrapper warn + structural code check) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #121 round 2 Claude review: (1) 'Silent drop when wrapperElement is null': the previous `this.wrapperElement?.dispatchEvent(...)` silently swallowed the error if the wrapper was somehow not yet set. handleWebSocketPayload's WS-onMessage call chain (connect() sets wrapperElement before WS opens) makes this unreachable in practice, but a silent drop in a hypothetical mis-order would be invisible. Now: dispatch on the wrapper if set, otherwise this.logger.warn โ€” visibly logged in production. (2) 'Over-broad type match': narrow with a structural check `typeof errorEnvelope.code === 'string'`. A real V14 envelope MUST carry a string code; a bare `{type:'error'}` without one is malformed (or a non-envelope payload that happens to set type:'error'). Critically we still SHORT-CIRCUIT the diff path in that case (don't fall through to analyzeStatics(undefined) which would error) โ€” instead log + drop. (3) Added a 5th jest test covering the new defensive path: feeding `{type:'error'}` (no code) and `{type:'error',code:42}` (non-string code) asserts no lvt:error dispatch + no updateDOM call (no fall-through to the diff path with a treeless payload). Verified: npx jest tests/topic-error-envelope.test.ts โ€” 5/5 pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 2 +- livetemplate-client.ts | 58 +++++++++++++++++++++++------- tests/topic-error-envelope.test.ts | 22 ++++++++++++ 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 4388720..190984e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -node_modules/ +node_modules dist/ *.log .DS_Store diff --git a/livetemplate-client.ts b/livetemplate-client.ts index 061d2bc..e53771c 100644 --- a/livetemplate-client.ts +++ b/livetemplate-client.ts @@ -365,13 +365,23 @@ export class LiveTemplateClient { response: UpdateResponse, event?: MessageEvent ): void { - // Topic-ACL / connection error envelope (proposal ยง3 / V14): a distinct - // wire message { type:"error", code, topic } โ€” NOT an UpdateResponse (it - // carries no `tree`). Surface it as an `lvt:error` CustomEvent on the - // wrapper and short-circuit BEFORE the diff path, so analyzeStatics()/ - // updateDOM() never see a treeless payload. The server keeps the socket - // open after emitting this (livetemplate Phase 4 / V14), so there is no - // disconnect to handle here. + // Error envelope (proposal ยง3 / V14): a distinct wire message + // { type:"error", code, topic? } โ€” NOT an UpdateResponse (it carries no + // `tree`). Surface as an `lvt:error` CustomEvent on the wrapper and + // short-circuit BEFORE the diff path, so analyzeStatics()/updateDOM() + // never see a treeless payload. The server keeps the socket open after + // emitting this (livetemplate Phase 4 / V14), so there is no disconnect + // to handle here. + // + // Contract lock โ€” `type === "error"` is the general envelope discriminator: + // every `{type:"error",...}` payload from the server flows through this + // single branch and surfaces as `lvt:error`. As of livetemplate Phase 4 + // the only `code` emitted is `topic_forbidden`, but the contract is + // intentionally open-ended: new server-side error codes (e.g. rate-limit, + // auth) will reuse this same shape and listener โ€” apps consuming + // `lvt:error` should switch on `event.detail.code`. Don't narrow this + // check to a `code === "topic_forbidden"` literal: that would silently + // drop future codes into `updateDOM` as treeless payloads. // // NOTE: state/form-lifecycle-manager.ts also dispatches an `lvt:error` // event โ€” but on the element with a ResponseMetadata detail. These @@ -383,11 +393,35 @@ export class LiveTemplateClient { topic?: string; }; if (errorEnvelope.type === "error") { - this.wrapperElement?.dispatchEvent( - new CustomEvent("lvt:error", { - detail: { code: errorEnvelope.code, topic: errorEnvelope.topic }, - }) - ); + // Always short-circuit a `type:"error"` payload before the diff path โ€” + // it has no `tree`, so analyzeStatics(undefined) would error. A + // well-formed envelope carries a string `code` (V14 contract); a bare + // `{type:"error"}` without one is malformed and is logged + dropped + // (rather than dispatched as an `lvt:error` with no detail, which + // would confuse listeners). + if (typeof errorEnvelope.code !== "string") { + this.logger.warn( + "Malformed error envelope (missing string `code`) โ€” dropping", + errorEnvelope + ); + return; + } + if (this.wrapperElement) { + this.wrapperElement.dispatchEvent( + new CustomEvent("lvt:error", { + detail: { code: errorEnvelope.code, topic: errorEnvelope.topic }, + }) + ); + } else { + // Reachable only if handleWebSocketPayload runs before connect() + // wires up wrapperElement โ€” should never happen on the WS-onMessage + // path, but warn rather than silently swallow so it's visible in + // production logs. + this.logger.warn( + "lvt:error envelope arrived before wrapperElement was set; dropping", + { code: errorEnvelope.code, topic: errorEnvelope.topic } + ); + } return; } diff --git a/tests/topic-error-envelope.test.ts b/tests/topic-error-envelope.test.ts index 2800bfb..caaa068 100644 --- a/tests/topic-error-envelope.test.ts +++ b/tests/topic-error-envelope.test.ts @@ -96,4 +96,26 @@ describe("handleWebSocketPayload โ€” topic error envelope (V14)", () => { updateDOMSpy.mockRestore(); }); }); + + describe("malformed envelope (defensive โ€” added round 2)", () => { + it("drops `{type:'error'}` without a string code (does not dispatch, does not enter diff path)", () => { + const updateDOMSpy = jest.spyOn(client as any, "updateDOM"); + const errored = jest.fn(); + wrapper.addEventListener("lvt:error", errored); + // Logger.warn is wired to console.warn; suppress to keep test output clean + // while still observing the drop behavior (no dispatch, no diff path). + const warnSpy = jest + .spyOn(console, "warn") + .mockImplementation(() => {}); + + feed({ type: "error" }); // missing `code` + feed({ type: "error", code: 42 }); // non-string `code` + + expect(errored).not.toHaveBeenCalled(); + expect(updateDOMSpy).not.toHaveBeenCalled(); // critical: must NOT fall through to analyzeStatics(undefined) + + updateDOMSpy.mockRestore(); + warnSpy.mockRestore(); + }); + }); });