docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

Unability to link to an internal non-SPA link

Open slorber opened this issue 5 years ago â€Ē 27 comments

🐛 Bug Report

Related to https://github.com/facebook/docusaurus/issues/3303#issuecomment-676541070

Imagine you have /static/javadoc/index.html

You should be able to link to this page that is not managed by Docusaurus, using:

<Link to="/javadoc">Javadoc</Link>

or

[javadoc](/javadoc)

Yet, the link uses history.push() as it thinks it's an SPA internal link, and we end up with the 404 as no SPA route is found.

It works after a refresh though, or if we use an URL with the http/https protocol.


Not sure how to solve this problem,

Maybe we should read the routes file directly on link, and when we press, only use history.push() if the link is a known internal SPA link, else fallback to a regular non-SPA navigation?

I don't think there is a std way to provide additional link infos in Markdown, nor if MDX supports that. Another option would be to use a special protocol so that we force the link to consider it as a non-SPA link:

[javadoc](external:///javadoc)

Another solution would be to provide a remark plugin so that user can tell which URLs are to be considered as external. It may be overwhelming for a site user to write a remark plugin, so we could just ship a default one in MDX loader, that would read a list of paths to consider external in site config? Inspiration => https://github.com/remarkjs/remark-external-links

slorber avatar Aug 19 '20 16:08 slorber

I'd strongly vote for the idea that *.md files (and their authors) should not care about the implementation details, i.e. navigation methods. "Does the link represent a valid URL?" - "Yes." Then it should work without any attribution and augmentation in the Markdown. Similar to how static website generators handle it. Imagine people migrating to Docusaurus from Jekyll and being forced to fix all their non-docs and non-images links to a new format.

So, from my understanding the problem is only in terms of navigating to such (valid) link as a SPA-route or not. And this, IMHO, is smth that can be handled transparently to the user. Also, from UX and marketing POV, "the less Docusaurus-specific modifications one needs to do when migrating from static generators, the better and more appealing that seems: 😎

colriot avatar Aug 20 '20 11:08 colriot

I'd strongly vote for the idea that *.md files (and their authors) should not care about the implementation details, i.e. navigation methods.

I agree that we should try to provide good defaults, and not modify the docs for this.

"Does the link represent a valid URL?" - "Yes." Then it should work without any attribution and augmentation in the Markdown.

As explained there: https://github.com/facebook/docusaurus/issues/3303#issuecomment-677638781 A valid markdown link may lead to different kind of navigations, with or without baseUrl

Imagine people migrating to Docusaurus from Jekyll and being forced to fix all their non-docs and non-images links to a new format.

Jekyll is not a SPA, it does not have to choose between history.push(path) (spa navigation) and window.location.href = path (classic navigation).

Also, if you have a jekyll doc at https://myDomain.com/baseUrl/myDoc, and you link to /javadoc, how can Jekyll know if you did put the doc at https://myDomain.com/javadoc or https://myDomain.com/baseUrl/javadoc, given that Docusaurus has no idea what you will do with the /build folder after you have built your site.

I'm pretty sure I've already seen both of these use cases already (one of the 2 is yours). How can we solve this without being biased in favor of one particular use-case, preventing the other to work? Apart from additional config or some markdown tricks to pass additional info, I don't see.

How did Jekyll do on your https://devserver.org/websites/litho/ deployments when you link to javadoc? Did it link to https://devserver.org/websites/litho/javadoc? Was there any way to link to https://devserver.org/javadoc instead?

slorber avatar Aug 20 '20 12:08 slorber

@coliff just seen this SO answer from @yangshun https://stackoverflow.com/a/55940426/82609

I suggest you try to add https://github.com/vitaliy-bobrov/remarkable-extlink with target=_blank, and Docusaurus will not try to navigate to it as a SPA link

Edit: oh nevermind it's code for v1

slorber avatar Aug 20 '20 18:08 slorber

As explained there: #3303 (comment) A valid markdown link may lead to different kind of navigations, with or without baseUrl

I'll move that discussion here, to keep it under the dedicated issue 🙂
You've wrote:

Can lead to one of the following navigation method being used

  • history.push('/baseUrl/javadoc'): this is what breaks your site, as /baseUrl/javadoc is not a SPA route
  • window.location.href = '/javadoc': this is not your case (as your javadoc is under the baseUrl path) but other sites may want to link to files outside of baseUrl path
  • window.location.href = '/baseUrl/javadoc': this is what you want

we need a way to tell docusaurus that /javadoc is a non-SPA path (can be infered) that should, or not, be prefixed by baseurl, and that's the hard part.

From your answer in https://github.com/facebook/docusaurus/issues/3278#issuecomment-674013091:

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 got an impression that, by default, "root" links are prepended with baseUrl, and to not prepend it we need to use pathname://.

This seems like a reasonable default behaviour to me. I understand that you maybe didn't think about non-SPA navigation then, but I don't see the reason, why there should be different default behaviours for SPA and non-SPA links – that would just be confusing. So, if you'll keep the same defaults, then to distinguish between cases 2 and 3 we just need to prepend the prefix (for window.location.href = '/javadoc'), or keep as is (for window.location.href = '/baseUrl/javadoc').

Jekyll is not a SPA, it does not have to choose between history.push(path) (spa navigation) and window.location.href = path (classic navigation).

Yep, that's why I refer to them as static generators. But that was not my point. Again, I'm talking from the user perspective here, and talking about default SPA-navigation behaviour, I see it reasonable to keep the same defaults for non-SPA. Wdyt?

Also, I can imagine that picking the right defaults to hit the majority of users is hard. Maybe it's worth to make this default behaviour configurable, if there is a real concern? But that may sound as an unnecessary complication 🙂

How did Jekyll do on your https://devserver.org/websites/litho/ deployments when you link to javadoc?

There was no internal deployment before. To introduce one, we are migrating our website to Docusaurus, that's one of the reasons.

colriot avatar Aug 21 '20 11:08 colriot

Actually checked it and I think pathname:// does not really work for navigation links ðŸĪŠ we made it work as an escape hatch for images, and for assets (like linking to a zip file)

The general idea is that we try to ensure that a site that works without a baseurl should keep working if we add a baseurl, so yes it probably makes sense to support your usecase by default (linking to /baseUrl/javadoc instead of /javadoc) and provide an escape hatch for the 2nd case.

Will try to find a good way to support these usecases, because with i18n it's very likely that we'll use baseUrl = /myBaseUrl/en-US/ etc (https://github.com/facebook/docusaurus/issues/3317) so this kind of issue is more likely to happen anyway.

slorber avatar Aug 21 '20 13:08 slorber

Actually checked it and I think pathname:// does not really work for navigation links ðŸĪŠ we made it work as an escape hatch for images, and for assets (like linking to a zip file)

👀

Will try to find a good way to support these usecases

@slorber thanks a lot! Do you have any ETA for this? I'm not sure what might be the priority of this. Also, while this is not implemented, what would you recommend right now to make these links work after the first click? (do we have any option to support baseUrl now, or are we forced to use absolute URL for some time?)

colriot avatar Aug 25 '20 09:08 colriot

I'll try to make this possible soon, as I think there is currently no workaround to solve your problem. Need to figure out which solution/api first, have some ideas but not sure which is best.

It's not possible to apply baseurl in markdown absolute links so that it works fine on your 2 distinct envs.

slorber avatar Aug 27 '20 12:08 slorber

So, I made this PR to make the pathname:///javadoc escape hatch work.

https://github.com/facebook/docusaurus/pull/3347

It's not an ideal solution as it requires updating markdown but it should work with or without baseurl, as seen on this page: https://deploy-preview-3347--docusaurus-2.netlify.app/classic/markdown-tests/

Other ideas I have are more complex to implement, and I'm not even sure to get the API right, so I prefer to take more time to figure this out.

slorber avatar Aug 27 '20 17:08 slorber

@slorber not sure I got it right. I thought you decided to make /javadocs links work with baseUrl by default, without any pathname://. Why did everything change? If we need to use pathname:// for all non-SPA links how to determine whether it should be prepended with baseUrl or not?

The testing section seems incorrect. I suppose these 2 urls should not point to the same destination: pathname:///dogfooding/javadoc pathname://../dogfooding/javadoc am I wrong?

colriot avatar Aug 27 '20 17:08 colriot

/javadoc -> considered as internal link -> will show 404 page pathname:///javadoc -> considered as external link -> will open in a new tab

Both will be prefixed by baseUrl (if any) so that links are not sensitive to baseUrl.

It's not a final solution, but if we find a better one, there should be a simple migration path (like just removing the pathname:// prefix in all your docs, quite easy to automate)

slorber avatar Aug 27 '20 17:08 slorber

@slorber sorry! I updated the comment in between, after actually checking the Testing Page. I see your point, I guess quicker solution is better for now (sad, that we need to have this special case when writing docs :))

While testing I guess I found at least one strange thing:

The testing section seems incorrect. I suppose these 2 urls should not point to the same destination: pathname:///dogfooding/javadoc pathname://../dogfooding/javadoc am I wrong?

colriot avatar Aug 27 '20 18:08 colriot

Yes, I'll figure out how to make this configurable from the outside later.

Yes you are wrong both should resolve to the same path, one is a relative path, the other is absolute but it's the same path in the end

slorber avatar Aug 27 '20 19:08 slorber

@slorber I've tried to update current version to alpha.63, and now I'm getting "Error: Markdown link url is mandatory." error. So, before website would compile, but some links won't work. Now it's not even passing through compilation.

./docs/recycler-collection-component.md
Error: Markdown link url is mandatory. filePath=docs/recycler-collection-component.md, title=null

I saw the code around it: https://github.com/facebook/docusaurus/commit/c7fc781ce09c7899527685cad82cd925e7e7f7af#diff-58bf8f2cad3b8c1c7da9cd722792b33fR122-R125 but not sure what is node and how is it formed.

Can you help figure this out, please?

colriot avatar Sep 09 '20 14:09 colriot

Yes @colriot , this is because you have an empty link here:

https://github.com/facebook/litho/blame/master/website/docs/recycler-collection-component.md#L33

https://fblitho.com/docs/recycler-collection-component#null__horizontal-lists

[LinearLayoutManager]()

I could probably allow this link again and try to find a more retrocompatible setup, but this looks like a mistake in your doc to me, and I try to make Docusaurus defaults to be more strict over time to actually be able to catch such mistakes.

I think there is still a problem here, and the error message should say "title=LinearLayoutManager" instead of null

Does it make sense?

I guess if you really want to have an empty link (link to self?), you could just link to # as a workaround.

If it's not a mistake and you have a real usecase to produce such link, I'd like to know what's your intent.

slorber avatar Sep 10 '20 14:09 slorber

@slorber thanks! No, I don't think it makes sense, but thank you for the workaround.

  • To the context of errors: is it possible to add more info about it? Like, a line in Markdown where this error happened. Or, filled in the title of the link. I.e. Error: Markdown link url is mandatory. filePath=docs/recycler-collection-component.md, title=LinearLayoutManager as I guess that field supposed to be the link's title.

  • Another feedback: only know after fixing this issue, I understand that there were 2 more with CSS: image So, somehow new error block doesn't start from \n, but sticks right at the end of the previous error message at the same line. As on the picture.

  • And regarding these CSS errors, I'm again not sure what does that mean, since I don't have any mentions of id in neither ./src/pages/styles.module.scss nor ./src/css/custom.scss.

colriot avatar Sep 10 '20 15:09 colriot

I have tried to use the pathname:// with version alpha.63 but it not recognise it. Should I add something to use the hatch? Can it be used in sidebar with link category item?

fasolens avatar Sep 10 '20 19:09 fasolens

@colriot I've fixed the error message and added the line number here https://github.com/facebook/docusaurus/pull/3435, so it will be better in alpha 64 on next release:

image

No, I don't think it makes sense

@colriot I don't understand what you mean, what does not make sense exactly? because we seem to agree that this error message needed to be fixed no?

The Sass issue is related to this pending PR that the author still didn't merge: https://github.com/rlamana/docusaurus-plugin-sass/pull/5

@colriot if you want help upgrading, Discord or a brand new issue is a better fit, because these things are totally unrelated to the inability to link to non-SPA links

slorber avatar Sep 11 '20 10:09 slorber

@fasolens I just tested on Docusaurus 2 site and it works for me. The best way to help me help you is to show me actual code (preferable a git branch that I can run) where I see what you are trying to do, and what are your exact expectations.

slorber avatar Sep 11 '20 11:09 slorber

@slorber

I've fixed the error message and added the line number here #3435, so it will be better in alpha 64 on next release

Yay! Super cool! Thanks

I don't understand what you mean, what does not make sense exactly? because we seem to agree that this error message needed to be fixed no?

Sorry for confusion. I thought your questions was related to the empty link. And I meant that this empty link didn't make any sense – was just an error, I guess.

The Sass issue is related to this pending PR that the author still didn't merge: rlamana/docusaurus-plugin-sass#5

I see now it's merged. Thanks for pointing this out!

colriot avatar Sep 11 '20 13:09 colriot

great @colriot :)

