refactor(painter): remove legacy createDomPainter wrapper API#2853
refactor(painter): remove legacy createDomPainter wrapper API#2853tupizz wants to merge 1 commit intotadeu/sd-2563-13-testing-cleanupfrom
Conversation
The painter previously shipped a backward-compat API layer (setData,
setResolvedLayout, paint(Layout) overload, blocks/measures constructor
options, LegacyDomPainterState, buildLegacyPaintInput). This mixed-path
logic existed only to smooth the migration to DomPainterInput.
All production callers now use the modern contract directly. The
benchmark is updated to call resolveLayout explicitly. Test call sites
that still expressed fixtures as {blocks, measures} + raw Layout now
route through a shared createTestPainter helper in
painters/dom/src/test-utils/test-painter.ts, which wraps createDomPainter
and runs resolveLayout internally.
DomPainterOptions: blocks?/measures? removed
DomPainterHandle: setData/setResolvedLayout removed, paint() takes only DomPainterInput
Internal: LegacyDomPainterState, buildLegacyPaintInput, createEmptyResolvedLayout,
isDomPainterInput, assertRequiredBlockMeasurePair all deleted.
Closes SD-2563 criterion: painter does not carry mixed-path logic.
No rendering change (layout-compare 0/407 changed).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0277744abc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping) { | ||
| painter.paint(input, mount, mapping); |
There was a problem hiding this comment.
This change forwards paint() input straight into DomPainter.paint without normalizing raw Layout inputs, but there are still in-repo legacy call sites that pass Layout directly (for example packages/layout-engine/pm-adapter/src/integration.test.ts still does createDomPainter({ blocks, measures }) + paint(layout, mount)). In that scenario DomPainter.paint reads input.sourceLayout, so a raw Layout makes layout undefined and throws at runtime; please migrate remaining callers in the same commit (or add an explicit runtime guard/error) to avoid this break.
Useful? React with 👍 / 👎.
1 participant
Summary
Closes SD-2563's remaining criterion: "Painter does not carry fallback or mixed-path logic that belongs in other stages."
The painter shipped a backward-compat API layer that existed only to smooth the migration to
DomPainterInput. All known consumers now use the modern contract directly.What's deleted from
createDomPainterwrapperblocks?andmeasures?fields onDomPainterOptionssetData(blocks, measures)method onDomPainterHandlesetResolvedLayout(resolvedLayout)method onDomPainterHandlepaint(input: DomPainterInput | Layout)union overload →paint(input: DomPainterInput)onlyLegacyDomPainterState,buildLegacyPaintInput,createEmptyResolvedLayout,isDomPainterInput,assertRequiredBlockMeasurePairNet: -101 lines in
index.ts.What changed
index.ts— deleted all legacy paths. Painter wrapper is now a thin pass-through.layout-bridge/benchmarks/index.ts— migrated to the modern API. CallsresolveLayoutexplicitly, passes{resolvedLayout, sourceLayout}topaint().createTestPainterintopackages/layout-engine/painters/dom/src/test-utils/test-painter.tsand switched 13 painter-dom test files to use it. Previously each file had inline or duplicated versions.Testing rationale (applied /testing-excellence)
The previous tests violated Iron Law #2 ("never add methods to production only because a test needs it"):
setData,setResolvedLayout, and thepaint(Layout)overload existed in production largely to serve test ergonomics. Moving the legacy-shape translation into a shared test helper puts the responsibility in the test layer where it belongs — production code now exposes only its actual contract.Verification
painter-dombuild + 952/952 testslayout-resolved103/103 testslayout-bridge1162/1162 tests (benchmark compiles + runs)super-editorbuild + 11350/11361 testspnpm layout:compare --input-root test-corpus→ 0 changed / 407 unchanged (byte-identical rendering)PR Stack (SD-2563: Dumb Painter Refactor)