Type the render client: no implicit any, no implicit this#177
Merged
Conversation
Enables noImplicitAny, noImplicitThis, strictFunctionTypes, strictBindCallApply, noImplicitReturns, noFallthroughCasesInSwitch, and useUnknownInCatchVariables on js/tsconfig.json, and annotates the client to satisfy them: 2321 diagnostics -> 0. Every parameter, local, and `this` in the client is now typed; no value acquires `any` implicitly. New js/src/05_types.ts carries the wire contract the kernel emits (ChartSpec, TraceSpec, AxisSpec, ChannelSpec, ColumnMeta, GpuTrace, Hit, Row, Comm, TickResult, Theme, MarkKind, Rgba). It is deliberately permissive where the wire is permissive: open index signatures stay, because tightening one is a compatibility decision to make alongside the Python side that emits it. Per-module types cover shapes with no shared name (LOD drill/density updates, animation easings, view-state patches, interaction option bags, GL program and buffer tags). The prototype-augmentation modules (51-57) get `as ThisType<ChartView>` on their Object.assign literals, so `this` is a ChartView inside those methods instead of the literal — that alone resolved 1190 of the diagnostics and, being a type assertion, erases at compile. Types-only by construction and by proof: the committed minified bundles in python/xy/static are byte-identical before and after, so the shipped client is unchanged. The render smoke reports XY_OK with pixel counts and context-loss hashes identical to the pre-typing run. Source-marker tests that grep exact client signatures are updated to the typed forms, preserving their parameter-contract intent. tests/reflex_adapter/ test_assets.py additionally moves off unminified markers - it only runs when reflex is installed, so it was missed when the bundles began shipping minified. Deferred, documented in js/tsconfig.json with the reasons and order: removing ChartView's [key: string]: any index signature (needs ~403 declared members) and strictNullChecks, then strict: true.
Merging this PR will not alter performance
Comparing Footnotes
|
Greptile SummaryThis PR adds full implicit-type checking to the TypeScript render client. The main changes are:
Confidence Score: 5/5This looks safe to merge. No blocking issues found in the changed code.
What T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "Type the render client: no implicit any,..." | Re-trigger Greptile |
This was referenced Jul 22, 2026
Member
Author
|
Docs verification passed on
The PR changes only the typed render client; the documentation site remains healthy. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #159 (base:
feat/ts-vite-client). Merge that first.What
Turns the TypeScript conversion from "compiles" into "typechecked". 2321 diagnostics → 0 under the newly enabled flags:
Every parameter, local, and
thisin the client is now typed — no value acquiresanyimplicitly.tscalready gates every build (js/build.mjsruns it first, since oxc strips types without checking them), so this is enforced on every PR.How
js/src/05_types.ts(new) carries the wire contract the kernel emits:ChartSpec,TraceSpec,AxisSpec,ChannelSpec,ColumnMeta,GpuTrace,Hit,Row,Comm,TickResult,Theme,MarkKind,Rgba. Deliberately permissive where the wire is permissive — open index signatures stay, because tightening one is a compatibility decision to make alongside the Python that emits it.as ThisType<ChartView>on the prototype-augmentation literals (51–57) typesthisas ChartView instead of the object literal. That one type assertion resolved 1190 of the 2321 diagnostics, and erases at compile.XyProgram,XyBuffer),ColumnView,AxisMap._listenbecame generic over its event type so call sites keep precise handlers (PointerEvent,WheelEvent, …) understrictFunctionTypes.Types-only — proven, not asserted
The committed minified bundles in
python/xy/static/are byte-identical before and after this change; the diff contains no bundle. Independently, three of the workers transpiled HEAD vs. working copies and byte-compared the emitted JS per file.That check earned its keep twice: it caught a
&&→?.rewrite that had slipped into a catch-narrowing fix, and a case where an interface placed above the first runtime statement absorbed a file's header comment and changed emit.Verification:
tscclean · bundles byte-identical · full pytest 2208 passed · render smokeXY_OKwith pixel counts and context-loss hashes identical to the pre-typing run · ruff + pre-commit clean.Test fallout
Source-marker tests that grep exact client signatures are updated to the typed forms, preserving their parameter-contract intent. Separately,
tests/reflex_adapter/test_assets.pystill grepped unminified markers (function renderStandalone() — it only runs when reflex is installed, so it was missed when the bundles began shipping minified in #159. Fixed here to use the same export-alias contractverify_wheel.pyasserts.Latent bugs surfaced (reported, not fixed — types-only)
Typing the code found real issues. None are touched here; each deserves its own change:
lodApplyDensityUpdatereadsg.density.colormap/.color/.lutunguarded, while every sibling access guards — a density update on a trace whose density was never built is an uncaught TypeError.RECT_MARK/BAR_MARK/SEGMENT_MARK/AREA_MARK/linedereferenceg.trace.style.colorunguarded;styleis optional on the wire (MESH_MARKand hexbin do guard).54_kernelbuildspendingTraceIdswithNumber(upd.id)but tests membership with the rawg.trace.id;TraceSpec.idisstring | number, so a string id silently skips theclearPendingsweep._onA11yKeycomputeshit.trace * 1e9 + hit.indexfrom a trace id; a string id makes_hoverIdNaN and every!== -1guard read true forever.51_annotationsliftBounds.widthis only safe via a two-expression short-circuit that a future edit could split.57_viewstatecanBandPan()returns truthy via_axisContainedeven when pan policy excludes the axis — possibly deliberate, worth a look.Items 1, 2, and 5 are exactly the class
strictNullCheckswould catch, which motivates the order below.Deferred (documented in
js/tsconfig.json)ChartView's[key: string]: any. It is what still letsthis.typocompile and what stops the annotated signatures in 51–57 from being enforced at call sites. Needs ~403 declared members (172 class methods, 119 augmentation methods viadeclare modulemerging, 112 instance fields) — mechanical, but its own reviewable PR.strictNullChecks— largest remaining win, largest diff.strict: trueonce 1 and 2 land.