Skip to content

Commit 51627ac

Browse files
authored
refactor(direction): drop ParagraphAttrs.direction scalar field (SD-2778) (#3290)
* refactor(direction): centralize last paragraph isRtl reads on helper (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. * test(direction): pin SD-2778 migration via typed-path + broader-fallback 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. * refactor(direction): drop ParagraphAttrs.direction scalar field (SD-2778) 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. * refactor(direction): finish dropping attrs.direction (diff.ts + stale 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. * docs(direction): retire ParagraphAttrs.direction mention in pm-adapter 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. * test(direction): pin producer contract that attrs.direction is not emitted 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.
1 parent 202325b commit 51627ac

16 files changed

Lines changed: 85 additions & 98 deletions

File tree

packages/layout-engine/contracts/src/direction-context.test.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,37 +7,24 @@ describe('getParagraphInlineDirection', () => {
77
expect(getParagraphInlineDirection(null)).toBeUndefined();
88
});
99

10-
it('prefers directionContext.inlineDirection over legacy fields', () => {
10+
it('prefers directionContext.inlineDirection over paragraphProperties.rightToLeft', () => {
1111
const attrs = {
1212
directionContext: { inlineDirection: 'rtl' as const },
13-
direction: 'ltr',
14-
rtl: false,
13+
paragraphProperties: { rightToLeft: false },
1514
};
1615
expect(getParagraphInlineDirection(attrs)).toBe('rtl');
1716
});
1817

1918
it('falls back past directionContext when inlineDirection is null', () => {
2019
// Per resolver semantics, inlineDirection=null/undefined means "no explicit
21-
// w:bidi"; the legacy `direction` field is the next fallback in the chain.
22-
const attrs = { directionContext: { inlineDirection: null }, direction: 'rtl' };
20+
// w:bidi"; paragraphProperties.rightToLeft is the PM-node/editor fallback.
21+
const attrs = {
22+
directionContext: { inlineDirection: null },
23+
paragraphProperties: { rightToLeft: true },
24+
};
2325
expect(getParagraphInlineDirection(attrs)).toBe('rtl');
2426
});
2527

26-
it('falls back to attrs.direction', () => {
27-
expect(getParagraphInlineDirection({ direction: 'rtl' })).toBe('rtl');
28-
expect(getParagraphInlineDirection({ direction: 'ltr' })).toBe('ltr');
29-
});
30-
31-
it('falls back to attrs.dir', () => {
32-
expect(getParagraphInlineDirection({ dir: 'rtl' })).toBe('rtl');
33-
expect(getParagraphInlineDirection({ dir: 'ltr' })).toBe('ltr');
34-
});
35-
36-
it('falls back to attrs.rtl boolean', () => {
37-
expect(getParagraphInlineDirection({ rtl: true })).toBe('rtl');
38-
expect(getParagraphInlineDirection({ rtl: false })).toBe('ltr');
39-
});
40-
4128
it('falls back to paragraphProperties.rightToLeft', () => {
4229
expect(getParagraphInlineDirection({ paragraphProperties: { rightToLeft: true } })).toBe('rtl');
4330
expect(getParagraphInlineDirection({ paragraphProperties: { rightToLeft: false } })).toBe('ltr');

packages/layout-engine/contracts/src/direction-context.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ export type RunScriptContext = {
161161
* Read a paragraph's inline base direction from its attributes.
162162
*
163163
* Prefers the resolved {@link ParagraphDirectionContext} (SD-2776) when
164-
* present, then falls back to the legacy scalar fields (`direction`,
165-
* `dir`, `rtl`, `paragraphProperties.rightToLeft`) for compatibility
166-
* until SD-2778 collapses the duplicates.
164+
* present. Falls back to `paragraphProperties.rightToLeft` for PM-node /
165+
* editor paths that store direction on the raw OOXML properties rather
166+
* than the typed direction context.
167167
*
168168
* Consumers should call this instead of inspecting attrs ad hoc so the
169169
* direction source check stays in one place.
@@ -172,25 +172,16 @@ export function getParagraphInlineDirection(
172172
attrs:
173173
| {
174174
directionContext?: { inlineDirection?: BaseDirection | null } | null;
175-
direction?: string | null;
176-
dir?: string | null;
177-
rtl?: boolean | null;
178175
paragraphProperties?: { rightToLeft?: boolean | null } | null;
179176
}
180177
| null
181178
| undefined,
182179
): BaseDirection | undefined {
183180
const fromContext = attrs?.directionContext?.inlineDirection;
184181
if (fromContext != null) return fromContext;
185-
// AIDEV-NOTE: compat-fallback - used when ParagraphAttrs.directionContext.inlineDirection is absent.
186-
// Retire once SD-2778 collapses the duplicate scalar fields onto directionContext.
187182
const ppRtl = attrs?.paragraphProperties?.rightToLeft;
188-
if (attrs?.direction === 'rtl' || attrs?.dir === 'rtl' || attrs?.rtl === true || ppRtl === true) {
189-
return 'rtl';
190-
}
191-
if (attrs?.direction === 'ltr' || attrs?.dir === 'ltr' || attrs?.rtl === false || ppRtl === false) {
192-
return 'ltr';
193-
}
183+
if (ppRtl === true) return 'rtl';
184+
if (ppRtl === false) return 'ltr';
194185
return undefined;
195186
}
196187

packages/layout-engine/contracts/src/index.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,19 +1533,14 @@ export type ParagraphAttrs = {
15331533
trackedChangesEnabled?: boolean;
15341534
/** Marks an empty paragraph that only exists to carry section properties. */
15351535
sectPrMarker?: boolean;
1536-
/**
1537-
* Resolved paragraph inline base direction. Populated from `directionContext.inlineDirection`
1538-
* during pm-adapter conversion; left undefined when no explicit bidi is set so the browser
1539-
* can apply UBA via missing `dir` attribute.
1540-
*
1541-
* Prefer reading `directionContext` (typed, complete) over this scalar field. The scalar
1542-
* remains for backwards compatibility with consumers that only need inline direction.
1543-
*/
1544-
direction?: 'ltr' | 'rtl';
15451536
/**
15461537
* Resolved direction context for the paragraph (inline direction + writing mode).
15471538
* Single source of truth for paragraph direction-aware rendering decisions.
15481539
*
1540+
* Read via `getParagraphInlineDirection(attrs)` rather than inspecting this
1541+
* field directly so the helper can normalize `null` vs `undefined` and fall
1542+
* back to `paragraphProperties.rightToLeft` for PM-node / editor paths.
1543+
*
15491544
* See `@superdoc/contracts/direction-context` for axis semantics.
15501545
*/
15511546
directionContext?: ParagraphDirectionContext;

packages/layout-engine/layout-bridge/src/diff.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020
DropCapDescriptor,
2121
ParagraphFrame,
2222
} from '@superdoc/contracts';
23+
import { getParagraphInlineDirection } from '@superdoc/contracts';
2324
import { fieldAnnotationKey } from './field-annotation-key.js';
2425
import { hashRunVisualMarks } from './run-visual-marks.js';
2526
import { hasTrackedChange, resolveTrackedChangesEnabled } from './tracked-changes-utils.js';
@@ -369,7 +370,7 @@ const paragraphAttrsEqual = (a?: ParagraphAttrs, b?: ParagraphAttrs): boolean =>
369370
a.tabIntervalTwips !== b.tabIntervalTwips ||
370371
a.keepNext !== b.keepNext ||
371372
a.keepLines !== b.keepLines ||
372-
a.direction !== b.direction ||
373+
getParagraphInlineDirection(a) !== getParagraphInlineDirection(b) ||
373374
a.floatAlignment !== b.floatAlignment
374375
) {
375376
return false;

packages/layout-engine/layout-bridge/test/cache.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,8 +2024,12 @@ describe('MeasureCache', () => {
20242024

20252025
describe('other paragraph attribute changes', () => {
20262026
it('invalidates cache when direction changes', () => {
2027-
const block1 = paragraphWithAttrs('p1', 'Hello', { direction: 'ltr' });
2028-
const block2 = paragraphWithAttrs('p1', 'Hello', { direction: 'rtl' });
2027+
const block1 = paragraphWithAttrs('p1', 'Hello', {
2028+
directionContext: { inlineDirection: 'ltr', writingMode: 'horizontal-tb' },
2029+
});
2030+
const block2 = paragraphWithAttrs('p1', 'Hello', {
2031+
directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' },
2032+
});
20292033

20302034
cache.set(block1, 400, 600, { totalHeight: 20 });
20312035
expect(cache.get(block2, 400, 600)).toBeUndefined();
@@ -2061,7 +2065,7 @@ describe('MeasureCache', () => {
20612065
{ val: 'end', pos: 8640, leader: 'dot' },
20622066
],
20632067
keepNext: true,
2064-
direction: 'ltr',
2068+
directionContext: { inlineDirection: 'ltr', writingMode: 'horizontal-tb' },
20652069
};
20662070

20672071
const block1 = paragraphWithAttrs('p1', 'Hello', complexAttrs);

packages/layout-engine/layout-bridge/test/diff.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,16 @@ describe('computeDirtyRegions', () => {
434434

435435
describe('other paragraph attribute changes', () => {
436436
it('detects direction change', () => {
437-
const prev = [paragraphWithAttrs('p1', 'Hello', { direction: 'ltr' })];
438-
const next = [paragraphWithAttrs('p1', 'Hello', { direction: 'rtl' })];
437+
const prev = [
438+
paragraphWithAttrs('p1', 'Hello', {
439+
directionContext: { inlineDirection: 'ltr', writingMode: 'horizontal-tb' },
440+
}),
441+
];
442+
const next = [
443+
paragraphWithAttrs('p1', 'Hello', {
444+
directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' },
445+
}),
446+
];
439447
const result = computeDirtyRegions(prev, next);
440448
expect(result.firstDirtyIndex).toBe(0);
441449
});
@@ -492,7 +500,7 @@ describe('computeDirtyRegions', () => {
492500
{ val: 'end' as const, pos: 8640, leader: 'dot' as const },
493501
],
494502
keepNext: true,
495-
direction: 'ltr' as const,
503+
directionContext: { inlineDirection: 'ltr' as const, writingMode: 'horizontal-tb' as const },
496504
};
497505
const prev = [paragraphWithAttrs('p1', 'Hello', complexAttrs)];
498506
const next = [paragraphWithAttrs('p1', 'Hello', complexAttrs)];

packages/layout-engine/layout-bridge/test/position-hit.test.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,15 @@ describe('isRtlBlock', () => {
2323
).toBe(true);
2424
});
2525

26-
it('keeps legacy paragraph direction as a fallback', () => {
27-
expect(isRtlBlock(paragraph({ direction: 'rtl' }))).toBe(true);
28-
});
29-
3026
it('does not treat writing mode as inline RTL direction', () => {
3127
expect(isRtlBlock(paragraph({ textDirection: 'tbRl' }))).toBe(false);
3228
});
3329

34-
it('lets resolved direction context override legacy scalar direction', () => {
30+
it('lets resolved direction context override paragraphProperties.rightToLeft', () => {
3531
expect(
3632
isRtlBlock(
3733
paragraph({
38-
direction: 'rtl',
34+
paragraphProperties: { rightToLeft: true },
3935
directionContext: {
4036
inlineDirection: 'ltr',
4137
writingMode: 'horizontal-tb',
@@ -45,14 +41,14 @@ describe('isRtlBlock', () => {
4541
).toBe(false);
4642
});
4743

48-
it('falls through to legacy direction when directionContext.inlineDirection is undefined', () => {
44+
it('falls through to paragraphProperties.rightToLeft when directionContext.inlineDirection is undefined', () => {
4945
// The resolver may produce inlineDirection: undefined when no paragraph w:bidi is set
5046
// anywhere in the cascade. In that case the typed context carries no inline-direction
51-
// signal, and the legacy `direction` / `dir` field (if any) should still be honored.
47+
// signal, and the PM-node paragraphProperties.rightToLeft fallback still applies.
5248
expect(
5349
isRtlBlock(
5450
paragraph({
55-
direction: 'rtl',
51+
paragraphProperties: { rightToLeft: true },
5652
directionContext: {
5753
inlineDirection: undefined,
5854
writingMode: 'horizontal-tb',

packages/layout-engine/layout-resolved/src/versionSignature.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('deriveBlockVersion - bidi', () => {
3333
const makeParagraph = (bidi?: TextRun['bidi']): FlowBlock => ({
3434
kind: 'paragraph',
3535
id: 'p1',
36-
attrs: { direction: 'rtl' },
36+
attrs: { directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' } },
3737
runs: [
3838
{
3939
text: '23.03.2026',

packages/layout-engine/painters/dom/src/formatting-marks.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,11 @@ describe('DomPainter formatting marks', () => {
159159

160160
container.innerHTML = '';
161161
const rtlPainter = createDomPainter({
162-
blocks: [createParagraphBlock(text, { direction: 'rtl' })],
162+
blocks: [
163+
createParagraphBlock(text, {
164+
directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' },
165+
}),
166+
],
163167
measures: [measure],
164168
showFormattingMarks: true,
165169
});

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5006,7 +5006,7 @@ describe('DomPainter', () => {
50065006
],
50075007
attrs: {
50085008
alignment: 'center',
5009-
direction: 'rtl',
5009+
directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' },
50105010
},
50115011
};
50125012
const footerMeasure: Measure = {
@@ -6072,7 +6072,7 @@ describe('DomPainter', () => {
60726072
kind: 'paragraph',
60736073
id: 'resolved-rtl-marker',
60746074
runs: [{ text: 'RTL nested item', fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 16 }],
6075-
attrs: { direction: 'rtl' as const },
6075+
attrs: { directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' } },
60766076
};
60776077

60786078
const paragraphMeasure: Measure = {
@@ -8536,7 +8536,7 @@ describe('DomPainter', () => {
85368536
kind: 'paragraph',
85378537
id: 'rtl-block',
85388538
runs: [{ text: 'مرحبا', fontFamily: 'Arial', fontSize: 16 }],
8539-
attrs: { direction: 'rtl' as const, ...attrs },
8539+
attrs: { directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' }, ...attrs },
85408540
});
85418541

85428542
const rtlMeasure: Measure = {
@@ -8591,7 +8591,7 @@ describe('DomPainter', () => {
85918591
{ kind: 'tab', width: 40, fontFamily: 'Arial', fontSize: 16 } as any,
85928592
{ text: 'عالم', fontFamily: 'Arial', fontSize: 16 },
85938593
],
8594-
attrs: { direction: 'rtl' as const },
8594+
attrs: { directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' } },
85958595
};
85968596

85978597
const tabMeasure: Measure = {

0 commit comments

Comments
 (0)