From bab316b58a966cbbb66d7c8f0b36807c1c33e935 Mon Sep 17 00:00:00 2001 From: Alek Petuskey Date: Tue, 21 Jul 2026 20:08:47 -0700 Subject: [PATCH 1/2] Convert the render client to TypeScript with a vite-minified build The client source js/src/* moves from concat-ordered plain JS parts to TypeScript ES modules: explicit imports/exports replace filename-order concatenation, the prototype-augmentation parts (51-57) import ChartView directly, and bytesToSpan moves 60_entries -> 00_header to keep the module graph acyclic. Typing starts pragmatic (js/tsconfig.json documents the migration posture); tsc --noEmit now gates every build. js/build.mjs keeps its CLI (node js/build.mjs / --check) but now runs tsc -> the shader-convention lint -> two vite (rolldown/oxc) builds, and the shipped artifacts are minified: static/index.js (anywidget ESM, same export surface) and static/standalone.js (IIFE; top-level var xy == window.xy). Bundle drops 389 KB -> 283 KB (~76 KB gzipped). --check byte-compares a scratch rebuild, verified byte-identical across macOS-arm64, linux-arm64, and linux-x86_64. Tests that grepped exact source lines out of the built bundles now assert them against the TS sources only; bundles are asserted via minification-surviving markers (string literals, .innerHTML counts, export aliases), with the --check freshness gate carrying source-level invariants into the shipped artifacts. CI/release jobs that build the client gain npm ci; the sdist ships js/ plus the npm manifests; CLAUDE.md and the spec amend the no-npm stance (SS33) to dev-time-only toolchain dependencies. Verified: full pytest suite (2117 passed), render_smoke_nonumpy probe matrix green against the minified standalone bundle, ruff/pre-commit clean. --- .gitattributes | 1 + .github/workflows/_build-docs-images.yml | 2 + .github/workflows/ci.yml | 11 + .github/workflows/release.yml | 6 + CLAUDE.md | 16 +- js/build.mjs | 306 +- js/src/{00_header.js => 00_header.ts} | 16 +- js/src/{10_colormaps.js => 10_colormaps.ts} | 4 +- js/src/{20_theme.js => 20_theme.ts} | 14 +- js/src/{30_ticks.js => 30_ticks.ts} | 20 +- js/src/{40_gl.js => 40_gl.ts} | 48 +- js/src/{45_lod.js => 45_lod.ts} | 18 +- js/src/{46_worker.js => 46_worker.ts} | 4 +- js/src/{50_chartview.js => 50_chartview.ts} | 56 +- .../{51_annotations.js => 51_annotations.ts} | 3 + js/src/{52_tooltip.js => 52_tooltip.ts} | 9 +- .../{53_interaction.js => 53_interaction.ts} | 40 +- js/src/{54_kernel.js => 54_kernel.ts} | 11 +- js/src/{55_marks.js => 55_marks.ts} | 6 +- js/src/{56_animation.js => 56_animation.ts} | 5 +- js/src/{57_viewstate.js => 57_viewstate.ts} | 14 +- js/src/{60_entries.js => 60_entries.ts} | 35 +- js/tsconfig.json | 23 + package-lock.json | 1266 ++- package.json | 11 +- pyproject.toml | 7 +- python/xy/_arrowgeom.py | 2 +- python/xy/_svg.py | 8 +- python/xy/static/index.js | 9737 +--------------- python/xy/static/standalone.js | 9738 +---------------- scripts/verify_sdist.py | 38 +- scripts/verify_wheel.py | 6 +- spec/api/chart-kind-contract.md | 12 +- spec/api/chart-roadmap.md | 2 +- spec/api/export.md | 6 +- spec/api/interaction.md | 38 +- spec/api/styling.md | 4 +- spec/design-dossier.md | 28 +- spec/design/chart-grammar.md | 8 +- spec/design/lod-architecture.md | 8 +- spec/design/reflex-integration.md | 2 +- spec/design/renderer-architecture.md | 90 +- spec/design/view-state.md | 8 +- spec/design/wire-protocol.md | 4 +- .../pyplot/test_annotation_label_clearance.py | 2 +- tests/pyplot/test_color_pipeline_fixes.py | 8 +- tests/test_accessibility_contract.py | 29 +- tests/test_arrowgeom.py | 2 +- tests/test_benchmark_environment.py | 4 +- tests/test_example_apps.py | 2 +- tests/test_framing.py | 6 +- tests/test_static_client_security.py | 226 +- tests/test_svg_export.py | 8 +- tests/test_verify_sdist.py | 32 +- tests/test_verify_wheel.py | 20 +- tests/test_viewport_bounds.py | 2 +- 56 files changed, 2056 insertions(+), 19976 deletions(-) rename js/src/{00_header.js => 00_header.ts} (93%) rename js/src/{10_colormaps.js => 10_colormaps.ts} (98%) rename js/src/{20_theme.js => 20_theme.ts} (97%) rename js/src/{30_ticks.js => 30_ticks.ts} (94%) rename js/src/{40_gl.js => 40_gl.ts} (97%) rename js/src/{45_lod.js => 45_lod.ts} (98%) rename js/src/{46_worker.js => 46_worker.ts} (95%) rename js/src/{50_chartview.js => 50_chartview.ts} (99%) rename js/src/{51_annotations.js => 51_annotations.ts} (99%) rename js/src/{52_tooltip.js => 52_tooltip.ts} (97%) rename js/src/{53_interaction.js => 53_interaction.ts} (98%) rename js/src/{54_kernel.js => 54_kernel.ts} (98%) rename js/src/{55_marks.js => 55_marks.ts} (98%) rename js/src/{56_animation.js => 56_animation.ts} (99%) rename js/src/{57_viewstate.js => 57_viewstate.ts} (98%) rename js/src/{60_entries.js => 60_entries.ts} (72%) create mode 100644 js/tsconfig.json diff --git a/.gitattributes b/.gitattributes index bd18720e..da52d986 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,3 +3,4 @@ # Windows checkouts with autocrlf. *.js text eol=lf *.mjs text eol=lf +*.ts text eol=lf diff --git a/.github/workflows/_build-docs-images.yml b/.github/workflows/_build-docs-images.yml index 8cb017ef..579df74a 100644 --- a/.github/workflows/_build-docs-images.yml +++ b/.github/workflows/_build-docs-images.yml @@ -73,6 +73,8 @@ jobs: with: toolchain: 1.97.0 + - name: Install JS build toolchain + run: npm ci - name: Verify committed render client run: node js/build.mjs --check diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1bba6fb7..c4eefa83 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,8 @@ jobs: with: node-version: "22" + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check @@ -334,6 +336,9 @@ jobs: if: matrix.browser || matrix.build_js with: node-version: "22" + - name: Install JS build toolchain + if: matrix.build_js + run: npm ci - name: Build native core if: matrix.xy run: cargo build --release @@ -402,6 +407,8 @@ jobs: - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: "22" + - name: Install JS build toolchain + run: npm ci - name: Build core + JS client run: | node js/build.mjs @@ -548,6 +555,8 @@ jobs: with: node-version: "22" + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check @@ -606,6 +615,8 @@ jobs: with: node-version: "22" + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b0ef619d..a802593d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,6 +68,8 @@ jobs: - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: "22" + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check - name: Build JS client @@ -145,6 +147,8 @@ jobs: with: # Pyodide 0.29.x / pyodide_2025_0 ABI compiler version. version: "4.0.9" + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check - name: Build JS client @@ -199,6 +203,8 @@ jobs: # user's machine if they install from sdist and have a toolchain. There is # no fallback: without a toolchain the compute layer raises a clear # ImportError on first use (hatch_build.py). Wheels are the normal path. + - name: Install JS build toolchain + run: npm ci - name: Verify committed JS bundles are fresh run: node js/build.mjs --check - name: Build sdist diff --git a/CLAUDE.md b/CLAUDE.md index ccd66108..b6a759a0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,10 +48,15 @@ instead of treating the implementation alone as authoritative. package at app compile (no second copy to drift), and the adapter stays out of the root `xy` sdist (`scripts/verify_sdist.py` enforces it). Tests: `tests/reflex_adapter/` (skip unless reflex installed). -- `js/src/*.js` — the render client as ordered parts (concat order in - `js/build.mjs`; exports live only in `60_entries.js`), one dependency-free ES - module. **No npm packages.** `node js/build.mjs` copies it to - `python/xy/static/` (committed artifacts). +- `js/src/*.ts` — the render client as TypeScript ES modules (one module per + former concat part; `60_entries.ts` is the entry and the only public export + surface). `node js/build.mjs` typechecks (`js/tsconfig.json`), lints the + shaders, and has vite bundle + minify into `python/xy/static/index.js` + (anywidget ESM) and `standalone.js` (IIFE, `window.xy`) — committed, + minified artifacts; the minified bundles are what ships to the client. + npm devDependencies (vite/typescript/playwright, pinned in + `package-lock.json`) are build/test-time only — the shipped client stays + runtime-dependency-free. Run `npm ci` once per checkout. - `tests/`, `scripts/bench.py` (§12 harness), `scripts/smoke_render.py` (headless Chromium pixel probe). @@ -59,7 +64,8 @@ instead of treating the implementation alone as authoritative. ```bash cargo test && cargo build --release # core -node js/build.mjs # regenerate static/ after JS edits +npm ci # once per checkout: vite + tsc toolchain +node js/build.mjs # typecheck + regenerate minified static/ after JS edits python3 scripts/abi_smoke.py # C-ABI seam, stdlib only (no PyPI needed) python3 scripts/render_smoke_nonumpy.py # WebGL2 render path in headless Chromium uv venv && uv pip install -e ".[dev]" diff --git a/js/build.mjs b/js/build.mjs index a62257b6..cd6fc7b6 100644 --- a/js/build.mjs +++ b/js/build.mjs @@ -1,41 +1,37 @@ #!/usr/bin/env node -// Dependency-free "bundler": the client is hand-written ES-module source split -// into ordered parts under src/ (plain script content; exports live only in the -// final part). Building = concatenating them (anywidget ESM) and wrapping the -// export-free body (standalone IIFE for static HTML export). No npm, no -// registry, no supply chain — deliberate (§33). -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +// Build the render client. The source is TypeScript ES modules under src/ +// (one module per former concat part; import order replaces concat order). +// Vite (rolldown + oxc) bundles and minifies them into the two committed +// artifacts in python/xy/static/ — the minified bundles are what ships to the +// client (§33 amended: vite/typescript are the only, dev-time-only, npm deps): +// index.js — ESM bundle (anywidget `_esm`; named exports + default) +// standalone.js — IIFE bundle exposing `window.xy`, inlined by +// `Figure.to_html()` into static HTML exports +// Steps: tsc typecheck → shader-convention lint → vite build (×2 formats). +// `--check` rebuilds into a scratch dir and byte-compares against the +// committed artifacts (the CI freshness gate); it never touches static/. +import { mkdtempSync, readFileSync, mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; - -// Concatenation order is load-bearing (const/class definitions before use). -const PARTS = [ - "00_header.js", // doc header, "use strict", PROTOCOL version - "10_colormaps.js", // colormap stops + LUT builder (§36 CVD-safe defaults) - "20_theme.js", // CSS color resolution + --chart-* tokens (§36) - "30_ticks.js", // f64 tick/format math — never through f32 (§16) - "40_gl.js", // GL helpers + all shaders (marks, pick, density, line) - "45_lod.js", // chart-agnostic tier LOD: drill lifecycle, fades, cache (§5/§28) - "46_worker.js", // standalone density re-bin worker (blob URL, off-main-thread) - "50_chartview.js", // ChartView core: layout, GL, marks, draw, chrome, pick - "51_annotations.js", // + annotation canvas overlay (prototype augmentation) - "52_tooltip.js", // + hover->row tooltip resolution + DOM - "53_interaction.js", //+ pointer/drag/wheel, selection, modebar, view anim - "54_kernel.js", // + kernel comm: view-requests, append, drill (§16) - "55_marks.js", // MARK_KINDS: per-chart-kind build/draw dispatch registry - "56_animation.js", // declarative entrance/data transition controller - "57_viewstate.js", // + durable view state, history, axis bands, hover payload - "60_entries.js", // anywidget + standalone entry points, export tail -]; +import { spawnSync } from "node:child_process"; const here = dirname(fileURLToPath(import.meta.url)); +const root = join(here, ".."); const checkOnly = process.argv.includes("--check"); -// Normalize CRLF: a Windows checkout with autocrlf otherwise mixes CRLF part -// content with the wrapper's literal \n, making the freshness check fail on -// exactly one platform. Build output is LF everywhere, deterministically. -const readText = (p) => readFileSync(p, "utf8").replace(/\r\n/g, "\n"); -const src = PARTS.map((p) => readText(join(here, "src", p))).join(""); -const outDir = join(here, "..", "python", "xy", "static"); +const staticDir = join(root, "python", "xy", "static"); + +// Typecheck first: a bundle must never be built from source tsc rejects +// (esbuild strips types without checking them, so this is the only gate). +{ + const tsc = join(root, "node_modules", ".bin", "tsc"); + const res = spawnSync(tsc, ["-p", join(here, "tsconfig.json")], { stdio: "inherit" }); + if (res.error) { + console.error(`tsc failed to start (${res.error.message}); run \`npm install\` first`); + process.exit(1); + } + if (res.status !== 0) process.exit(res.status ?? 1); +} // Shader convention lint (renderer audit R5). The conventions are load-bearing: // a non-highp fragment default already caused one precision-mismatch hunt @@ -46,7 +42,7 @@ const outDir = join(here, "..", "python", "xy", "static"); // GRID_VS is the fullscreen-quad vertex stage shared by density + heatmap: // it maps via u_view (data-space window), not per-vertex u_*map attributes. const VIEWMAP_EXEMPT = new Set(["GRID_VS"]); - const glSrc = readText(join(here, "src", "40_gl.js")); + const glSrc = readFileSync(join(here, "src", "40_gl.ts"), "utf8"); const errs = []; let shaders = 0; for (const m of glSrc.matchAll(/const (\w+_(?:VS|FS)) = `([^`]*)`/g)) { @@ -79,188 +75,78 @@ const outDir = join(here, "..", "python", "xy", "static"); } } -// Compact the shipped bundles: strip comments, leading indentation, and blank -// lines — nothing else. Not a renaming minifier; every code token survives -// verbatim (the client-security test greps exact source lines in the built -// bundles). String/template/regex-literal content is untouched: GLSL shaders -// live in template literals and keep their own comments. Line structure is -// preserved (a removed multi-line block comment leaves one newline), so ASI -// behavior cannot change. Payoff: every `Figure.to_html()` inlines the -// standalone bundle, and the anywidget ESM parses on first chart paint — the -// small-data TTFR path (audit small-data #3). -function compact(source) { - const n = source.length; - let out = ""; - let i = 0; - // Frame stack tracks template-literal nesting: `${` pushes a code frame, - // its matching `}` pops back into the template. `brace` counts plain - // braces inside a code frame so object literals don't pop early. - const frames = [{ mode: "code", brace: 0 }]; - let atLineStart = true; - const emit = (ch) => { - out += ch; - atLineStart = ch === "\n"; - }; - while (i < n) { - const top = frames[frames.length - 1]; - const c = source[i]; - const c2 = source[i + 1]; - if (top.mode === "template") { - if (c === "\\") { - emit(c); - if (i + 1 < n) emit(source[i + 1]); - i += 2; - continue; - } - if (c === "`") { - emit(c); - frames.pop(); - i++; - continue; - } - if (c === "$" && c2 === "{") { - emit(c); - emit(c2); - frames.push({ mode: "code", brace: 0 }); - i += 2; - continue; - } - emit(c); - i++; - continue; - } - // mode === "code" - if (atLineStart && (c === " " || c === "\t")) { - i++; // strip indentation - continue; - } - if (c === "\n") { - // Removing a trailing line comment can leave the separating space that - // preceded it. Do not ship that whitespace into every standalone HTML - // artifact (and keep generated files clean under `git diff --check`). - while (out.endsWith(" ") || out.endsWith("\t")) out = out.slice(0, -1); - if (!atLineStart) emit("\n"); // collapse blank lines - i++; - continue; - } - if (c === "/" && c2 === "/") { - while (i < n && source[i] !== "\n") i++; - continue; // the newline itself is handled above - } - if (c === "/" && c2 === "*") { - const end = source.indexOf("*/", i + 2); - const closed = end >= 0 ? end + 2 : n; - const removed = source.slice(i, closed); - // Keep separation so `a/* */b` can't fuse tokens; keep a newline if the - // comment spanned lines so the following line still starts a line. - emit(removed.includes("\n") ? "\n" : " "); - i = closed; - continue; - } - if (c === "'" || c === '"') { - emit(c); - i++; - while (i < n) { - const s = source[i]; - emit(s); - i++; - if (s === "\\") { - if (i < n) { - emit(source[i]); - i++; - } - continue; - } - if (s === c) break; - } - continue; - } - if (c === "`") { - emit(c); - frames.push({ mode: "template" }); - i++; - continue; - } - if (c === "{") { - top.brace++; - emit(c); - i++; - continue; - } - if (c === "}") { - if (top.brace > 0) { - top.brace--; - emit(c); - } else if (frames.length > 1) { - frames.pop(); // close of a ${...} interpolation - emit(c); - } else { - emit(c); - } - i++; - continue; - } - emit(c); - i++; - } - return out; -} +const { build } = await import("vite"); -// standalone build: strip the export tail, expose a window global. -const marker = "// ---- exports ----"; -const cut = src.indexOf(marker); -if (cut < 0) throw new Error("export marker not found in 60_entries.js"); -// The marker must own its whole line: exportTail is emitted raw into index.js, -// so any text trailing the marker on its line would land as bare (invalid) code -// in the ESM bundle. Split at the line boundary and reject a non-empty tail. -const markerLineEnd = src.indexOf("\n", cut); -const trailing = src.slice(cut + marker.length, markerLineEnd < 0 ? undefined : markerLineEnd); -if (trailing.trim()) { - throw new Error(`text after "${marker}" would leak into index.js: "${trailing.trim()}"`); +/** Build both bundles into outDir. One entry, two formats: the ESM build keeps + * 60_entries' export shape verbatim; the IIFE build assigns the same namespace + * to a top-level `var xy`, which is `window.xy` when the bundle runs as the + * classic inline