Public: anyone can read this; the crew can change it.
No page can poll on a timer: the unused data-live-every beat leaves the client #10817
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10789-no-page-can-poll-on-a-timer-the-unused-d → main 2 commits fork
4 files
changed +4
−125
src/web/client.ts −19
| @@ -2414,8 +2414,6 @@ export const liveRegionsJs = /* js */ ` | ||
| 2414 | 2414 | mine.innerHTML = theirs.innerHTML; |
| 2415 | 2415 | opened.forEach(function (i) { var d = document.getElementById(i); if (d) d.open = true; }); |
| 2416 | 2416 | Array.prototype.forEach.call(theirs.attributes, function (a) { if (a.name !== 'id') mine.setAttribute(a.name, a.value); }); |
| 2417 | if (!theirs.hasAttribute('data-live-every')) mine.removeAttribute('data-live-every'); | |
| 2418 | arm(mine); | |
| 2419 | 2417 | mine.dispatchEvent(new CustomEvent('forge:swapped', { bubbles: true })); |
| 2420 | 2418 | }); |
| 2421 | 2419 | } |
| @@ -2528,23 +2526,6 @@ export const liveRegionsJs = /* js */ ` | ||
| 2528 | 2526 | if (hit && timer === null) |
| 2529 | 2527 | timer = setTimeout(refresh, Math.max(600, last + MIN_GAP - Date.now())); |
| 2530 | 2528 | } |
| 2531 | // A region with data-live-every="<seconds>" is also due on that beat, for | |
| 2532 | // what no event announces; it rides the same fetch, etag and swap. | |
| 2533 | // The beat lasts as long as the region asks for it: a swap that drops the | |
| 2534 | // attribute ends it, and one that brings it (a check starting) begins it. | |
| 2535 | var beats = {}; | |
| 2536 | function arm(r) { | |
| 2537 | var s = parseInt(r.getAttribute('data-live-every'), 10); | |
| 2538 | if (!r.id) return; | |
| 2539 | if (!(s > 0)) { | |
| 2540 | if (beats[r.id]) { window.clearInterval(beats[r.id]); delete beats[r.id]; } | |
| 2541 | return; | |
| 2542 | } | |
| 2543 | if (beats[r.id]) return; | |
| 2544 | var id = r.id; | |
| 2545 | beats[id] = window.setInterval(function () { due[id] = true; if (timer === null) timer = setTimeout(refresh, 0); }, s * 1000); | |
| 2546 | } | |
| 2547 | Array.prototype.forEach.call(document.querySelectorAll('[data-live-every]'), arm); | |
| 2548 | 2529 | connect(); |
| 2549 | 2530 | // Back online: reconnect from the last id and look at every region once, |
| 2550 | 2531 | // in case the replay window no longer reaches back that far. |
test/live-pages.test.ts −102
| @@ -189,108 +189,6 @@ describe("live browsing pages", () => { | ||
| 189 | 189 | }); |
| 190 | 190 | }); |
| 191 | 191 | |
| 192 | describe("the beat of a region with data-live-every", () => { | |
| 193 | type Attrs = Record<string, string>; | |
| 194 | const region = (attrs: Attrs, html: string) => { | |
| 195 | const r = { | |
| 196 | id: attrs["id"] ?? "", | |
| 197 | attrs: { ...attrs }, | |
| 198 | innerHTML: html, | |
| 199 | getAttribute: (n: string) => r.attrs[n] ?? null, | |
| 200 | hasAttribute: (n: string) => n in r.attrs, | |
| 201 | setAttribute: (n: string, v: string) => void (r.attrs[n] = v), | |
| 202 | removeAttribute: (n: string) => void delete r.attrs[n], | |
| 203 | get attributes() { | |
| 204 | return Object.entries(r.attrs).map(([name, value]) => ({ name, value })); | |
| 205 | }, | |
| 206 | querySelector: () => null, | |
| 207 | querySelectorAll: () => [], | |
| 208 | contains: () => false, | |
| 209 | dispatchEvent: () => true, | |
| 210 | }; | |
| 211 | return r; | |
| 212 | }; | |
| 213 | ||
| 214 | /** Runs the script against a fake page; `answers` are the fragments the server gives, in order. */ | |
| 215 | const run = async (js: string, answers: Attrs[]) => { | |
| 216 | const mine = region( | |
| 217 | { id: "live-checks", "data-live": "run.*", "data-live-src": "/x/checks-line", "data-live-every": "5" }, | |
| 218 | "running", | |
| 219 | ); | |
| 220 | let n = 0; | |
| 221 | let fragment = region({ id: "live-checks" }, ""); | |
| 222 | const timers: Array<() => void> = []; | |
| 223 | const intervals = new Map<number, () => void>(); | |
| 224 | const cleared: number[] = []; | |
| 225 | let nextId = 1; | |
| 226 | const doc = { | |
| 227 | hidden: false, | |
| 228 | activeElement: null, | |
| 229 | querySelectorAll: (sel: string) => | |
| 230 | sel === "[data-live-every]" | |
| 231 | ? "data-live-every" in mine.attrs ? [mine] : [] | |
| 232 | : sel === "[data-live]" ? [mine] : [], | |
| 233 | getElementById: (id: string) => (id === "live-checks" ? mine : null), | |
| 234 | createElement: () => ({ | |
| 235 | set innerHTML(_: string) {}, | |
| 236 | content: { querySelector: () => null, getElementById: () => fragment }, | |
| 237 | }), | |
| 238 | dispatchEvent: () => true, | |
| 239 | addEventListener: () => {}, | |
| 240 | }; | |
| 241 | const win = { | |
| 242 | setInterval: (f: () => void) => (intervals.set(nextId, f), nextId++), | |
| 243 | clearInterval: (id: number) => (intervals.delete(id), void cleared.push(id)), | |
| 244 | addEventListener: () => {}, | |
| 245 | }; | |
| 246 | class ES { | |
| 247 | addEventListener() {} | |
| 248 | close() {} | |
| 249 | } | |
| 250 | const fake = { | |
| 251 | document: doc, | |
| 252 | window: win, | |
| 253 | location: { href: "http://x/page" }, | |
| 254 | EventSource: ES, | |
| 255 | CustomEvent: class {}, | |
| 256 | DOMParser: class {}, | |
| 257 | setTimeout: (f: () => void) => (timers.push(f), 0), | |
| 258 | clearTimeout: () => {}, | |
| 259 | fetch: async () => { | |
| 260 | fragment = region(answers[n++] ?? {}, "changed " + n); | |
| 261 | return { status: 200, ok: true, headers: { get: () => null }, text: async () => "" }; | |
| 262 | }, | |
| 263 | }; | |
| 264 | try { new Function(...Object.keys(fake), js)(...Object.values(fake)); } catch (e) { console.log("ERR", e); } | |
| 265 | const beat = async () => { | |
| 266 | for (const f of [...intervals.values()]) f(); | |
| 267 | while (timers.length > 0) timers.shift()!(); | |
| 268 | await new Promise((r) => setTimeout(r, 0)); | |
| 269 | }; | |
| 270 | return { mine, intervals, cleared, beat }; | |
| 271 | }; | |
| 272 | ||
| 273 | test("a swap without the attribute clears the beat and the region stops asking", async () => { | |
| 274 | const { liveRegionsJs } = await import("../src/web/client.ts"); | |
| 275 | const { mine, intervals, cleared, beat } = await run(liveRegionsJs, [{}]); | |
| 276 | expect(intervals.size).toBe(1); | |
| 277 | await beat(); | |
| 278 | expect(cleared.length).toBe(1); | |
| 279 | expect(intervals.size).toBe(0); | |
| 280 | expect(mine.hasAttribute("data-live-every")).toBe(false); | |
| 281 | }); | |
| 282 | ||
| 283 | test("a swap that brings the attribute arms one beat, and a repeat does not arm a second", async () => { | |
| 284 | const { liveRegionsJs } = await import("../src/web/client.ts"); | |
| 285 | const every = { "data-live-every": "5" }; | |
| 286 | const { intervals, cleared, beat } = await run(liveRegionsJs, [every, every]); | |
| 287 | await beat(); | |
| 288 | await beat(); | |
| 289 | expect(intervals.size).toBe(1); | |
| 290 | expect(cleared.length).toBe(0); | |
| 291 | }); | |
| 292 | }); | |
| 293 | ||
| 294 | 192 | describe("the live mount's markup", () => { |
| 295 | 193 | test("the status line is a polite region drawn with §5's .sr-only, and the pill keeps its words and link", async () => { |
| 296 | 194 | const { liveMount } = await import("../src/web/live.ts"); |
test/no-polling.test.ts +1 −1
| @@ -11,7 +11,7 @@ import { Glob } from "bun"; | ||
| 11 | 11 | const ALLOWED: Record<string, { allow: string[]; why: string }> = { |
| 12 | 12 | "src/web/sw.ts": { allow: ['http-equiv="refresh"'], why: "the restart page: the forge is down, so there is no /events to hear" }, |
| 13 | 13 | "src/web/pages/talk.ts": { allow: ["setInterval"], why: "server side: the stream of one turn watches the run's row, which has no event to hear" }, |
| 14 | "src/web/client.ts": { allow: ["data-live-every", "setInterval"], why: "the data-live-every mechanism itself, and clocks that only redraw text" }, | |
| 14 | "src/web/client.ts": { allow: ["setInterval"], why: "clocks that only redraw text" }, | |
| 15 | 15 | "src/web/space_client.ts": { allow: ["setInterval"], why: "the map clock label only redraws text; it fetches and reloads nothing" }, |
| 16 | 16 | }; |
| 17 | 17 |
test/pull-checks-line-route.test.ts +3 −3
| @@ -137,9 +137,9 @@ test("the page that draws the line also ships the script that drives it", async | ||
| 137 | 137 | expect(text).toContain('id="live-checks"'); |
| 138 | 138 | // The line itself carries no beat attribute; the script still knows the mechanism. |
| 139 | 139 | expect(text).not.toMatch(/id="live-checks"[^>]*data-live-every/); |
| 140 | // liveRegionsJs reads data-live-src and arms the beat; liveClientJs alone does not. | |
| 140 | // liveRegionsJs reads data-live-src and swaps the region; liveClientJs alone does not. | |
| 141 | 141 | expect(text).toContain("data-live-src"); |
| 142 | expect(await withScripts(servers.url, text)).toContain("function arm("); | |
| 142 | expect(await withScripts(servers.url, text)).toContain("data-live-src"); | |
| 143 | 143 | }); |
| 144 | 144 | |
| 145 | 145 | test("once the check has passed, the line has no beat, no counts, and reads as passed", async () => { |
| @@ -165,5 +165,5 @@ test("the change page the owner lands on draws the same live line, with the coun | ||
| 165 | 165 | expect(text).toContain('id="live-checks"'); |
| 166 | 166 | expect(text).not.toMatch(/id="live-checks"[^>]*data-live-every/); |
| 167 | 167 | expect(text).toContain("2 of 170 files · 500 passed, 0 failed"); |
| 168 | expect(await withScripts(servers.url, text)).toContain("function arm("); | |
| 168 | expect(await withScripts(servers.url, text)).toContain("data-live-src"); | |
| 169 | 169 | }); |