Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Shared constants for the Markdown ↔ ProseMirror conversion pipeline.
*/

/**
* The font family used to mark monospace ("code") text in both directions of
* the markdown ↔ ProseMirror conversion:
* - `mdastToProseMirror.ts` applies this font (via a `textStyle` mark and/or
* direct `runProperties.fontFamily`) to fenced code blocks (`code`) and
* inline code spans (`inlineCode`).
* - `proseMirrorToMdast.ts` detects this font (on either the mark or the
* direct run properties) to convert monospace runs back into `inlineCode`
* mdast nodes.
*
* Keeping this as a single shared constant avoids the two directions
* silently drifting out of sync if the monospace font ever changes.
*/
export const MARKDOWN_MONOSPACE_FONT = 'Courier New';
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import { describe, expect, it } from 'vitest';
import { Schema } from 'prosemirror-model';
import { convertMdastToBlocks } from './mdastToProseMirror.js';
import { MARKDOWN_MONOSPACE_FONT } from './constants.js';
import type { MdastConversionContext } from './types.js';
import type { Root, Code, Paragraph, InlineCode } from 'mdast';

// ---------------------------------------------------------------------------
// Minimal schema mirroring SuperEditor's node/mark types (enough for
// mdastToProseMirror's JSON output to round-trip through nodeFromJSON if
// a test wants to materialize it — most assertions here work directly on
// the raw JSON returned by convertMdastToBlocks).
// ---------------------------------------------------------------------------

const schema = new Schema({
nodes: {
doc: { content: 'block+' },
paragraph: {
group: 'block',
content: 'inline*',
attrs: {
paragraphProperties: { default: null },
numberingProperties: { default: null },
},
},
run: {
inline: true,
group: 'inline',
content: 'inline*',
attrs: { runProperties: { default: null } },
},
text: { group: 'inline' },
lineBreak: { inline: true, group: 'inline' },
},
marks: {
bold: {},
italic: {},
strike: {},
textStyle: { attrs: { fontFamily: { default: null } } },
},
});

function createMockEditor(): any {
return {
converter: { numbering: { definitions: {}, abstracts: {} } },
state: { doc: { descendants: () => {} } },
};
}

function makeCtx(): MdastConversionContext {
return {
editor: createMockEditor(),
schema,
diagnostics: [],
options: { dryRun: true },
};
}

/** Build a minimal mdast Root wrapping a single `code` node. */
function codeRoot(value: string, lang: string | null = null): Root {
const code: Code = { type: 'code', lang, value };
return { type: 'root', children: [code] };
}

/** Build a minimal mdast Root wrapping a paragraph containing an inlineCode span. */
function inlineCodeRoot(value: string): Root {
const inlineCode: InlineCode = { type: 'inlineCode', value };
const paragraph: Paragraph = { type: 'paragraph', children: [inlineCode] };
return { type: 'root', children: [paragraph] };
}

// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------

