fix(v2): remove trailing slash from router links

[lex111]

Motivation

Resolves #2394

Some hosting providers automatically add a trailing slash to the website links, which leads to the fact that some functionality stops working correctly. For example, in such cases, relative links to other Markdown files do not work. Therefore, to avoid this, we can remove the trailing slash from the internal (which are processed by the router) links before they get to the router. This is safe operation, because we usually get links without a trailing slash, and we can safely delete it.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

1. Create a Markdown file in a new subdirectory so that it refers to the file in the parent directory OR follow instructions in To Reproduce section in the issue Hosting environments that add a trailing back slash break page links #2394

docs/main.md:

---
id: main
title: Main page
---

Some page...

docs/sub/index.md:

---
id: sub
title: Subcategory page
---

See [main page](../main).

sidebars.js

module.exports = {
  docs: {
    Root: ['main', 'sub/sub']
  }
}

Now, when navigating to the docs/sub/sub path without a trailing slash and with it (docs/sub/sub/), the result should be the same:

<p><a href="/docs/main">See main page</a>.</p>

Earlier, when navigating a path with a trailing slash, the result was as follows:

<p><a href="/docs/sub/main">See main page</a>.</p>

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

[docusaurus-bot]

Deploy preview for docusaurus-2 ready!

Built with commit af2eb48

https://deploy-preview-2465--docusaurus-2.netlify.com

[yangshun]

Is this truly safe? What if users were doing ../../ to circumvent the issue and now that we strip the trailing slash, their path will be wrong.

But I agree we should try to normalize and either add trailing slashes to all or strip all the trailing slashes.

[lex111]

I do not quite understand what you mean, someone will have links like `[link](../../)`? In this PR, an extra (redundant) slash in the current path is removed if the hosting provider has automatically added it (eg. Netlify). Therefore, it should not break anything. пт, 27 мар. 2020 г. в 21:44, Yangshun Tay <[email protected]>:

…

Is this truly safe? What if users were doing ../../ to circumvent the issue and now that we strip the trailing slash, their path will be wrong. But I agree we should try to normalize and either add trailing slashes to all or strip all the trailing slashes. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2465 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBUIOZMEU2IUV37XVEAODDRJTXZLANCNFSM4LTI6OIA> .

[lex111]

I checked this change with links of more than one nesting level ([Link](../../id)), it is works fine.

[yangshun]

Oh great, I'll do more testing first.

[yangshun]

I'm not very sure what this PR actually does. If we want to modify the link, why aren't we stripped it from targetLink and modifying the window.location instead? Also, causing side effects on the window in the render path of a component doesn't seem like a great idea IMO, not sure if intended.

[lex111]

What side effects do you see here? Removing the trailing slash (which was added by the hosting provider), we process the page as originally "intended" (i.e. without a slash). Therefore, if you call working linking a side effect, then yes, in this case it is.
We only change the path of current location, just as if you opened this link without trailing slash.

[yangshun]

The side effect is that you're modifying window.location (which a a global environment) within a component. window.location is not owned by the component. What if there's another similar component in some part of the code base that for some reason wants to add trailing slashes to window.location? Wouldn't both components be competing for control over window.location? What's worse is that you can't guarantee yours will always "win".

Why don't we modify the targetLink instead and leave the window.location alone?

[lex111]

Not sure if this will help in the issue solution. Well then I close it, maybe later it will be resolved by someone else.