[2/3] refactor(painter): collapse legacy API surface (SD-2836)#3117
Open
tupizz wants to merge 4 commits intotadeu/sd-2836-1-consume-resolved-layoutfrom
Open
[2/3] refactor(painter): collapse legacy API surface (SD-2836)#3117tupizz wants to merge 4 commits intotadeu/sd-2836-1-consume-resolved-layoutfrom
tupizz wants to merge 4 commits intotadeu/sd-2836-1-consume-resolved-layoutfrom
Conversation
3 tasks
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
8 tasks
tupizz
force-pushed
the
tadeu/sd-2836-1-consume-resolved-layout
branch
from
33e0471 to
c5d0b09
Compare
tupizz
force-pushed
the
tadeu/sd-2836-2-collapse-legacy-api
branch
2 times, most recently
from
befe9c0 to
8f18041
Compare
Make ResolvedLayout the only painter input contract:
* DomPainterInput collapses to `{ resolvedLayout: ResolvedLayout }`.
sourceLayout, blocks/measures, headerBlocks/Measures,
footerBlocks/Measures all removed.
* DomPainterOptions: drop blocks/measures.
* DomPainterHandle: drop setData, setResolvedLayout. paint takes only
DomPainterInput. Drops the `paint(Layout)` overload across painter,
PresentationPainterAdapter, and (transitively) PresentationEditor's
paintInput.
* createDomPainter shrinks to a thin pass-through. Removes
buildLegacyPaintInput, normalizeDomPainterInput, isDomPainterInput,
wrapProvider, resolveDecorationItems, normalizeOptionalBlockMeasurePair,
assertRequiredBlockMeasurePair, createEmptyResolvedLayout,
LegacyDomPainterState, OptionalBlockMeasurePair.
* PageDecorationPayload.items becomes required (the synthesis path
is gone).
Tests:
* New `_test-utils.ts` exposes a legacy-shaped `createTestPainter`
that test files import in place of `createDomPainter`. The helper
builds a real DomPainterInput internally and synthesizes decoration
items so existing test bodies don't have to be rewritten.
* All 17 painter test files migrated to the helper.
* Two source-anchor tests still failing under investigation;
remaining work is bounded and tracked.
createTestPainter was overwriting the user-supplied onPaintSnapshot callback with its own, so tests that relied on the callback (source-anchor tests) saw an undefined snapshot. Chain the user callback after the internal one.
The three integration tests in pm-adapter were calling painter.paint(layout, mount) with raw Layout. They now resolveLayout() first and call painter.paint({ resolvedLayout }, mount). All 1794 pm-adapter tests pass.
The layout-bridge incremental-pipeline performance benchmark called painter.paint(layout, mount) and painter.setData(...). Both are removed by the API collapse. Migrate to resolveLayout() + DomPainterInput so the benchmark continues to exercise the painter under the new contract.
tupizz
force-pushed
the
tadeu/sd-2836-2-collapse-legacy-api
branch
from
8f18041 to
1c869c1
Compare
luccas-harbour
requested changes
May 4, 2026
Contributor
luccas-harbour
left a comment
luccas-harbour
left a comment
There was a problem hiding this comment.
hey @tupizz!
I started reviewing this one and ran into an issue running tests, both packages/layout-engine/layout-bridge/test/performance.test.ts and packages/layout-engine/pm-adapter/src/integration.test.ts failed for me. It looks like you need to add @superdoc/layout-resolved as a test dependency to both @superdoc/pm-adapter and @superdoc/layout-bridge
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
2 participants
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.
Stack
What changed
Builds on PR1 (which made DomPainter consume
ResolvedLayoutinternally) by collapsing the legacy public surface so thatResolvedLayoutis the only painter input.Painter (
packages/layout-engine/painters/dom)index.tscollapsed to a thin pass-through. Deleted:buildLegacyPaintInput,normalizeDomPainterInput,isDomPainterInput,wrapProvider,resolveDecorationItems,normalizeOptionalBlockMeasurePair,assertRequiredBlockMeasurePair,createEmptyResolvedLayout.paint()now accepts{ resolvedLayout, mount, mapping? }only. The legacy(layout, mount)shape is gone._test-utils.ts(new) hosts acreateTestPainterhelper that wraps a painter for tests still expressing inputs in the legacy shape, chaining the user'sonPaintSnapshotafter the internal one.Tests
pm-adapterintegration tests migrated toResolvedLayout.layout-bridgeperf benchmark migrated toresolveLayout()+paint({ resolvedLayout }).super-editorPresentationEditor.test.tsmock forresolveLayoutsynthesizes aResolvedPageper sourceLayoutpage sorebuildRegions(resolvedLayout)populates regions correctly.Why this split
PR1 keeps the internal migration reviewable on its own. This PR removes the legacy entry points and forces every consumer onto
ResolvedLayout. PR3 then locks the boundary (test guard + drops@superdoc/layout-resolvedfrom runtime deps).Verification
painter-dom,pm-adapter,layout-bridge,layout-engine,super-editor.Test plan