perf(core)!: measure in one layout pass, drop the shape alias - #56
Merged
Conversation
Times buildMarkGeometry, clipAtFront, buildEdge, and buildNoiseTile against the built core so hot-path changes can be measured rather than guessed at. Runs with `pnpm bench` after `pnpm build`.
Drops the running commentary across core, the framework wrappers, and the site, leaving comments only where they explain a why the code cannot. Alongside it, the core render and measure paths get leaner: - highlight() no longer computes an anchor before measuring, so each range is measured in one layout pass instead of two. rangesToLineRects takes the anchor as optional and falls back to the caller's columnBounds. - Style writes route through a per-element cache, so a re-render that resolves to the same declaration skips the DOM write. - smoothEdge walks edge vertices directly instead of materialising two intermediate arrays per frame, and buildMarkGeometry shifts edges into mark space in place. - typeof guards collapse into a single hasGlobal() helper.
JaceThings
force-pushed
the
perf/lean-hot-paths
branch
from
August 23, 2026 17:26
cb96a6e to
643fb25
Compare
BREAKING CHANGE: `shape` and `ShapeType` are gone. Both were undocumented aliases for `markType` and `MarkType`, and `mergeOptions` already collapsed `shape` into `markType` and deleted it, so the alias only ever gave one concept a second name. Pass `markType`; import `MarkType`. The rest is the same idea applied inward: values get parsed once where they enter, instead of being asserted into shape at each use. Core no longer indexes `el.style` through a `CSSStyleDeclaration & Record<string, string>` cast; it calls `setProperty` with real CSS property names. That also fixes a latent lie, since a camelCase write only ever set an expando in jsdom and never a real declaration. `mergeRectsByLine` returns real `DOMRect` instances rather than object literals wearing the type. TreeWalker results go through one `nextTextNode` helper instead of `as Text` at every call site, and the remaining `typeof x !== "undefined"` probes collapse into `hasGlobal`. The website gains a single `browser-env` module that reads the environment once into a named capability record, so components branch on a domain value instead of probing globals inline. The playground's option paths are now a real union derived from `PlaygroundOptions`, with a value type per path, which removes the `unknown` plumbing that ran from `set()` down through the option demos. The consumer smoke scripts share one export-contract parser that validates the packed surface in a single place, and the react hook's test injects a runtime through the existing provider seam instead of mocking the core module. `findSelectionAnchor` now skips a positioned ancestor that is not an `HTMLElement` instead of returning it as one. Only reachable through a positioned SVG ancestor, where the old result could not host an overlay anyway.
Owner
Author
|
Pushed |
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.
highlight()was measuring every range twice: once throughcomputeAnchorto find the column, then again for the line rects. The anchor was only ever a fallback forcolumnBounds, which the caller already passes whenever there's an anchor host, so the second layout pass bought nothing in the common path.Measurement is now a single pass per range,
rangesToLineRectstakes the anchor as optional, andcomputeAnchorstays exported for callers measuring a column themselves. Alongside it: style writes go through a per-element cache so a re-render resolving to the same declaration skips the DOM write,smoothEdgewalks edge vertices directly instead of materialising two intermediate arrays per frame, andbuildMarkGeometryshifts edges into mark space in place rather than mapping copies. Thetypeof x !== "undefined"guards scattered across targeting and the tiers collapse into onehasGlobal().Breaking:
shapeandShapeTypeare goneShapeTypewasexport type ShapeType = MarkType, andshape?was an undocumented alias formarkType?thatmergeOptionsalready collapsed intomarkTypeand then deleted. It only ever gave one concept a second name. Callers passmarkTypeand importMarkType. Nothing else in the public surface moved, and neither name appears anywhere in the README or docs. The changeset is a major, so thelinkedgroup takes all four packages to 2.0.0.The rest: parse at the boundary, don't assert at the use
Same idea applied inward, across core, the wrappers, the site and the tests.
Core stopped indexing
el.stylethrough aCSSStyleDeclaration & Record<string, string>cast and callssetPropertywith real CSS property names. That turned out to fix a latent lie rather than just a type: a camelCaseel.style.maskPosition = vwrite only creates an expando in jsdom and never a real declaration, so four test assertions moved togetPropertyValueand check something real now.mergeRectsByLinereturns realDOMRectinstances instead of object literals wearing the type. Everywalker.nextNode() as Textgoes through onenextTextNodehelper.The website gained a single
browser-envmodule that reads the environment once into a named capability record, so components branch on a domain value instead of probing globals inline. The playground's option paths are a real union derived fromPlaygroundOptionswith a value type per path, which removed theunknownthat ran fromset()all the way down through the option demos. The consumer smoke scripts share one export-contract parser instead of scatteringtypeof x === "function"across four files, and the react hook's test injects a runtime through the provider seam instead of mocking the core module.Two behaviour changes worth naming.
findSelectionAnchornow skips a positioned ancestor that isn't anHTMLElementinstead of returning it as one; that's only reachable through a positioned SVG ancestor, where the old result couldn't host an overlay anyway. And the twomergeOptionstests that covered theshapealias were replaced with ones covering what the merge actually does now: the override wins, and an explicitundefinedkeeps the base.Where to start
packages/core/src/render/highlight.tsandtargeting/line-rects.tscarry the measure change and everything else follows from it.render/renderer.tshas the style cache and thesetPropertyswitch,geometry/clip-path.tsthe edge walk, andapps/website/src/playground/options-context.tsxthe typed option paths. The comment strip in643fb25is mechanical and skimmable; the code inside it is not.Verified
pnpm typecheck,pnpm test(241 pass),pnpm build,pnpm knip,pnpm sizeandpnpm consumer-smokeall green locally. Core sits at 14.8 kB brotlied against its 15 kB budget.The type work was driven by a linter run locally and removed afterwards, so nothing new is vendored or added to the toolchain. It went from 278 findings to 3, all three being Vue's
String as PropType<…>/Object as SlotsType<…>prop declarations, which are the documented way to declare Vue runtime props and can't be written without the assertion.scripts/bench.mjs(pnpm bench) is new so the geometry hot paths can be timed instead of guessed at. It needspnpm buildfirst.