fix: support relative markdown linking #1138

endiliey · 2018-12-03T11:42:36Z

Motivation

Upon finding, we actually never really support relative markdown linking since initial release. Things has been working so far because lot of people refer to doc by doing [test](dir1/test.md], however [test](../test.md) will not work because we never really properly resolve it when replacing the markdown linking

The key change here is this

mdToHtml[resolve(metadata.source, mdMatch[1])];

Oh another thing is we no longer replace markdown linking in fenced blocks 👍

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Clone the Redux repo at https://github.com/reduxjs/redux
Check out the docs-rebuild branch
Copy this PR patched file at Docusaurus/v1/lib/server/docs.js to redux/website/node_modules/Docusaurus/lib/server/docs.js
Start Docusaurus

Before

After

cc @markerikson

docusaurus-bot · 2018-12-03T11:46:38Z

Deploy preview for docusaurus-preview ready!

Built with commit 552716c

https://deploy-preview-1138--docusaurus-preview.netlify.com

markerikson · 2018-12-03T16:33:22Z

One other question: will this handle Markdown file links that also have an anchor attached to them, like [link text](./SomeFile.md#some-anchor) ? We've got a bunch of those.

I have to say I'm kinda surprised that not many other people have run across this issue yet :)

endiliey · 2018-12-03T16:39:00Z

@markerikson

Yes it will. Actually everyone is referencing stuff with [alt text](dir1/filename.md) and never with relative link. Everytime anyone encountered this on discord, I'd point out doing this instead of [alt text](../filename.md]

What it means by https://docusaurus.io/docs/en/doc-markdown#linking-other-documents is that we need to reference it based on docs folder.

It's been like that since the very first release of Docusaurus 😮

But anyway, seems that it's not hard to implement the relative linking

markerikson · 2018-12-03T17:01:53Z

Cool. If you can push this out real soon (like, today maybe? :) ), I think this might let me move forward with the Redux docs conversion.

tibdex · 2018-12-03T20:59:58Z

Thanks for fixing this @endiliey! Would it be possible to also fix relative links for assets in this PR?

A link like ![nice picture](../assets/picture.png) would still not work with the current suggested changes.

The issue was already reported in #705 (see #705 (comment)).

Basically, the goal is to be able to use the same link paths (for markdown documents and assets) both when browsing the source in GitHub (or the Markdown Preview in VS Code) and when building the docs with Docusaurus.

markerikson · 2018-12-04T02:31:19Z

I just pasted this modified file into my current Redux local repo Docusaurus install.

The good news is that it does fix the original issue I described in #1137. Relative Markdown page links now work, including anchors.

The bad news is that I found one other pattern we use heavily in our current docs files: absolute paths from the root of the repo. For example, here's one from an FAQ entry that links to another docs page:

- [Introduction: Motivation](/docs/introduction/Motivation.md)

I think part of the issue was wanting the links to work okay when rendered by Github.

Is that something that's easily fixable? Or would we be better off trying to convert all docs links to be relative?

endiliey · 2018-12-04T02:59:04Z

@markerikson we don't recommend that absolute path because in versioned docs (which is the core of Docusaurus), e.g: if you are in v1.0.0, you would want to refer to v1.0.0 versioned docs

I recommend converting it to how Docusaurus works. If in the future Redux is going to version their docs like how babel does, its going to work flawlessly

markerikson · 2018-12-04T16:16:30Z

Gotcha. Yeah, I went ahead and converted all the absolute links to relative ones last night.

Can you put this out as a 1.6.1 release so that we can use it? Thanks!

reintroducing · 2018-12-06T17:13:16Z

@endiliey Any word on releasing 1.6.1? I want to see if it fixed the issues I mentioned in the issue ticket as well. Thanks.

endiliey · 2018-12-07T16:17:20Z

Sorry for the delay ! Just released 1.6.1 to npm

reintroducing · 2018-12-07T17:20:47Z

@endiliey thank you. unfortunately after upgrading, my issue does not seem resolved. the pertinent link is here: https://spothero.com/uniform/ace/docs/upgrading-v5/#code-splitting

if you click on the guide on code splitting link, you'll see it 404s. The code that drives that link is A full [guide on code splitting](docs-code-splitting) is available.

The docs-code-splitting.md file lives at the same level as upgrading-v5.md. cleanUrl: true, is in the siteConfig.js.

Am I misunderstanding something? Mis-linking?

reintroducing · 2018-12-07T17:23:36Z

Its also worth noting that after the upgrade I noticed that when I went to upload my files to S3 almost all of them said "No change", almost as if it was not re-run at all.

JoelMarcey · 2018-12-07T17:26:19Z

Not sure this helps you here, but we do have an issue with 1.6.1 - #1149

Fixed by: #1150

reintroducing · 2018-12-07T17:29:28Z

Actually, i'm getting a different issue when i ran development mode (i was going straight to production). I'm seeing this:

TypeError: replaceAssetsLink is not a function
    at Object.replaceAssetsLink [as getMarkup] (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/docusaurus/lib/server/docs.js:114:13)
    at getMarkup (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/docusaurus/lib/server/server.js:125:19)
    at Layer.handle [as handle_request] (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/layer.js:95:5)
    at /Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/index.js:281:22
    at Function.process_params (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/router/index.js:275:10)
    at expressInit (/Users/mprzybylski/Work/SpotHero/ace/website/node_modules/express/lib/middleware/init.js:40:5)

