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

Proposal: Prepend base url to absolute markdown links #3278

Closed
jknoxville opened this issue Aug 13, 2020 · 8 comments · Fixed by #3283
Closed

Proposal: Prepend base url to absolute markdown links #3278

jknoxville opened this issue Aug 13, 2020 · 8 comments · Fixed by #3283
Labels
mlh Major League Hacking Fellowship proposal This issue is a proposal, usually non-trivial change

Comments

@jknoxville
Copy link
Contributor

💥 Proposal

Current state

At the moment you have two options for linking to another page on your docusaurus V2 site:

  • Use a relative link, e.g. [click here](./destination)
  • Write html with an absolute reference, and the useBaseUrl hook, e.g.
import useBaseUrl from '@docusaurus/useBaseUrl';
import Link from '@docusaurus/Link';

<Link to={useBaseUrl('/docs/destination')}>click here</Link>

Restricting yourself to using the first method is fine if you know that you need to do this, but one advantage of markdown is that you can easily write it without having to worry much about technicalities. To someone familiar with markdown but not necessarily docusaurus, [click here](/docs/destination) looks like something that should work, but unfortunately it won't if the site uses a baseUrl, causing confusion - especially if they write the docs with a base-url-less config, and the site then gets hosted with a base-url.

Given that the power of docusaurus is in making documentation easy, and the second option requires familiarity with javascript imports and knowledge of docusaurus, I think we can rule out the second option as a first-class way to link between pages.

There are also other advantages to absolute links, such as portability - you can copy and paste the link between different pages without concerns.

Suggestion

I'm wondering if we should automatically prepend the base url to any (domain-less) markdown links, with the goal of making it "just work" in as many scenarios as possible.

So [click here](/docs/destination) would resolve to /the/base/url/docs/destination

Downside

Currently, you can link to pages "outside" of the baseUrl, but hosted on the same domain, using absolute links of the above form. They don't get the base url at the moment, so these use cases would break.

However, I expect that those use cases are much more rare than linking to pages inside the base url. In addition, by including the domain name, you would still be able to get this behavior, e.g. [click here](https://site.com/something).

This would mean that all link types (relative, absolute inside base, absolute outside base) are still achievable without needing to use HTML or JS (which isn't the case at the moment).

Images

The above is written with hyperlinks in mind, but I think the same should apply to images too.

Have you read the Contributing Guidelines on issues?

Yes

@jknoxville jknoxville added status: needs triage This issue has not been triaged by maintainers proposal This issue is a proposal, usually non-trivial change labels Aug 13, 2020
@anshulrgoyal
Copy link
Contributor

I can update my transformAssets remark plugin to perform this operation but I have to check with @slorber first.

@anshulrgoyal
Copy link
Contributor

anshulrgoyal commented Aug 13, 2020

For images, I would suggest using relative paths.

![image](./path/to/image)

@slorber
Copy link
Collaborator

slorber commented Aug 14, 2020

Hey @jknoxville

So click here would resolve to /the/base/url/docs/destination

Actually, we already do this. I added this change recently, and such a link will automatically add the baseurl.

I don't like the "useBaseUrl()" hook very much, and it's something I want to get rid of, so that you can move your site from / to /baseUrl without having to add any code.

You will notice that in any PR we now deploy Docusaurus website under a baseURL, this ensures that we dogfood using baseUrl.

Downside

I've added a "secret" escape hatch for that just in case: [click here](pathname:///destination), this allows to link to /destination, while your site is at /baseUrl


I've just tried the following:

[absolute link](/docs/installation)

[md link](./installation.md)

![img](../static/img/docusaurus.png)

![img](/img/docusaurus.png)

Only the last image does not seem to apply the baseUrl.

Note: relative image paths go through webpack file loaders so they apply the baseUrl automatically through webpack config. Here it's not a very good example because linking to ../static is likely to break if the site becomes versioned, but relative image paths are good if you want to "colocate" an image in the docs folder, so that this image is versioned alongside the docs.

@anshulrgoyal we should add this baseUrl for absolute image paths in the existing imageTransformer code.
I think we should also go through the webpack loader for absolute image paths, currently we only check that the image exist in /static

Going through webpack loader means the asset will be served at /baseUrl/assets/img/myFileName-hash.png which has pros (cachability) and cons (not keeping the original filename, as we add a hash).

Going consistently through the webpack loader seems to me better

@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Aug 14, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 14, 2020

BTW, just saw that we have a broken image on the deploy previews :)

https://deploy-preview-3274--docusaurus-2.netlify.app/build/blog/2017/12/14/introducing-docusaurus

Whether we add baseUrl or use webpack file-loader, this image should get fixed by the solution :)

@jknoxville
Copy link
Contributor Author

Wow, best response I could have hoped for! Thanks @slorber !

Just tried upgrading to 61 and the absolute link does indeed work as expected, brilliant!

@anshulrgoyal
Copy link
Contributor

I will add support to convert images with the absolute path to go through webpack-loader

@slorber
Copy link
Collaborator

slorber commented Aug 14, 2020

great to know it works for you :)

thanks @anshulrgoyal

@anshulrgoyal
Copy link
Contributor

anshulrgoyal commented Aug 14, 2020

@slorber Can u add MLH label to this issue and I have created a PR for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlh Major League Hacking Fellowship proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants