Skip to content

Commit edd24a6

Browse files
caio-pizzolcaio-pizzol
committed
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.
1 parent 2699e3a commit edd24a6

11 files changed

Lines changed: 56 additions & 85 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/test/cache.test.ts‎

Lines changed: 6 additions & 2 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();

‎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/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 = {

‎packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ describe('computeParagraphAttrs', () => {
324324

325325
const { paragraphAttrs } = computeParagraphAttrs(paragraph as never);
326326

327-
expect(paragraphAttrs.direction).toBe('rtl');
327+
expect(paragraphAttrs.directionContext?.inlineDirection).toBe('rtl');
328328
});
329329

330330
it('does NOT inherit section direction for paragraph inline direction (§17.6.1)', () => {
@@ -345,25 +345,21 @@ describe('computeParagraphAttrs', () => {
345345
};
346346

347347
const { paragraphAttrs } = computeParagraphAttrs(paragraph as never, converterContext as never);
348-
expect(paragraphAttrs.direction).toBeUndefined();
348+
expect(paragraphAttrs.directionContext?.inlineDirection).toBeUndefined();
349349
});
350350

351-
// SD-2777 invariant: pm-adapter must keep `direction` and
352-
// `directionContext.inlineDirection` aligned (or both absent). This is the
353-
// load-bearing property that lets `getParagraphInlineDirection` produce a
354-
// hash byte-identical to the legacy `attrs.direction` read across the
355-
// entire R2 corpus. If this breaks, the helper-based hash sites in
356-
// layout-bridge / layout-resolved / painter-dom will drift from the
357-
// pre-migration output.
358-
describe('SD-2777: direction and directionContext.inlineDirection are paired', () => {
351+
// SD-2778: pm-adapter writes inline direction onto `directionContext.inlineDirection`
352+
// as the single source of truth. The legacy scalar `attrs.direction` field has been
353+
// removed; `getParagraphInlineDirection` reads `directionContext` directly.
354+
describe('SD-2778: directionContext.inlineDirection mirrors paragraphProperties.rightToLeft', () => {
359355
const cases: Array<{ name: string; rightToLeft: boolean | undefined; expected: 'rtl' | 'ltr' | undefined }> = [
360356
{ name: 'rightToLeft=true', rightToLeft: true, expected: 'rtl' },
361357
{ name: 'rightToLeft=false', rightToLeft: false, expected: 'ltr' },
362358
{ name: 'rightToLeft=undefined', rightToLeft: undefined, expected: undefined },
363359
];
364360

365361
for (const { name, rightToLeft, expected } of cases) {
366-
it(`${name}: attrs.direction === directionContext.inlineDirection (${String(expected)})`, () => {
362+
it(`${name}: directionContext.inlineDirection === ${String(expected)}`, () => {
367363
const paragraph: PMNode = {
368364
type: { name: 'paragraph' },
369365
attrs: {
@@ -373,7 +369,6 @@ describe('computeParagraphAttrs', () => {
373369

374370
const { paragraphAttrs } = computeParagraphAttrs(paragraph as never);
375371

376-
expect(paragraphAttrs.direction).toBe(expected);
377372
expect(paragraphAttrs.directionContext?.inlineDirection).toBe(expected);
378373
});
379374
}

‎packages/layout-engine/pm-adapter/src/attributes/paragraph.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,6 @@ export const computeParagraphAttrs = (
388388
keepLines: resolvedParagraphProperties.keepLines,
389389
floatAlignment: floatAlignment,
390390
pageBreakBefore: resolvedParagraphProperties.pageBreakBefore,
391-
...(normalizedDirection ? { direction: normalizedDirection } : {}),
392391
directionContext,
393392
};
394393

‎packages/layout-engine/pm-adapter/src/index.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3188,7 +3188,7 @@ describe('toFlowBlocks', () => {
31883188
expect(blocks).toHaveLength(1);
31893189
const paragraph = blocks[0];
31903190
expect(paragraph.kind).toBe('paragraph');
3191-
expect(paragraph.attrs?.direction).toBe('rtl');
3191+
expect(paragraph.attrs?.directionContext?.inlineDirection).toBe('rtl');
31923192
expect(paragraph.attrs?.indent?.left).toBe(12);
31933193
expect(paragraph.attrs?.indent?.right).toBe(24);
31943194
});
@@ -3214,7 +3214,7 @@ describe('toFlowBlocks', () => {
32143214
expect(blocks).toHaveLength(1);
32153215
const paragraph = blocks[0];
32163216
expect(paragraph.kind).toBe('paragraph');
3217-
expect(paragraph.attrs?.direction).toBe('ltr');
3217+
expect(paragraph.attrs?.directionContext?.inlineDirection).toBe('ltr');
32183218
});
32193219

32203220
it('does NOT inherit paragraph inline direction from body sectPr w:bidi (§17.6.1)', () => {
@@ -3247,7 +3247,7 @@ describe('toFlowBlocks', () => {
32473247
expect(paragraph.kind).toBe('paragraph');
32483248
// Paragraph inline direction stays undefined; the browser applies UBA via
32493249
// the missing dir attribute. Section pageDirection is preserved separately.
3250-
expect(paragraph.attrs?.direction).toBeUndefined();
3250+
expect(paragraph.attrs?.directionContext?.inlineDirection).toBeUndefined();
32513251
});
32523252

32533253
it('section bidi=0 also does not affect paragraph inline direction', () => {
@@ -3275,7 +3275,7 @@ describe('toFlowBlocks', () => {
32753275
expect(blocks).toHaveLength(1);
32763276
const paragraph = blocks[0];
32773277
expect(paragraph.kind).toBe('paragraph');
3278-
expect(paragraph.attrs?.direction).toBeUndefined();
3278+
expect(paragraph.attrs?.directionContext?.inlineDirection).toBeUndefined();
32793279
});
32803280

32813281
it('handles multiple page breaks', () => {
@@ -4620,7 +4620,7 @@ describe('toFlowBlocks', () => {
46204620
const { blocks } = toFlowBlocks(pmDoc);
46214621

46224622
expect(blocks).toHaveLength(1);
4623-
expect(blocks[0].attrs?.direction).toBe('rtl');
4623+
expect(blocks[0].attrs?.directionContext?.inlineDirection).toBe('rtl');
46244624
expect(blocks[0].attrs?.alignment).toBeUndefined();
46254625
});
46264626

@@ -4649,7 +4649,7 @@ describe('toFlowBlocks', () => {
46494649
const { blocks } = toFlowBlocks(pmDoc);
46504650

46514651
expect(blocks).toHaveLength(1);
4652-
expect(blocks[0].attrs?.direction).toBe('rtl');
4652+
expect(blocks[0].attrs?.directionContext?.inlineDirection).toBe('rtl');
46534653
expect(blocks[0].attrs).toMatchObject({
46544654
alignment: 'center',
46554655
});
@@ -4681,7 +4681,7 @@ describe('toFlowBlocks', () => {
46814681
const { blocks } = toFlowBlocks(pmDoc);
46824682

46834683
expect(blocks).toHaveLength(1);
4684-
expect(blocks[0].attrs?.direction).toBe('rtl');
4684+
expect(blocks[0].attrs?.directionContext?.inlineDirection).toBe('rtl');
46854685
expect(blocks[0].attrs).toMatchObject({
46864686
alignment: 'right',
46874687
});
@@ -4712,7 +4712,7 @@ describe('toFlowBlocks', () => {
47124712
const { blocks } = toFlowBlocks(pmDoc);
47134713

47144714
expect(blocks).toHaveLength(1);
4715-
expect(blocks[0].attrs?.direction).toBe('rtl');
4715+
expect(blocks[0].attrs?.directionContext?.inlineDirection).toBe('rtl');
47164716
expect(blocks[0].attrs).toMatchObject({
47174717
alignment: 'left',
47184718
});
@@ -4776,7 +4776,7 @@ describe('toFlowBlocks', () => {
47764776

47774777
for (const jc of ['both', 'distribute', 'numTab', 'thaiDistribute']) {
47784778
const { blocks } = toFlowBlocks(makeDoc(jc));
4779-
expect(blocks[0].attrs?.direction).toBe('rtl');
4779+
expect(blocks[0].attrs?.directionContext?.inlineDirection).toBe('rtl');
47804780
expect(blocks[0].attrs).toMatchObject({ alignment: 'justify' });
47814781
}
47824782
});

0 commit comments

Comments
 (0)