describe('convertMdastToBlocks — code block (fenced)', () => {
it('wraps a single-line code block in a paragraph of one run', () => {
const blocks = convertMdastToBlocks(codeRoot('const x = 1;'), makeCtx());

expect(blocks).toHaveLength(1);
const [paragraph] = blocks;
expect(paragraph.type).toBe('paragraph');
expect(paragraph.content).toHaveLength(1);

const [run] = paragraph.content!;
expect(run.type).toBe('run');
expect(run.content).toHaveLength(1);
expect(run.content![0]).toMatchObject({ type: 'text', text: 'const x = 1;' });
});

it('sets both the textStyle mark and the direct runProperties.fontFamily attr', () => {
const blocks = convertMdastToBlocks(codeRoot('code'), makeCtx());
const run = blocks[0].content![0];

// Mark (consumed by calculateInlineRunPropertiesPlugin's mark-based recalculation)
expect(run.content![0].marks).toEqual([{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }]);

// Direct attrs (consumed by callers that never dispatch through a live Editor)
expect(run.attrs).toEqual({
runProperties: {
fontFamily: {
ascii: MARKDOWN_MONOSPACE_FONT,
eastAsia: MARKDOWN_MONOSPACE_FONT,
hAnsi: MARKDOWN_MONOSPACE_FONT,
cs: MARKDOWN_MONOSPACE_FONT,
},
},
});
});

it('uses the OOXML rFonts key ("fontFamily"), never a bare "rFonts" key', () => {
const blocks = convertMdastToBlocks(codeRoot('code'), makeCtx());
const run = blocks[0].content![0];

expect(run.attrs!.runProperties).toHaveProperty('fontFamily');
expect(run.attrs!.runProperties).not.toHaveProperty('rFonts');
});

it('joins multiple lines with lineBreak nodes, one run per non-empty line', () => {
const blocks = convertMdastToBlocks(codeRoot('line one\nline two\nline three'), makeCtx());
const content = blocks[0].content!;

// run, lineBreak, run, lineBreak, run
expect(content.map((n) => n.type)).toEqual(['run', 'lineBreak', 'run', 'lineBreak', 'run']);
expect(content[0].content![0].text).toBe('line one');
expect(content[2].content![0].text).toBe('line two');
expect(content[4].content![0].text).toBe('line three');
});

it('emits a lineBreak but no run for a blank line within the code block', () => {
const blocks = convertMdastToBlocks(codeRoot('line one\n\nline three'), makeCtx());
const content = blocks[0].content!;

// run, lineBreak, lineBreak, run — no run is emitted for the empty middle line
expect(content.map((n) => n.type)).toEqual(['run', 'lineBreak', 'lineBreak', 'run']);
});

it('produces an empty paragraph for an empty code block', () => {
const blocks = convertMdastToBlocks(codeRoot(''), makeCtx());

expect(blocks).toHaveLength(1);
expect(blocks[0].type).toBe('paragraph');
expect(blocks[0].content).toBeUndefined();
});
});