the Sass fix is not published to npm yet, so you can use this workaround in the meantime: https://github.com/rlamana/docusaurus-plugin-sass/pull/5#issuecomment-685099274

slorber avatar Sep 11 '20 13:09 slorber

@slorber I hadn't update docusaurus. I thought it was alpha.63 but it was alpha.53 or something :( It is working fine right now.

fasolens avatar Oct 06 '20 14:10 fasolens

it would be great to add the pathname:/// trick to the docs. i just needed to do this for the sidebar and needed to go hunting for this issue.

jhackett1 avatar Feb 18 '21 10:02 jhackett1

Would be really great to be able to force the link to be internal and also use the pathname:// trick. I've got a link right now to a page that is proxied to another site, but I want the navigation to skip the SPA routing but also not add target="_blank". Doesn't seem possible at the moment since the isInternalUrl check simply checks for the existence of any protocol. A simple fix might be to explicitly check for pathname:// and count it as internal in that function.

mlynch avatar Aug 05 '21 20:08 mlynch

In the meantime a workaround is setting target={null} on the <Link> to disable the new tab opening

mlynch avatar Aug 05 '21 22:08 mlynch

So you mean using target="_self" but navigate with browser instead of history.push right?

Afaik React-Router always handles _self as a SPA (history.push) link and we don't have any code to prevent that. Didn't know that using null was a possible workaround, good to know.

