fix(v2): linking to asset or external html page -> don't use history.push() (#3347)

* Rework markdown links to asset require processing + add test page

* implement pathname:// protocol / escape hatch at the Link level

* linking to assets: fix tests + avoid creating an useless nested paragraph

* fix assets linking doc

* attempt to fix windows e2e test

* try to fix windows errors

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

@@ -14,14 +14,14 @@ const stringifyObject = require('stringify-object');
 const slug = require('./remark/slug');
 const rightToc = require('./remark/rightToc');
 const transformImage = require('./remark/transformImage');
-const tranformAsset = require('./remark/transformAssets');
+const transformLinks = require('./remark/transformLinks');
 
 const DEFAULT_OPTIONS = {
   rehypePlugins: [],
   remarkPlugins: [emoji, slug, rightToc],
 };
 
-module.exports = async function (fileString) {
+module.exports = async function docusaurusMdxLoader(fileString) {
   const callback = this.async();
 
   const {data, content} = matter(fileString);
@@ -36,7 +36,7 @@ module.exports = async function (fileString) {
         {staticDir: reqOptions.staticDir, filePath: this.resourcePath},
       ],
       [
-        tranformAsset,
+        transformLinks,
         {staticDir: reqOptions.staticDir, filePath: this.resourcePath},
       ],
       ...(reqOptions.remarkPlugins || []),

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

@@ -1,33 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`transformAsset plugin fail if asset does not exist 1`] = `"Asset packages/docusaurus-mdx-loader/src/remark/transformAssets/__tests__/fixtures/doesNotExist.pdf used in packages/docusaurus-mdx-loader/src/remark/transformAssets/__tests__/fixtures/fail.md not found."`;
-
-exports[`transformAsset plugin fail if asset url is absent 1`] = `"Markdown link url is mandatory. filePath=packages/docusaurus-mdx-loader/src/remark/transformAssets/__tests__/fixtures/noUrl.md"`;
-
-exports[`transformAsset plugin pathname protocol 1`] = `
-"[asset](/asset/unchecked.pdf)
-"
-`;
-
-exports[`transformAsset plugin transform md links to <a /> 1`] = `
-"[asset](https://example.com/asset.pdf)
-
-<a  target=\\"_blank\\" href={require('./asset.pdf').default}  ></a>
-
-<a  target=\\"_blank\\" href={require('./asset.pdf').default}  >asset</a>
-
-[asset](asset.pdf \\"Title\\") ![seet](asset)
-
-## Heading
-
-\`\`\`md
-[asset](./asset.pdf)
-\`\`\`
-
-<a  target=\\"_blank\\" href={require('!file-loader!./asset.pdf').default}  >assets</a>
-
-[assets](/github/!file-loader!/assets.pdf)
-
-[asset](asset.pdf)
-"
-`;

packages/docusaurus-mdx-loader/src/remark/transformAssets/__tests__/fixtures/fail.md

@@ -1 +0,0 @@
-[asset](./doesNotExist.pdf)

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

@@ -1,91 +0,0 @@
-/**
- * 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 visit = require('unist-util-visit');
-const path = require('path');
-const url = require('url');
-const fs = require('fs-extra');
-
-// Needed to throw errors with computer-agnostic path messages
-// Absolute paths are too dependant of user FS
-function toRelativePath(filePath) {
-  return path.relative(process.cwd(), filePath);
-}
-
-async function ensureAssetFileExist(assetPath, sourceFilePath) {
-  const assetExists = await fs.exists(assetPath);
-  if (!assetExists) {
-    throw new Error(
-      `Asset ${toRelativePath(assetPath)} used in ${toRelativePath(
-        sourceFilePath,
-      )} not found.`,
-    );
-  }
-}
-
-async function processLinkNode(node, index, parent, {filePath}) {
-  if (!node.url) {
-    throw new Error(
-      `Markdown link url is mandatory. filePath=${toRelativePath(filePath)}`,
-    );
-  }
-  const parsedUrl = url.parse(node.url);
-  const assetPath = node.url;
-  if (parsedUrl.protocol) {
-    // pathname:// is an escape hatch,
-    // in case user does not want his assets to be converted to require calls going through webpack loader
-    // we don't have to document this for now,
-    // it's mostly to make next release less risky (2.0.0-alpha.59)
-    if (parsedUrl.protocol === 'pathname:') {
-      node.url = node.url.replace('pathname://', '');
-    }
-    return;
-  }
-  if (
-    assetPath.match(/#|.md|.mdx/) ||
-    path.isAbsolute(assetPath) ||
-    !path.extname(assetPath) ||
-    !assetPath.startsWith('.')
-  ) {
-    if (!assetPath.startsWith('!')) {
-      return;
-    }
-  }
-
-  const expectedAssetPath = path.join(
-    path.dirname(filePath),
-    assetPath.replace(/!.*!/, ''),
-  );
-  await ensureAssetFileExist(expectedAssetPath, filePath);
-
-  node.type = 'jsx';
-  node.value = `<a  target="_blank" ${
-    assetPath ? `href={require('${assetPath}').default}` : ''
-  } ${node.title ? `title={${node.title}}` : ''} >`;
-  const {children} = node;
-  delete node.children;
-
-  parent.children.splice(index + 1, 0, {
-    type: 'paragraph',
-    children,
-  });
-
-  parent.children.splice(index + 2, 0, {type: 'jsx', value: '</a>'});
-}
-
-const plugin = (options) => {
-  const transformer = async (root) => {
-    const promises = [];
-    visit(root, 'link', (node, index, parent) => {
-      promises.push(processLinkNode(node, index, parent, options));
-    });
-    await Promise.all(promises);
-  };
-  return transformer;
-};
-
-module.exports = plugin;

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

@@ -0,0 +1,35 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`transformAsset plugin fail if asset url is absent 1`] = `"Markdown link url is mandatory. filePath=packages/docusaurus-mdx-loader/src/remark/transformLinks/__tests__/fixtures/noUrl.md, title=null"`;
+
+exports[`transformAsset plugin pathname protocol 1`] = `
+"[asset](pathname:///asset/unchecked.pdf)
+"
+`;
+
+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}  >asset</a>
+
+<a target=\\"_blank\\" href={require('!file-loader?name=assets/files/[name]-[hash].[ext]!./asset.pdf').default} title={Title} >asset</a> ![seet](asset)
+
+## Heading
+
+\`\`\`md
+[asset](./asset.pdf)
+\`\`\`
+
+[assets](!file-loader!./asset.pdf)
+
+[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]!./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>
+"
+`;

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

@@ -17,3 +17,7 @@
 [assets](/github/!file-loader!/assets.pdf)
 
 [asset](asset.pdf)
+
+[staticAsset.pdf](/staticAsset.pdf)
+
+[@site/static/staticAsset.pdf](@site/static/staticAsset.pdf)

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

@@ -14,20 +14,18 @@ import slug from '../../slug';
 
 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(plugin, {...options, filePath: path})
+    .use(plugin, {...options, filePath: path, staticDir})
     .process(file);
 
   return result.toString();
 };
 
 describe('transformAsset plugin', () => {
-  test('fail if asset does not exist', async () => {
-    await expect(processFixture('fail')).rejects.toThrowErrorMatchingSnapshot();
-  });
   test('fail if asset url is absent', async () => {
     await expect(
       processFixture('noUrl'),

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

@@ -0,0 +1,154 @@
+/**
+ * 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 {posixPath} = require('@docusaurus/utils');
+
+const visit = require('unist-util-visit');
+const path = require('path');
+const url = require('url');
+const fs = require('fs-extra');
+const {getFileLoaderUtils} = require('@docusaurus/core/lib/webpack/utils');
+
+const {
+  loaders: {inlineMarkdownLinkFileLoader},
+} = getFileLoaderUtils();
+
+// Needed to throw errors with computer-agnostic path messages
+// Absolute paths are too dependant of user FS
+function toRelativePath(filePath) {
+  return path.relative(process.cwd(), filePath);
+}
+
+async function ensureAssetFileExist(fileSystemAssetPath, sourceFilePath) {
+  const assetExists = await fs.exists(fileSystemAssetPath);
+  if (!assetExists) {
+    throw new Error(
+      `Asset ${toRelativePath(fileSystemAssetPath)} used in ${toRelativePath(
+        sourceFilePath,
+      )} not found.`,
+    );
+  }
+}
+
+// transform the link node to a jsx link with a require() call
+function toAssetRequireNode({node, index, parent, filePath, requireAssetPath}) {
+  let relativeRequireAssetPath = posixPath(
+    path.relative(path.dirname(filePath), requireAssetPath),
+  );
+
+  // nodejs does not like require("assets/file.pdf")
+  relativeRequireAssetPath = relativeRequireAssetPath.startsWith('.')
+    ? relativeRequireAssetPath
+    : `./${relativeRequireAssetPath}`;
+
+  const hrefProp = `require('${inlineMarkdownLinkFileLoader}${relativeRequireAssetPath}').default`;
+
+  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>'});
+}
+
+// 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,
+}) {
+  const assetPath = node.url;
+
+  const hasSiteAlias = assetPath.startsWith('@site/');
+  const hasAssetLikeExtension =
+    path.extname(assetPath) && !assetPath.match(/#|.md|.mdx|.html/);
+
+  const looksLikeAssetLink = hasSiteAlias || hasAssetLikeExtension;
+
+  if (!looksLikeAssetLink) {
+    return;
+  }
+
+  function toAssetLinkNode(requireAssetPath) {
+    toAssetRequireNode({
+      node,
+      index,
+      parent,
+      filePath,
+      requireAssetPath,
+    });
+  }
+
+  if (assetPath.startsWith('@site/')) {
+    const siteDir = path.join(staticDir, '..');
+    const fileSystemAssetPath = path.join(
+      siteDir,
+      assetPath.replace('@site/', ''),
+    );
+    await ensureAssetFileExist(fileSystemAssetPath, filePath);
+    toAssetLinkNode(fileSystemAssetPath);
+  } else if (path.isAbsolute(assetPath)) {
+    const fileSystemAssetPath = path.join(staticDir, assetPath);
+    if (await fs.exists(fileSystemAssetPath)) {
+      toAssetLinkNode(fileSystemAssetPath);
+    }
+  } else {
+    const fileSystemAssetPath = path.join(path.dirname(filePath), assetPath);
+    if (await fs.exists(fileSystemAssetPath)) {
+      toAssetLinkNode(fileSystemAssetPath);
+    }
+  }
+}
+
+async function processLinkNode({node, index, parent, filePath, staticDir}) {
+  if (!node.url) {
+    throw new Error(
+      `Markdown link url is mandatory. filePath=${toRelativePath(
+        filePath,
+      )}, title=${node.title}`,
+    );
+  }
+
+  const parsedUrl = url.parse(node.url);
+  if (parsedUrl.protocol) {
+    return;
+  }
+
+  await convertToAssetLinkIfNeeded({
+    node,
+    index,
+    parent,
+    staticDir,
+    filePath,
+  });
+}
+
+const plugin = (options) => {
+  const transformer = async (root) => {
+    const promises = [];
+    visit(root, 'link', (node, index, parent) => {
+      promises.push(processLinkNode({node, index, parent, ...options}));
+    });
+    await Promise.all(promises);
+  };
+  return transformer;
+};
+
+module.exports = plugin;

packages/docusaurus/src/client/exports/Link.tsx

@@ -55,19 +55,29 @@ function Link({
   const targetLinkUnprefixed = to || href;
 
   function maybeAddBaseUrl(str: string) {
-    return shouldAddBaseUrlAutomatically(str)
+    return shouldAddBaseUrlAutomatically(str) ? withBaseUrl(str) : str;
-      ? withBaseUrl(str)
-      : targetLinkUnprefixed;
   }
 
+  const isInternal = isInternalUrl(targetLinkUnprefixed);
+
+  // pathname:// is a special "protocol" we use to tell Docusaurus link
+  // that a link is not "internal" and that we shouldn't use history.push()
+  // this is not ideal but a good enough escape hatch for now
+  // see https://github.com/facebook/docusaurus/issues/3309
+  // note: we want baseUrl to be appended (see issue for details)
+  // TODO read routes and automatically detect internal/external links?
+  const targetLinkWithoutPathnameProtocol = targetLinkUnprefixed?.replace(
+    'pathname://',
+    '',
+  );
+
   // TODO we should use ReactRouter basename feature instead!
   // Automatically apply base url in links that start with /
   const targetLink =
-    typeof targetLinkUnprefixed !== 'undefined'
+    typeof targetLinkWithoutPathnameProtocol !== 'undefined'
-      ? maybeAddBaseUrl(targetLinkUnprefixed)
+      ? maybeAddBaseUrl(targetLinkWithoutPathnameProtocol)
       : undefined;
 
-  const isInternal = isInternalUrl(targetLink);
   const preloaded = useRef(false);
   const LinkComponent = isNavLink ? NavLink : RRLink;
 

packages/docusaurus/src/webpack/utils.ts

@@ -173,17 +173,19 @@ export function compile(config: Configuration[]): Promise<void> {
   });
 }
 
+type AssetFolder = 'images' | 'files' | 'medias';
+
 // Inspired by https://github.com/gatsbyjs/gatsby/blob/8e6e021014da310b9cc7d02e58c9b3efe938c665/packages/gatsby/src/utils/webpack-utils.ts#L447
 export function getFileLoaderUtils() {
   // files/images < 10kb will be inlined as base64 strings directly in the html
   const urlLoaderLimit = 10000;
 
   // defines the path/pattern of the assets handled by webpack
-  const fileLoaderFileName = (folder: string) =>
+  const fileLoaderFileName = (folder: AssetFolder) =>
     `${STATIC_ASSETS_DIR_NAME}/${folder}/[name]-[hash].[ext]`;
 
   const loaders = {
-    file: (options: {folder: string}) => {
+    file: (options: {folder: AssetFolder}) => {
       return {
         loader: require.resolve(`file-loader`),
         options: {
@@ -191,7 +193,7 @@ export function getFileLoaderUtils() {
         },
       };
     },
-    url: (options: {folder: string}) => {
+    url: (options: {folder: AssetFolder}) => {
       return {
         loader: require.resolve(`url-loader`),
         options: {
@@ -210,6 +212,9 @@ export function getFileLoaderUtils() {
     inlineMarkdownImageFileLoader: `!url-loader?limit=${urlLoaderLimit}&name=${fileLoaderFileName(
       'images',
     )}&fallback=file-loader!`,
+    inlineMarkdownLinkFileLoader: `!file-loader?name=${fileLoaderFileName(
+      'files',
+    )}!`,
   };
 
   const rules = {

website/docs/markdown-features.mdx

@@ -948,12 +948,11 @@ Let's imagine the following file structure:
 # Some assets you want to use
 /website/docs/assets/docusaurus-asset-example-banner.png
 /website/docs/assets/docusaurus-asset-example-pdf.pdf
-/website/docs/assets/docusaurus-asset-example.xyz
 ```
 
-### Image assets
+### Images
 
-You can use images by requiring them and using an image tag through MDX:
+You can use images in Markdown, or by requiring them and using a JSX image tag:
 
 ```mdx
 # My markdown page
@@ -985,7 +984,7 @@ If you are using [@docusaurus/plugin-ideal-image](./using-plugins.md#docusaurusp
 
 :::
 
-### Common assets
+### Files
 
 In the same way, you can link to existing assets by requiring them and using the returned url in videos, links etc...
 
@@ -998,7 +997,7 @@ In the same way, you can link to existing assets by requiring them and using the
   Download this PDF !!!
 </a>
 
-or 
+or
 
 [Download this PDF using Markdown !!!](./assets/docusaurus-asset-example-pdf.pdf)
 ```
@@ -1009,53 +1008,18 @@ or
   Download this PDF !!!
 </a>
 
-
 [Download this PDF using Markdown !!!](./assets/docusaurus-asset-example-pdf.pdf)
 
-### Unknown assets
-
-This require behavior is not supported for all file extensions, but as an escape hatch you can use the special Webpack syntax to force the `file-loader` to kick-in:
-
-```mdx
-# My markdown page
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>
-
-or
-
-[Download this unknown file using Markdown](!file-loader!./assets/docusaurus-asset-example.xyz)
-```
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>
-
-[Download this unknown file using Markdown !!!](!file-loader!./assets/docusaurus-asset-example.xyz)
-
-
-```md
-[![](./assets/docusaurus-asset-example-banner.png)](./assets/docusaurus-asset-example-pdf.pdf)
-```
-
-[![](./assets/docusaurus-asset-example-banner.png)](./assets/docusaurus-asset-example-pdf.pdf)
-
 ### Inline SVGs
 
 Docusaurus support inlining SVGs out of the box.
 
 ```js
+import DocusaurusSvg from './docusaurus.svg';
 
-import DocusaurusSvg from "./docusaurus.svg"
+<DocusaurusSvg />;
-
-<DocusaurusSvg />
-
 ```
-import DocusaurusSvg from "@site/static/img/docusaurus.svg"
+
+import DocusaurusSvg from '@site/static/img/docusaurus.svg';
 
 <DocusaurusSvg />

website/src/pages/markdown-tests.md

@@ -0,0 +1,31 @@
+# Markdown tests
+
+This is a test page to see if Docusaurus markdown features are working properly
+
+## Linking to assets
+
+See [#3337](https://github.com/facebook/docusaurus/issues/3337)
+
+- [/dogfooding/someFile.pdf](/dogfooding/someFile.pdf)
+
+- [/dogfooding/someFile.xyz](/dogfooding/someFile.xyz)
+
+- [../../static/dogfooding/someFile.pdf](../../static/dogfooding/someFile.pdf)
+
+- [../../static/dogfooding/someFile.xyz](../../static/dogfooding/someFile.xyz)
+
+- [@site/static/dogfooding/someFile.pdf](@site/static/dogfooding/someFile.pdf)
+
+- [@site/static/dogfooding/someFile.xyz](@site/static/dogfooding/someFile.xyz)
+
+## Linking to non-SPA page hosted within website
+
+See [#3309](https://github.com/facebook/docusaurus/issues/3309)
+
+- [pathname:///dogfooding/javadoc](pathname:///dogfooding/javadoc)
+
+- [pathname:///dogfooding/javadoc/index.html](pathname:///dogfooding/javadoc/index.html)
+
+- [pathname://../dogfooding/javadoc](pathname://../dogfooding/javadoc)
+
+- [pathname://../dogfooding/javadoc/index.html](pathname://../dogfooding/javadoc/index.html)

website/static/dogfooding/javadoc/index.html

@@ -0,0 +1,6 @@
+<html>
+  <body>
+    static HTML file used for testing we should be able to link to it with a
+    markdown link see also https://github.com/facebook/docusaurus/issues/3309
+  </body>
+</html>

website/versioned_docs/version-2.0.0-alpha.60/markdown-features.mdx

@@ -948,7 +948,6 @@ Let's imagine the following file structure:
 # Some assets you want to use
 /website/docs/assets/docusaurus-asset-example-banner.png
 /website/docs/assets/docusaurus-asset-example-pdf.pdf
-/website/docs/assets/docusaurus-asset-example.xyz
 ```
 
 ### Image assets
@@ -1004,23 +1003,3 @@ In the same way, you can link to existing assets by requiring them and using the
   href={require('./assets/docusaurus-asset-example-pdf.pdf').default}>
   Download this PDF !!!
 </a>
-
-### Unknown assets
-
-This require behavior is not supported for all file extensions, but as an escape hatch you can use the special Webpack syntax to force the `file-loader` to kick-in:
-
-```mdx
-# My markdown page
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>
-```
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>

website/versioned_docs/version-2.0.0-alpha.61/markdown-features.mdx

@@ -948,7 +948,6 @@ Let's imagine the following file structure:
 # Some assets you want to use
 /website/docs/assets/docusaurus-asset-example-banner.png
 /website/docs/assets/docusaurus-asset-example-pdf.pdf
-/website/docs/assets/docusaurus-asset-example.xyz
 ```
 
 ### Image assets
@@ -1004,23 +1003,3 @@ In the same way, you can link to existing assets by requiring them and using the
   href={require('./assets/docusaurus-asset-example-pdf.pdf').default}>
   Download this PDF !!!
 </a>
-
-### Unknown assets
-
-This require behavior is not supported for all file extensions, but as an escape hatch you can use the special Webpack syntax to force the `file-loader` to kick-in:
-
-```mdx
-# My markdown page
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>
-```
-
-<a
-  target="_blank"
-  href={require('!file-loader!./assets/docusaurus-asset-example.xyz').default}>
-  Download this unknown file !!!
-</a>