reintroducing · 2018-12-07T17:30:32Z

Disregard, disabling babel cache brought me back to the issue being discussed there. That being said, I'm not sure if that fix will fix my issue as well.

endiliey · 2018-12-10T00:50:57Z

1.6.2 should fix it @reintroducing

Let us know how it goes

endiliey · 2018-12-10T00:51:50Z

@tibdex sorry for the late reply. Didnt notice the comment.

Seems like a good idea, I will probably push a fix for relative linking on assets for next release

reintroducing · 2018-12-10T15:24:45Z

@endiliey Just updated to 1.6.2 and it does not seem to have fixed the issue still :\

endiliey · 2018-12-10T17:29:43Z

Send a reproducible repo in for bug report

reintroducing · 2018-12-10T17:33:13Z

@endiliey there are a few issues with that. 1 being is that this only happens when its deployed to production and 2 is that this is pretty heavily tied to a private repo we have. i've tried to outline our setup above and i'd be happy to try and provide any info possible to help troubleshoot this. i'd also be happy to hop on a screenshare or similar if you wanted to live debug it, i just am not sure that trying to re-create this repo without actually pushing to production in this environment would be helpful.

endiliey · 2018-12-10T17:36:17Z

The production and development consume the same api for replaceAssetsLink. Either way a yarn build on local repo is in fact a production build so we can easily debug it if there is a reproducible repo.

If you are deploying via a CI, node_modules cache is usually quite the problem.

It's hard to fix it without any clonable & reproducible repo. I've at least confirmed with > 5 repo and it seems that 1.6.2 fixed the relative linking issue.

Edit:
Appreciate if you can try reproducing it with some fresh project and filing it so we'll fix it right away 😄

reintroducing · 2018-12-10T17:40:33Z

@endiliey if i provide you with a repo that is just the docusaurus setup we have for that project, would that suffice?

endiliey · 2018-12-10T17:41:42Z

As long as we can reproduce the bug, that will be fine!

Sorry for the trouble but that'd be helpful. There are lot of cases where we can't even reproduce the bug so we can't even fix it 😊

reintroducing · 2018-12-10T17:48:56Z

i went ahead and put the pertinent code on dropbox here: https://www.dropbox.com/s/eyqygyc0kv1cx8y/docusaurus-bug.zip?dl=0

As you can see, locally the issue i described here (#1138 (comment)) is fine, but when it goes to prod, which you can hit in the url provided in that comment, the result is not correct. let me know if you need anything else.

reintroducing · 2019-01-14T15:42:35Z

@endiliey Just coming back to this and was wondering if you had a chance to look over the files I provided?

endiliey added 2 commits December 3, 2018 19:33

fix: relative markdown reference linking

6b3d3d2

prioritize relative linking resolve

552716c

facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 3, 2018

endiliey requested a review from yangshun December 3, 2018 11:59

endiliey changed the title ~~fix: relative markdown reference linking~~ fix: support relative markdown linking Dec 3, 2018

endiliey merged commit 9360739 into facebook:master Dec 4, 2018

endiliey deleted the mdlink branch December 4, 2018 04:26

endiliey mentioned this pull request Mar 5, 2019

Referencing documents are not working if .md extension is used #1261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: support relative markdown linking #1138

fix: support relative markdown linking #1138

endiliey commented Dec 3, 2018 •

edited

docusaurus-bot commented Dec 3, 2018

markerikson commented Dec 3, 2018 •

edited

endiliey commented Dec 3, 2018

markerikson commented Dec 3, 2018

tibdex commented Dec 3, 2018

markerikson commented Dec 4, 2018 •

edited

endiliey commented Dec 4, 2018

markerikson commented Dec 4, 2018

reintroducing commented Dec 6, 2018

endiliey commented Dec 7, 2018

reintroducing commented Dec 7, 2018

reintroducing commented Dec 7, 2018

JoelMarcey commented Dec 7, 2018

reintroducing commented Dec 7, 2018

reintroducing commented Dec 7, 2018 •

edited

endiliey commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018 •

edited

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

reintroducing commented Jan 14, 2019

fix: support relative markdown linking #1138

fix: support relative markdown linking #1138

Conversation

endiliey commented Dec 3, 2018 • edited

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

docusaurus-bot commented Dec 3, 2018

markerikson commented Dec 3, 2018 • edited

endiliey commented Dec 3, 2018

markerikson commented Dec 3, 2018

tibdex commented Dec 3, 2018

markerikson commented Dec 4, 2018 • edited

endiliey commented Dec 4, 2018

markerikson commented Dec 4, 2018

reintroducing commented Dec 6, 2018

endiliey commented Dec 7, 2018

reintroducing commented Dec 7, 2018

reintroducing commented Dec 7, 2018

JoelMarcey commented Dec 7, 2018

reintroducing commented Dec 7, 2018

reintroducing commented Dec 7, 2018 • edited

endiliey commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018 • edited

reintroducing commented Dec 10, 2018

endiliey commented Dec 10, 2018

reintroducing commented Dec 10, 2018

reintroducing commented Jan 14, 2019

endiliey commented Dec 3, 2018 •

edited

markerikson commented Dec 3, 2018 •

edited

markerikson commented Dec 4, 2018 •

edited

reintroducing commented Dec 7, 2018 •

edited

endiliey commented Dec 10, 2018 •

edited