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

Unability to link to an internal non-SPA link #3309

Open
slorber opened this issue Aug 19, 2020 · 27 comments
Open

Unability to link to an internal non-SPA link #3309

slorber opened this issue Aug 19, 2020 · 27 comments
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

🐛 Bug Report

Related to #3303 (comment)

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 slorber added the bug An error in the Docusaurus core causing instability or issues with its execution label Aug 19, 2020
@colriot
Copy link

colriot commented Aug 20, 2020

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: 😎

@slorber
Copy link
Collaborator Author

slorber commented Aug 20, 2020

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: #3303 (comment)
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
Copy link
Collaborator Author

slorber commented Aug 20, 2020

@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

@colriot
Copy link

colriot commented Aug 21, 2020

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 #3278 (comment):

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.

@slorber
Copy link
Collaborator Author

slorber commented Aug 21, 2020

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 (#3317) so this kind of issue is more likely to happen anyway.

@colriot
Copy link

colriot commented Aug 25, 2020

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?)

@slorber
Copy link
Collaborator Author

slorber commented Aug 27, 2020

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
Copy link
Collaborator Author

slorber commented Aug 27, 2020

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

#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.

@colriot
Copy link

colriot commented Aug 27, 2020

@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?

@slorber
Copy link
Collaborator Author

slorber commented Aug 27, 2020

/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)

@colriot
Copy link

colriot commented Aug 27, 2020

@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?

@slorber
Copy link
Collaborator Author

slorber commented Aug 27, 2020

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

@colriot

This comment has been minimized.

@slorber

This comment has been minimized.

@colriot

This comment has been minimized.

@fasolens

This comment has been minimized.

@slorber

This comment has been minimized.

@slorber

This comment has been minimized.

@colriot

This comment has been minimized.

@slorber

This comment has been minimized.

@fasolens

This comment has been minimized.

@jhackett1
Copy link

jhackett1 commented Feb 18, 2021

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.

facebook-github-bot pushed a commit to facebook/infer that referenced this issue Feb 19, 2021
Summary:
Links to pages in static resources have been broken for a while it
seems: Docusaurus generates a Page not Found error for them, then
refreshing the URL loads the page properly... This is because of wrong
routing from the Docusaurus Single-Page App.

See facebook/docusaurus#3309

A suggested workaround is to prepend `pathname://` in front of each link
not managed by docusaurus. This makes the page open in a new tab, which
is actually quite ok for our use cases: OCaml API docs and man pages.

Reviewed By: skcho

Differential Revision: D26544273

fbshipit-source-id: 2ed86ca4f
@mlynch
Copy link

mlynch commented Aug 5, 2021

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
Copy link

mlynch commented Aug 5, 2021

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

@slorber
Copy link
Collaborator Author

slorber commented Sep 2, 2021

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.

@thexeos
Copy link

thexeos commented Jun 24, 2024

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
Copy link

thexeos commented Jun 24, 2024

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} />
    </>
  )
}

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

No branches or pull requests

6 participants