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

Links validation failures after updating to 2.0.0-alpha.61 #3303

Closed
colriot opened this issue Aug 18, 2020 · 11 comments · Fixed by #3308
Closed

Links validation failures after updating to 2.0.0-alpha.61 #3303

colriot opened this issue Aug 18, 2020 · 11 comments · Fixed by #3308
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@colriot
Copy link

colriot commented Aug 18, 2020

🐛 Bug Report

We are in the process of migrating Litho UI Framework website to Docusaurus v2. After updating Docusaurus setup to alpha.61 we started getting weird behaviour of not seeing the content of static/javadoc folder on the website and prod build started to fail on links validation step.

Have you read the Contributing Guidelines on issues?

Yep

To Reproduce

Validation errors:

  1. Check out this PR: Update Docusaurus to the latest version litho#692
  2. Go to website folder and run yarn run build for links validation to kick in
  3. See lots of validation errors for /javadoc/... links

Navigation failures:

  1. Check out the same PR: Update Docusaurus to the latest version litho#692
  2. Go to website folder and run yarn run start to access website locally
  3. Open website URL and click to "API" item in the right part of the header
  4. You'll see 404 error for the link http://localhost:3000/javadoc/
  5. Update the page with Cmd+R/Ctrl+R and you'll see the correct API Javadocs page

Expected behavior

No validation errors on build and /javadocs/... links navigation working without reloads.

Actual Behavior

Validation errors:

Console log screenshot Console log

Navigation failures:
Video in Dropbox

Your Environment

  • Docusaurus version used: 2.0.0-alpha.61
  • Environment: Chrome 84.0.4147.125 (Official Build) (64-bit), Node.js 14.6.0):
  • Operating system: MacOS 10.15.5

Reproducible Demo

Steps are present in "To Reproduce" section.

@colriot colriot added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Aug 18, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 18, 2020

Hi,

This is a security we put in place to detect broken links. This will catch false positives, like there, because Docusaurus is not aware of the /javadoc path, as it did not create it.

Note, at the end of the error message, there is an explanation about a setting so that instead of throwing, it errors, warn, or ignore all broken links we found.

Also, if you use full URLs (including the domain) these URLs won't be checked against the broken link checker.

As I knew this feature could be annoying and bring false positive, I've added a "secret" escape hatch. The navbar/footer items linking to the javadoc could be:

            {
              label: 'API',
              href: '/javadoc/',
              'data-noBrokenLinkCheck': true,
            },

This way you opt out of the broken link checker on a per-link basis, and can keep detecting legit broken links.

@colriot
Copy link
Author

colriot commented Aug 18, 2020

Hi @slorber! Thanks for your answer. I'll add some clarification questions:

  • I tried generic config for link checker not throwing, but we still would like this functionality for the website. Plus, this doesn't solve reload issue.
  • You said Docusaurus doesn't know about static/javadoc, but it's in the same state as static/images and with images we don't have any problems in our case. Does Docusaurus handle them differently?
  • In the message above we talk about validation solely. Does that escape-hatch also help with link not being loaded in the browser at first and succeeding only after additional reload?
  • /javadoc in navbar is only one case, we also have lots of Javadoc links scattered across Markdown documentation files. Do we need to opt-out on a per link basis?

@slorber
Copy link
Collaborator

slorber commented Aug 18, 2020

Hi,

The escape hatch is mostly for sites that use a baseurl like /myWebsiteBaseUrl and yet they want to link to some page that is "outside" of the baseurl. If the content is in /static/javadoc, we should prevent the broken link checker to report /javadoc as a broken link. As this system is new, I didn't think about that case :) And yes images go through a different path, using webpack's file-loader system.

So, I'd that we'll fix this soon, in the meantime you can ignore the broken link errors (or at least not throw) and re-enable later.

In the message above we talk about validation solely. Does that escape-hatch also help with link not being loaded in the browser at first and succeeding only after additional reload?

I have no idea what you are talking about 🤪

@colriot
Copy link
Author

colriot commented Aug 19, 2020

Hello @slorber,
Thanks for explanation and for taking care of it!
Though I'd still consider everything inside the static folder as part of "inside" baseurl stuff :) Do I get it right, that all the content of /static is deployed at the same level of baseUrl? Like, in our case, images, videos, javadoc would be deployed at <baseUrl>/images, <baseUrl>/videos and <baseUrl>/javadoc respectively.

Does that escape-hatch also help with link not being loaded in the browser at first and succeeding only after additional reload?

I have no idea what you are talking about 🤪

It's about this part:

Navigation failures:

  1. Check out the same PR: Update Docusaurus to the latest version litho#692
  2. Go to website folder and run yarn run start to access website locally
  3. Open website URL and click to "API" item in the right part of the header
  4. You'll see 404 error for the link http://localhost:3000/javadoc/
  5. Update the page with Cmd+R/Ctrl+R and you'll see the correct API Javadocs page

