fix(v2): handle multiple asset links in one line properly (#3653)

* fix(v2): handle multiple asset links in one line properly

* Fixes and improvements

* Make TypeScript happy

* Use relative path for image link

* Add example for JSX element inside asset link

packages/docusaurus-mdx-loader/src/remark/rightToc/search.js

@@ -9,7 +9,7 @@
 
 const toString = require('mdast-util-to-string');
 const visit = require('unist-util-visit');
-const escapeHtml = require('escape-html');
+const {toValue} = require('../utils');
 
 /** @typedef {import('@docusaurus/types').MarkdownRightTableOfContents} TOC */
 /** @typedef {import('unist').Node} Node */
@@ -23,33 +23,6 @@ const escapeHtml = require('escape-html');
  * @property {StringValuedNode[]} children
  */
 
-// https://github.com/syntax-tree/mdast#heading
-/**
- * @param {StringValuedNode | undefined} node
- * @returns {string}
- */
-function toValue(node) {
-  if (node && node.type) {
-    switch (node.type) {
-      case 'text':
-        return escapeHtml(node.value);
-      case 'heading':
-        return node.children.map(toValue).join('');
-      case 'inlineCode':
-        return `<code>${escapeHtml(node.value)}</code>`;
-      case 'emphasis':
-        return `<em>${node.children.map(toValue).join('')}</em>`;
-      case 'strong':
-        return `<strong>${node.children.map(toValue).join('')}</strong>`;
-      case 'delete':
-        return `<del>${node.children.map(toValue).join('')}</del>`;
-      default:
-    }
-  }
-
-  return toString(node);
-}
-
 // Visit all headings. We `slug` all headings (to account for
 // duplicates), but only take h2 and h3 headings.
 /**

packages/docusaurus-mdx-loader/src/remark/transformImage/__tests__/__snapshots__/index.test.js.snap

@@ -12,18 +12,18 @@ exports[`transformImage plugin pathname protocol 1`] = `
 exports[`transformImage plugin transform md images to <img /> 1`] = `
 "![img](https://example.com/img.png)
 
-<img  src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default}  />
+<img src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default} />
 
-<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default}  />
+<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default} />
 
-<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default} title={\\"Title\\"} /> <img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!packages/docusaurus-mdx-loader/src/remark/transformImage/__tests__/fixtures/img.png\\").default}  />
+<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default} title=\\"Title\\" /> <img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!packages/docusaurus-mdx-loader/src/remark/transformImage/__tests__/fixtures/img.png\\").default} />
 
 ## Heading
 
 \`\`\`md
 ![img](./img.png)
 \`\`\`
 
-<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default}  />
+<img alt={\\"img\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./img.png\\").default} />
 "
 `;

packages/docusaurus-mdx-loader/src/remark/transformImage/index.js

@@ -9,6 +9,7 @@ const visit = require('unist-util-visit');
 const path = require('path');
 const url = require('url');
 const fs = require('fs-extra');
+const escapeHtml = require('escape-html');
 const {getFileLoaderUtils} = require('@docusaurus/core/lib/webpack/utils');
 const {posixPath} = require('@docusaurus/utils');
 
@@ -18,11 +19,11 @@ const {
 
 const createJSX = (node, pathUrl) => {
   node.type = 'jsx';
-  node.value = `<img ${node.alt ? `alt={"${node.alt}"}` : ''} ${
+  node.value = `<img ${node.alt ? `alt={"${node.alt}"} ` : ''}${
     node.url
       ? `src={require("${inlineMarkdownImageFileLoader}${pathUrl}").default}`
       : ''
-  } ${node.title ? `title={"${node.title}"}` : ''} />`;
+  }${node.title ? ` title="${escapeHtml(node.title)}"` : ''} />`;
 
   if (node.url) {
     delete node.url;

packages/docusaurus-mdx-loader/src/remark/transformLinks/__tests__/__snapshots__/index.test.js.snap

@@ -10,11 +10,11 @@ exports[`transformAsset plugin pathname protocol 1`] = `
 exports[`transformAsset plugin transform md links to <a /> 1`] = `
 "[asset](https://example.com/asset.pdf)
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}  ></a>
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}></a>
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}  >asset</a>
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}>asset</a>
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default} title={Title} >asset</a> ![seet](asset)
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}title=\\"Title\\">asset</a>
 
 ## Heading
 
@@ -26,10 +26,16 @@ exports[`transformAsset plugin transform md links to <a /> 1`] = `
 
 [assets](/github/!file-loader!/assets.pdf)
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}  >asset</a>
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}>asset</a>
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}  >staticAsset.pdf</a>
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}>staticAsset.pdf</a>
 
-<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}  >@site/static/staticAsset.pdf</a>
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}>@site/static/staticAsset.pdf</a>
+
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}>Just staticAsset.pdf</a>, and <a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}><strong>awesome</strong> staticAsset 2.pdf &#39;It is really &quot;AWESOME&quot;&#39;</a>, but also <a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAsset.pdf').default}>coded <code>staticAsset 3.pdf</code></a>
+
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./static/staticAssetImage.png').default}><img alt={\\"Clickable Docusaurus logo\\"} src={require(\\"!url-loader?limit=10000&name=assets/images/[name]-[hash].[ext]&fallback=file-loader!./static/staticAssetImage.png\\").default} /></a>
+
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default}><span style={{color: \\"red\\"}}>Stylized link to asset file</span></a>
 "
 `;

packages/docusaurus-mdx-loader/src/remark/transformLinks/__tests__/fixtures/asset.md

@@ -4,7 +4,7 @@
 
 [asset](./asset.pdf)
 
-[asset](asset.pdf 'Title') ![seet](asset)
+[asset](asset.pdf 'Title')
 
 ## Heading
 
@@ -21,3 +21,9 @@
 [staticAsset.pdf](/staticAsset.pdf)
 
 [@site/static/staticAsset.pdf](@site/static/staticAsset.pdf)
+
+[Just staticAsset.pdf](/staticAsset.pdf), and [**awesome** staticAsset 2.pdf 'It is really "AWESOME"'](/staticAsset.pdf), but also [coded `staticAsset 3.pdf`](/staticAsset.pdf)
+
+[![Clickable Docusaurus logo](./static/staticAssetImage.png)](/staticAssetImage.png)
+
+[<span style={{color: "red"}}>Stylized link to asset file</span>](./asset.pdf)

packages/docusaurus-mdx-loader/src/remark/transformLinks/__tests__/index.test.js

@@ -10,15 +10,15 @@ import remark from 'remark';
 import mdx from 'remark-mdx';
 import vfile from 'to-vfile';
 import plugin from '..';
-import slug from '../../slug';
+import transformImage from '../../transformImage';
 
 const processFixture = async (name, options) => {
   const path = join(__dirname, 'fixtures', `${name}.md`);
   const staticDir = join(__dirname, 'fixtures', 'static');
   const file = await vfile.read(path);
   const result = await remark()
-    .use(slug)
     .use(mdx)
+    .use(transformImage, {...options, filePath: path, staticDir})
     .use(plugin, {...options, filePath: path, staticDir})
     .process(file);
 

packages/docusaurus-mdx-loader/src/remark/transformLinks/index.js

@@ -11,6 +11,8 @@ const visit = require('unist-util-visit');
 const path = require('path');
 const url = require('url');
 const fs = require('fs-extra');
+const escapeHtml = require('escape-html');
+const {toValue} = require('../utils');
 const {getFileLoaderUtils} = require('@docusaurus/core/lib/webpack/utils');
 
 const {
@@ -35,7 +37,9 @@ async function ensureAssetFileExist(fileSystemAssetPath, sourceFilePath) {
 }
 
 // transform the link node to a jsx link with a require() call
-function toAssetRequireNode({node, index, parent, filePath, requireAssetPath}) {
+function toAssetRequireNode({node, filePath, requireAssetPath}) {
+  /* eslint-disable no-param-reassign */
+
   let relativeRequireAssetPath = posixPath(
     path.relative(path.dirname(filePath), requireAssetPath),
   );
@@ -45,35 +49,18 @@ function toAssetRequireNode({node, index, parent, filePath, requireAssetPath}) {
     ? relativeRequireAssetPath
     : `./${relativeRequireAssetPath}`;
 
-  const hrefProp = `require('${inlineMarkdownLinkFileLoader}${relativeRequireAssetPath}').default`;
+  const href = `require('${inlineMarkdownLinkFileLoader}${relativeRequireAssetPath}').default`;
+  const children = (node.children || []).map((n) => toValue(n)).join('');
+  const title = node.title ? `title="${escapeHtml(node.title)}"` : '';
 
   node.type = 'jsx';
-
-  node.value = `<a target="_blank" href={${hrefProp}} ${
-    node.title ? `title={${node.title}}` : ''
-  } >`;
-
-  const linkText = (node.children[0] && node.children[0].value) || '';
-  delete node.children;
-
-  parent.children.splice(index + 1, 0, {
-    type: 'text',
-    value: linkText,
-  });
-
-  parent.children.splice(index + 2, 0, {type: 'jsx', value: '</a>'});
+  node.value = `<a target="_blank" href={${href}}${title}>${children}</a>`;
 }
 
 // If the link looks like an asset link, we'll link to the asset,
 // and use a require("assetUrl") (using webpack url-loader/file-loader)
 // instead of navigating to such link
-async function convertToAssetLinkIfNeeded({
-  node,
-  index,
-  parent,
-  staticDir,
-  filePath,
-}) {
+async function convertToAssetLinkIfNeeded({node, staticDir, filePath}) {
   const assetPath = node.url;
 
   const hasSiteAlias = assetPath.startsWith('@site/');
@@ -89,8 +76,6 @@ async function convertToAssetLinkIfNeeded({
   function toAssetLinkNode(requireAssetPath) {
     toAssetRequireNode({
       node,
-      index,
-      parent,
       filePath,
       requireAssetPath,
     });
@@ -117,7 +102,7 @@ async function convertToAssetLinkIfNeeded({
   }
 }
 
-async function processLinkNode({node, index, parent, filePath, staticDir}) {
+async function processLinkNode({node, _index, _parent, filePath, staticDir}) {
   if (!node.url) {
     // try to improve error feedback
     // see https://github.com/facebook/docusaurus/issues/3309#issuecomment-690371675
@@ -139,8 +124,6 @@ async function processLinkNode({node, index, parent, filePath, staticDir}) {
 
   await convertToAssetLinkIfNeeded({
     node,
-    index,
-    parent,
     staticDir,
     filePath,
   });

packages/docusaurus-mdx-loader/src/remark/utils/index.js

@@ -0,0 +1,39 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+const escapeHtml = require('escape-html');
+const toString = require('mdast-util-to-string');
+
+/**
+ * @param {StringValuedNode | undefined} node
+ * @returns {string}
+ */
+function toValue(node) {
+  if (node && node.type) {
+    switch (node.type) {
+      case 'text':
+        return escapeHtml(node.value);
+      case 'heading':
+        return node.children.map(toValue).join('');
+      case 'inlineCode':
+        return `<code>${escapeHtml(node.value)}</code>`;
+      case 'emphasis':
+        return `<em>${node.children.map(toValue).join('')}</em>`;
+      case 'strong':
+        return `<strong>${node.children.map(toValue).join('')}</strong>`;
+      case 'delete':
+        return `<del>${node.children.map(toValue).join('')}</del>`;
+      default:
+    }
+  }
+
+  return toString(node);
+}
+
+module.exports = {
+  toValue,
+};