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/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/livetemplate-client.ts b/livetemplate-client.ts index 768451a..e53771c 100644 --- a/livetemplate-client.ts +++ b/livetemplate-client.ts @@ -365,6 +365,66 @@ export class LiveTemplateClient { response: UpdateResponse, event?: MessageEvent ): void { + // 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 + // 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") { + // 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; + } + // Check if this is an upload-specific message const uploadMessage = response as any; if (uploadMessage.type === "upload_progress") { 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")!; diff --git a/tests/topic-error-envelope.test.ts b/tests/topic-error-envelope.test.ts new file mode 100644 index 0000000..caaa068 --- /dev/null +++ b/tests/topic-error-envelope.test.ts @@ -0,0 +1,121 @@ +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(); + }); + }); + + 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(); + }); + }); +});