Video reproducing the issue

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

Validation errors (broken links found)

Yes, things in ./static are part of the site.

https://github.com/facebook/docusaurus/blob/master/packages/docusaurus/src/server/brokenLinks.ts#L132

Actually the code only consider files like myFile.html, it does not consider folders like build/javadoc/index.html
We should make it consider the folder too if that folder contains an index.html file. Will try to provide that for the next release (soon)

Navigation failures:

If you use markdown links like [javadoc](/javadoc), the problem is that Docusaurus has no way to understand if you express an internal link (SPA-like navigation) or an external link (should probably open it with target=_blank), so it's hard for us to know if we should detect or not broken links, add a baseUrl if needed or not...

Do you have such links?

Because in your codebase I've seen the following:

You can find the full list of components and APIs within our [javadocs for the com.facebook.litho.widget package](https://fblitho.com/javadoc/index.html?com/facebook/litho/widget/package-summary.html).

This will work normally.

But if you want to avoid the domain, I've added a similar escape hatch so that Docusaurus leaves the path unmodified and pass it directly to the link:

[javadoc](pathname:///javadoc)

If you can give me link to files for which you have a problem, that will be helpful to see how to solve this problem

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

For the broken links, it should be solved in #3308

For the navigation failure, I'm not sure but I don't feel there's a regression there, it's likely that it didn't work in early versions of Docusaurus 2 and I don't see a better solution than the pathname:// prefix for now. Do you have an example?

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

Unfortunately I don't even think the pathname:// escape hatch works, as it seems filtered by our markdown parser.
There's no easy way to assign some metadata to a markdown link.

The problem is a bit different here and I don't think there's an easy way to solve it.

Unlike the broken link checker that can do its job at the end of the build, we have to choose what should be done when an user clicks on the link: should we navigate with classic HTML navigation, or should we navigate with SPA-like navigation (history.push("/javadoc"));

As we are building a static app, there is no easy way to know ahead of time if /javadoc is an SPA link route or not.

Not sure what is the best solution to this problem, but it's something we should try to solve indeed.

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

I've opened a proper issue to track this problem: #3309

@colriot
Copy link
Author

colriot commented Aug 20, 2020

Validation errors (broken links found)

Actually the code only consider files like myFile.html, it does not consider folders like build/javadoc/index.html

Thank you! That sounds right to me 😃 . Considering that any arbitrary number of subfolders will be handled.

Navigation failures

it's hard for us to know if we should detect or not broken links, add a baseUrl if needed or not... Do you have such links?

From #3278 I understood that all root (/blabla) links would be wrapped by useBaseUrl() automatically. And that was the last part we were missing for Docusaurus migration, because out website is deployed both externally to https://fblitho.com/ and internally to https://devserver.org/websites/litho/. I.e. without useBaseUrl() our /javadoc links would not work on the internal deployment, but at the same time we don't want to pollute pure *.md files with React hooks without a real need and change the common way of writing docs that all the teammates are used to.

So, I was considering that any /blabla link in Markdown would be prepended with baseUrl due to that change, and that's what we needed too. I.e. the link's value (string) should be correct with that, everything is generated as I would expect it to be, and the problem is only in terms of navigating to this link as a SPA-route or not. I strongly support the idea that documentation written in markdown should not know anything about navigation methods. Similar to how static website generators do (our /javadoc links came straight from Jekyll, where it worked).

Because in your codebase I've seen the following:
"APIs within our [javadocs for the com.facebook.litho.widget package](https://fblitho.com/javadoc/index.html?com/facebook/litho/widget/package-summary.html)"
This will work normally.

That was a mistake and it was fixed for Docusaurus in website folder. We should not have any absolute links right now, due to the newly added internal deployment (remember devserver.org/websites/litho/)

I'll copy the last part to the new dedicated issue as well, to continue discussion there 😃

@slorber
Copy link
Collaborator

slorber commented Aug 20, 2020

At some point we have to choose.

Supposing we use a /baseUrl, this markdown link

[link](/javadoc)

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

Given that we can only write very simple links in markdown syntax, how can we choose between these 3 code paths?

I think we can prevent case 1 from happening, if we try to see if the link matches a "known" route.

But there is not a clean way to define what behavior is the correct one for case 2 and case 3, yet we must support both, not only your /baseUrl/javadoc case :)

I'd like to not have to modify the docs, and solve this usecase from the outside, but somehow 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.

Note: history.push('/javadoc') never makes sense, as all the SPA routes live under the baseUrl

@colriot
Copy link
Author

colriot commented Aug 21, 2020

I've answered in #3309 (comment), to keep navigation discussion in its own issue.

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
3 participants