Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown links to markdown files will be broken if the file name does not contain space but is surrounded by <> #9614

Closed
6 of 7 tasks
axmmisaka opened this issue Dec 4, 2023 · 6 comments · Fixed by #9617
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@axmmisaka
Copy link
Contributor

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

Per CommonMark, [Haha](../hehe.mdx) and [Haha](<../hehe.mdx>) should be the same (if the path to file contains space only the latter will work, nevertheless.)
In Docusaurus 3, if the file path contains space only the latter syntax will work; however, if the file path does not contain space, only the former syntax will work and the latter will result in broken link.

image

Reproducible demo

No response

Steps to reproduce

  1. Open docusaurus.new
  2. In docs/tutorial-basics/congratulations.md,
- Have **5 more minutes**? Take a look at **[versioning](../tutorial-extras/manage-docs-versions.md)** and **[i18n](../tutorial-extras/translate-your-site.md)**.
+ Have **5 more minutes**? Take a look at **[versioning](<../tutorial-extras/manage-docs-versions.md>)** and **[i18n](<../tutorial-extras/translate-your-site.md>)**.
  1. yarn build

Expected behavior

As if nothing has changed

Actual behavior

Broken links found

Your environment

  • Public source code:
  • Public site URL:
  • Docusaurus version used:
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): NodeJS 20
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS):

Self-service

  • I'd be willing to fix this bug myself.
@axmmisaka axmmisaka added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Dec 4, 2023
@OzakIOne
Copy link
Collaborator

OzakIOne commented Dec 5, 2023

I've tried to create a repro here

Indeed link surrounded by <> will be broken however I could not get the link with space to work with <>

edit: Links with spaces will only work with <>

@axmmisaka
Copy link
Contributor Author

I've tried to create a repro here

Indeed link surrounded by <> will be broken, however I could not get the link with space to work with <>

I checked this reproduction and these two links work on my end, which is the expected behaviour? The second link looks weird in the editor though.

[page](./page.md)
[page](./page.md)
[space](./page space.md)
[space](<./page space.md>) <-
[space](page space.md)
[space](<page space.md>) <-
[space](./pagespace.md)

@slorber
Copy link
Collaborator

slorber commented Dec 5, 2023

hmmm 🤔 I thought we fixed this bug in #8867 but apparently it's back despite unit tests? Need to be investigated

@Josh-Cena
Copy link
Collaborator

I think the regex only matches links with both <> and spaces... Not sure exactly how

@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Dec 5, 2023
@axmmisaka
Copy link
Contributor Author

axmmisaka commented Dec 7, 2023

hmmm 🤔 I thought we fixed this bug in #8867 but apparently it's back despite unit tests? Need to be investigated

It was replaced by a plethora of big regexps which I found it powerful enough by looking at it while realising at the same time I am too stupid to comprehend it

let modifiedLine = line;
// Replace inline-style links or reference-style links e.g:
// This is [Document 1](doc1.md)
// [doc1]: doc1.md
const linkTitlePattern = '(?:\\s+(?:\'.*?\'|".*?"|\\(.*?\\)))?';
const linkSuffixPattern = '(?:\\?[^#>\\s]+)?(?:#[^>\\s]+)?';
const linkCapture = (forbidden: string) =>
`((?!https?://|@site/)[^${forbidden}#?]+)`;
const linkURLPattern = `(?:${linkCapture(
'()\\s',
)}${linkSuffixPattern}|<${linkCapture('>')}${linkSuffixPattern}>)`;
const linkPattern = new RegExp(
`\\[(?:(?!\\]\\().)*\\]\\(\\s*${linkURLPattern}${linkTitlePattern}\\s*\\)|^\\s*\\[[^[\\]]*[^[\\]\\s][^[\\]]*\\]:\\s*${linkURLPattern}${linkTitlePattern}$`,
'dgm',
);

Test shows that it could detect some but not all of similar cases:

image

@Josh-Cena
Copy link
Collaborator

@axmmisaka I've submitted a PR: #9617

The regex evolved over time. Now it's probably not a good idea to reason it from scratch😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants