facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
19.39k stars 1.64k forks source link

Bug: Markdown import/export of formatted link text #5148

Open robfig opened 11 months ago

robfig commented 11 months ago

Lexical version: 0.12.2

Steps To Reproduce

  1. In Lexical playground: linked text with inline code format:
 root
  └ (58) paragraph  
    └ (59) link  "https://lexical.dev"
>     └ (60) text  "XXX" { format: code }
  1. Export to markdown.

The current behavior

`[XXX](https://lexical.dev)`

The expected behavior

[`XXX`](https://lexical.dev)

Within an inline code block, markdown is not interpreted, so the current result prints as the literal markdown for the link. However, Markdown tags are valid inside the text content of a Link, so the valid export of that document is with inline code inside the text content of the link.

Preliminary Investigation

I added these two failing test cases to LexicalMarkdown.test.ts

    {
      html: '<p><a href="https://lexical.dev"><code spellcheck="false" style="white-space: pre-wrap;"><span>XXX</span></code></a><span style="white-space: pre-wrap;"> world</span></p>',
      md: '[`XXX`](https://lexical.dev) world',
    },
    {
      html: '<p><span style="white-space: pre-wrap;">Hello </span><a href="https://lexical.dev"><code spellcheck="false" style="white-space: pre-wrap;"><span>XXX</span></code></a><span style="white-space: pre-wrap;"> world</span></p>',
      md: 'Hello [`XXY`](https://lexical.dev) world',
    },

I was able to get the export working very easily, with this diff:

+++ b/packages/lexical-markdown/src/MarkdownTransformers.ts
@@ -344,16 +344,17 @@ export const LINK: TextMatchTransformer = {
       return null;
     }
     const title = node.getTitle();
-    const linkContent = title
-      ? `[${node.getTextContent()}](${node.getURL()} "${title}")`
-      : `[${node.getTextContent()}](${node.getURL()})`;
+    const linkHref = title ? `${node.getURL()} "${title}"` : node.getURL();
     const firstChild = node.getFirstChild();
     // Add text styles only if link has single text node inside. If it's more
     // then one we ignore it as markdown does not support nested styles for links
     if (node.getChildrenSize() === 1 && $isTextNode(firstChild)) {
-      return exportFormat(firstChild, linkContent);
+      return `[${exportFormat(
+        firstChild,
+        node.getTextContent(),
+      )}](${linkHref})`;
     } else {
-      return linkContent;
+      return `[${node.getTextContent()}](${linkHref})`;
     }
   },
   importRegExp:

I got stuck on making the import work. TextFormatTransformers (INLINE_CODE) are processed before TextMatchTransformers (LINK). As a result, the XXX ends up as a standalone text node, and TextMatchTransformers only apply to a single text node at a time, so the LINK does not match.

I don't see how to fix that within the current structure of the code. Considering LINK is builtin and there are special cases for other builtins, I think it would be reasonable to add handling for it, but I was hoping to get guidance / buy-in before going that route. IMO being able to style link text is a valid use case and lexical-markdown should support it.

robfig commented 11 months ago

Turned out to be easy to add special case code for processing links. The rest of the test suite continues to pass. Please have a look :pray: https://github.com/facebook/lexical/pull/5149