describe('convertMdastToBlocks — inline code', () => {
it('wraps inline code in a run with only the textStyle mark (no direct runProperties)', () => {
const blocks = convertMdastToBlocks(inlineCodeRoot('inline'), makeCtx());
const run = blocks[0].content![0];

expect(run.type).toBe('run');
expect(run.content![0]).toMatchObject({
type: 'text',
text: 'inline',
marks: [{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }],
});

// Unlike the fenced code-block case, inlineCode relies solely on the mark —
// see the comment on convertCodeBlock for why these two paths differ.
expect(run.attrs).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { ListHelpers } from '../list-numbering-helpers.js';
import { generateDocxRandomId } from '../generateDocxRandomId.js';
import { readImageDimensionsFromDataUri } from '../../super-converter/image-dimensions.js';
import type { MdastConversionContext, MarkdownDiagnostic } from './types.js';
import { MARKDOWN_MONOSPACE_FONT } from './constants.js';

// ---------------------------------------------------------------------------
// Public entry point
Expand Down Expand Up @@ -274,9 +275,44 @@ function convertCodeBlock(node: MdastCode, ctx: MdastConversionContext): JsonNod
content.push({ type: 'lineBreak' });
}
if (lines[i].length > 0) {
content.push(makeRun(lines[i], [], { rFonts: { ascii: 'Courier New', hAnsi: 'Courier New' } }));
// Use a `textStyle` mark (not just a direct `runProperties.fontFamily`
// attr) so `calculateInlineRunPropertiesPlugin`'s mark-based
// recalculation (which treats `fontFamily` as mark-derived) has a mark
// to decode the font from. A bare `runProperties.fontFamily` with no
// matching mark gets dropped on the first `editor.dispatch(tr)`, since
// that plugin recomputes inline run properties from marks/styles and
// `fontFamily` is not preserved via the existing-props fallback for
// mark-derived keys.
//
// Unlike the `inlineCode` case below, we ALSO set the direct
// `runProperties.fontFamily` attr here (belt-and-suspenders, not a true
// mirror of `inlineCode`). That's deliberate: code-block JSON can be
// consumed by callers that never dispatch a transaction through a live
// `Editor` (e.g. inspecting the converter's raw JSON output directly),
// in which case `calculateInlineRunPropertiesPlugin` never runs and the
// mark alone wouldn't populate `runProperties`. `inlineCode` spans are
// always produced within content destined for an editor dispatch, so a
// mark alone is sufficient there. Once dispatched, the plugin
// recomputes `runProperties.fontFamily` from the mark via
// `decodeRPrFromMarks` anyway (see styles.js). That decoder always sets
// all four rFonts slots (ascii/eastAsia/hAnsi/cs) to the same value, so
// the direct attrs set here match that 4-key shape upfront — otherwise
// the 2-key shape would be silently reshaped by the plugin on first
// dispatch, and any pre-dispatch consumer of the raw JSON would see a
// different (incomplete) shape than a post-dispatch consumer.
content.push(
makeRun(lines[i], [{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }], {
fontFamily: {
ascii: MARKDOWN_MONOSPACE_FONT,
eastAsia: MARKDOWN_MONOSPACE_FONT,
hAnsi: MARKDOWN_MONOSPACE_FONT,
cs: MARKDOWN_MONOSPACE_FONT,
},
}),
);
}
}

return makeParagraph(content);
}

Expand Down Expand Up @@ -443,7 +479,7 @@ function convertInlineNode(node: PhrasingContent, ctx: MdastConversionContext, p
return [
makeRun((node as MdastInlineCode).value, [
...parentMarks,
{ type: 'textStyle', attrs: { fontFamily: 'Courier New' } },
{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } },
]),
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe('proseMirrorDocToMdast', () => {
});

describe('inline code via run properties', () => {
it('converts Courier New rFonts in runProperties to inlineCode', () => {
it('converts Courier New fontFamily in runProperties to inlineCode', () => {
const doc = buildDoc({
type: 'doc',
content: [
Expand All @@ -296,7 +296,7 @@ describe('proseMirrorDocToMdast', () => {
content: [
{
type: 'run',
attrs: { runProperties: { rFonts: { ascii: 'Courier New', hAnsi: 'Courier New' } } },
attrs: { runProperties: { fontFamily: { ascii: 'Courier New', hAnsi: 'Courier New' } } },
content: [{ type: 'text', text: 'monospace' }],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import type {
} from 'mdast';
import { ListHelpers } from '../list-numbering-helpers.js';
import type { Editor } from '../../Editor.js';
import { MARKDOWN_MONOSPACE_FONT } from './constants.js';

// ---------------------------------------------------------------------------
// Public entry point
Expand Down Expand Up @@ -350,9 +351,11 @@ function convertRunNode(node: PmNode, editor: Editor): PhrasingContent[] {
node.forEach((child) => {
if (child.type.name === 'text') {
const text = child.text ?? '';
// Check for inline code via run properties (Courier New font)
// Check for inline code via run properties (Courier New font) or via
// the `textStyle` mark (see `isMonospaceSignal` — the two detectors
// are unified so both directions of the monospace signal stay in sync).
const runProps = node.attrs.runProperties as Record<string, unknown> | undefined;
if (isMonospaceRun(runProps)) {
if (isMonospaceSignal(runProps, child.marks)) {
const code: InlineCode = { type: 'inlineCode', value: text };
result.push(wrapWithMarks(code, child.marks));
} else {
Expand All @@ -365,11 +368,36 @@ function convertRunNode(node: PmNode, editor: Editor): PhrasingContent[] {
return result;
}

/**
* True when the direct `runProperties.fontFamily` attr is set to the
* monospace font used by `mdastToProseMirror.ts` for code blocks/spans.
*/
function isMonospaceRun(runProps: Record<string, unknown> | undefined): boolean {
if (!runProps) return false;
const rFonts = runProps.rFonts as Record<string, string> | undefined;
if (!rFonts) return false;
return rFonts.ascii === 'Courier New' || rFonts.hAnsi === 'Courier New';
const fontFamily = runProps.fontFamily as Record<string, string> | undefined;
if (!fontFamily) return false;
return fontFamily.ascii === MARKDOWN_MONOSPACE_FONT || fontFamily.hAnsi === MARKDOWN_MONOSPACE_FONT;
}
Comment on lines 375 to +380

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Misses w:ascii monospace 🐞 Bug ≡ Correctness

isMonospaceRun() only checks runProperties.fontFamily.ascii/hAnsi, so runs whose fontFamily
object uses OOXML-prefixed keys (e.g. w:ascii, w:hAnsi) won’t be recognized as monospace and
won’t export to inlineCode. This undermines the intended “docx-imported monospace run” round-trip
behavior when the prefixed key shape is present.
Agent Prompt
## Issue description
`isMonospaceRun()` only detects monospace when `runProps.fontFamily` contains unprefixed keys (`ascii`/`hAnsi`). Other parts of the codebase accept OOXML-prefixed keys (`w:ascii`, `w:hAnsi`, etc.), so monospace runs using that representation will not be converted to mdast `inlineCode` during markdown export.

## Issue Context
`getFontFamilyValue()` already normalizes between prefixed and unprefixed keys, and other code paths/tests demonstrate `runProperties.fontFamily` can carry `w:ascii`.

## Fix Focus Areas
- packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.ts[371-401]
- packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.test.ts[287-305]

## Implementation notes
- Update `isMonospaceRun()` to accept both key shapes, e.g.:
  - `const ascii = ff.ascii ?? ff['w:ascii']`
  - `const hAnsi = ff.hAnsi ?? ff['w:hAnsi']`
  - (optionally also check `eastAsia`/`cs` equivalents)
- Consider also handling the case where `runProps.fontFamily` is a string (defensive).
- Add/extend a unit test where `runProperties.fontFamily` uses `{ 'w:ascii': 'Courier New', 'w:hAnsi': 'Courier New' }` and assert it exports to `inlineCode`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


/**
* True when the `textStyle` mark's `fontFamily` attr is set to the
* monospace font used by `mdastToProseMirror.ts` for code blocks/spans.
*/
function isMonospaceMark(marks: readonly Mark[] | undefined): boolean {
if (!marks) return false;
const textStyleMark = marks.find((mark) => mark.type.name === 'textStyle');
return textStyleMark?.attrs?.fontFamily === MARKDOWN_MONOSPACE_FONT;
}

/**
* Single shared "is this monospace/code text" predicate. Combines the two
* independent signals a run can carry — the direct `runProperties.fontFamily`
* attr (checked by `convertRunNode`) and the `textStyle` mark (checked by
* `applyMark`) — so both call sites agree on what counts as code, and any
* future change to the detection logic only needs to happen in one place.
*/
function isMonospaceSignal(runProps: Record<string, unknown> | undefined, marks: readonly Mark[] | undefined): boolean {
return isMonospaceRun(runProps) || isMonospaceMark(marks);
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -437,9 +465,11 @@ function applyMark(mark: Mark, child: PhrasingContent): PhrasingContent {
children: [child],
} as Link;
case 'textStyle': {
// Courier New fontFamily → inlineCode (only if child is text)
// Monospace fontFamily → inlineCode (only if child is text). Uses the
// same `isMonospaceMark` predicate as `convertRunNode` via the shared
// `MARKDOWN_MONOSPACE_FONT` constant (see `isMonospaceSignal`).
const fontFamily = mark.attrs.fontFamily as string | undefined;
if (fontFamily === 'Courier New' && child.type === 'text') {
if (fontFamily === MARKDOWN_MONOSPACE_FONT && child.type === 'text') {
return { type: 'inlineCode', value: (child as Text).value } as InlineCode;
}
return child;
Expand Down
Loading
Loading