refactor(direction): drop ParagraphAttrs.direction scalar field (SD-2778)#3290
Merged
caio-pizzol merged 6 commits intoMay 14, 2026
Merged
Conversation
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
…(SD-2778) Migrate the two remaining direct reads of attrs.direction/.dir on the paragraph inline-direction axis onto getParagraphInlineDirection: - layout-bridge/src/position-hit.ts: isRtlBlock - layout-resolved/src/resolveParagraph.ts: isRtl Behavior is unchanged on the typed directionContext path and strictly broader on fallback (the helper also covers paragraphProperties.rightToLeft). After this, no consumer outside the helper reads the legacy scalar fields; a follow-up can stop pm-adapter from writing them and drop them from ParagraphAttrs.
…ack cases
Two coverage gaps caught in review:
- layout-resolved/src/resolveLayout.test.ts ("preserves increasing
first-line marker anchor for nested RTL list levels") used
attrs.direction: 'rtl'. The pre-migration code read attrs.direction
directly, so that fixture would have passed against the old
implementation. Switch to directionContext.inlineDirection so the test
only passes through the new helper-driven typed path.
- layout-bridge/test/position-hit.test.ts: switching isRtlBlock to
getParagraphInlineDirection is strictly broader on fallback (the helper
also picks up paragraphProperties.rightToLeft when no directionContext
is present). Pin that case so the broadening is intentional and not a
regression vector.
caio-pizzol
force-pushed
the
caio-pizzol/SD-2778-collapse-paragraph-direction-reads
branch
from
4a38d8e to
2699e3a
Compare
…778) After #3289 centralized every paragraph isRtl read on getParagraphInlineDirection, the scalar attrs.direction field has no remaining consumers. Drop it from the producer and the type: - pm-adapter no longer writes the conditional direction spread on ParagraphAttrs. directionContext.inlineDirection is the only source. - contracts/index.ts: remove direction?: 'ltr' | 'rtl' from ParagraphAttrs and point the directionContext doc at the helper. - contracts/direction-context.ts: tighten getParagraphInlineDirection's signature and body to drop the attrs.direction/.dir/.rtl fallbacks. paragraphProperties.rightToLeft fallback stays for PM-node / editor paths that read direction off the raw OOXML properties. Tests that asserted on attrs.direction or constructed hand-rolled FlowBlocks with { direction: 'rtl' } now use directionContext.inlineDirection. Expect additive layout JSON snapshot drift once the direction field disappears from paragraph attrs in serialized layouts. Tests: contracts 229, pm-adapter 1838, layout-bridge 1210, layout-resolved 118, painter-dom 1070, super-editor 12836, style-engine 129 - all green.
… fixtures) Follow-up to 19b13fc. Two gaps caught on review: - layout-bridge/src/diff.ts:372 still compared a.direction !== b.direction inside paragraphAttrsEqual. Vitest passed because esbuild/swc skips cross-package types; the tsup DTS build was the one that caught it. Migrate to getParagraphInlineDirection(a) !== getParagraphInlineDirection(b) so the diff respects directionContext + paragraphProperties fallback. - Test fixtures across versionSignature.test.ts, rtl-date-parity.test.ts, cache.test.ts, and diff.test.ts still constructed hand-rolled FlowBlocks with { direction: 'rtl' }. They typecheck via Record<string, unknown> casts but no longer model the actual ParagraphAttrs shape. Migrate them all onto directionContext. Build sweep (contracts → pm-adapter → layout-bridge → layout-resolved → painter-dom) now passes; tests still green across all five.
caio-pizzol
force-pushed
the
caio-pizzol/SD-2778-drop-paragraph-direction-field
branch
from
4863523 to
a77894c
Compare
…r direction README After SD-2778 drops the scalar `direction` field, the README sentence that pointed consumers at it is stale. Point them at `getParagraphInlineDirection` instead so the next reader lands on the helper that knows about the typed context + paragraphProperties fallback.
Base automatically changed from
caio-pizzol/SD-2778-collapse-paragraph-direction-reads
to
main
May 14, 2026 13:23
…itted SD-2778 removes ParagraphAttrs.direction from the type, but TypeScript's structural typing permits index-signature extra keys. Add a runtime assertion inside the SD-2778 describe block that pm-adapter never emits the legacy scalar field. Catches accidental re-introduction via a future spread that TypeScript would let through.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
caio-pizzol
deleted the
caio-pizzol/SD-2778-drop-paragraph-direction-field
branch
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.97 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.3.0-next.143 |
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.141 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-cli v0.8.0-next.112 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.95 |
Contributor
|
🎉 This PR is included in superdoc v1.30.0-next.92 The release is available on GitHub release |
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.
Producer-side half of SD-2778, stacked on #3289. After #3289 centralized every paragraph isRtl read on
getParagraphInlineDirection, the scalarattrs.directionfield has no remaining consumers.What lands
pm-adapter/src/attributes/paragraph.ts: drop the conditionaldirectionspread when assemblingParagraphAttrs.directionContext.inlineDirectionis the only source pm-adapter writes for inline direction.contracts/src/index.ts: removedirection?: 'ltr' | 'rtl'fromParagraphAttrs. The doc ondirectionContextnow points atgetParagraphInlineDirection.contracts/src/direction-context.ts: tightengetParagraphInlineDirectionto drop the deadattrs.direction/.dir/.rtlfallbacks.paragraphProperties.rightToLeftfallback stays for PM-node / editor paths that read direction off the raw OOXML properties.layout-bridge/src/diff.ts:paragraphAttrsEqualnow compares viagetParagraphInlineDirection(a) !== getParagraphInlineDirection(b)so the diff respects `directionContext` and the `paragraphProperties.rightToLeft` fallback in one place.Tests that asserted on
attrs.directionor constructed hand-rolled FlowBlocks with{ direction: 'rtl' }now usedirectionContext.inlineDirection.Behavior
For pm-adapter-produced FlowBlocks: equivalent. The scalar field was always mirroring
directionContext.inlineDirection, so every read site that goes through the helper gets the same answer.For PM-node / editor paths that read off raw
paragraphProperties.rightToLeft: equivalent. Helper still covers that fallback.Layout-comparison heads-up: removing
directionfromParagraphAttrsmeans it disappears from serialized layout JSON for RTL paragraphs. Expect subtractive schema-evolution diffs inpnpm layout:compare(field present in reference baseline, absent in candidate) until the reference baseline is updated. Not a behavior regression.Tests: contracts 229, pm-adapter 1838, layout-bridge 1210, layout-resolved 118, painter-dom 1070, super-editor 12836, style-engine 129. All green.
Build sweep: `pnpm --filter @superdoc/painter-dom... build` (full transitive type check) passes. Vitest alone wasn't enough - esbuild/swc skips cross-package types and the original commit had a tsup DTS failure in `diff.ts` that only surfaced under `tsc --noEmit`.
Stack: based on #3289. Both PRs close SD-2778: #3289 centralizes the reads, this one drops the producer + the type.