slorber avatar Sep 02 '21 14:09 slorber

Another downside of current solution is that when used in the footer, it appends the "external link" icon, even though the link is on the same domain and may be just the parent directory from where we currently are.

Were there any new ideas proposed to handle this? Maybe some RFC that I couldn't find through search?

thexeos avatar Jun 24 '24 01:06 thexeos

For anyone looking for a quick workaround, you can "swizzle" NotFound page and insert code there to detect this condition.

Run npm run swizzle @docusaurus/theme-classic NotFound, pick "Wrap" option, then edit src/theme/NotFound.js (or .ts) file.

Here is a sample implementation that would reload the page in hopes of getting the correct content and if it detects that current page was recently loaded due to a reload it would show the default 404 page.

export default function NotFoundWrapper(props: Props): JSX.Element {
  const { type, loadEventEnd } = window.performance.getEntriesByType(
    'navigation',
  )[0] as PerformanceNavigationTiming
  const isPageReload = type === 'reload'
  const isRecentLoad = window.performance.now() - loadEventEnd < 2500
  const isReload = isPageReload && isRecentLoad

  if (!isReload) {
    location.reload()
    return (
      <>
        <span></span>
      </>
    )
  }

  return (
    <>
      <NotFound {...props} />
    </>
  )
}

thexeos avatar Jun 24 '24 01:06 thexeos