docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

OnBrokenLinks throw errors with static HTML pages in production builds

Open jbltx opened this issue 1 year ago • 1 comments

Have you read the Contributing Guidelines on issues?

Prerequisites

  • [X] I'm using the latest version of Docusaurus.
  • [X] I have tried the npm run clear or yarn clear command.
  • [X] I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • [X] I have tried creating a repro with https://new.docusaurus.io.
  • [X] I have read the console error message carefully (if applicable).

Description

After upgrading from 3.0.1 to 3.1.0, the onBrokenLinks feature throw exception on static routes while it was not the case before.

Reproducible demo

https://stackblitz.com/edit/github-fausgf

Here is a fork of the repro project but using 3.0.1 without errors: https://stackblitz.com/edit/github-fausgf-qpa5vb

Steps to reproduce

  • Create a new Docusaurus project
  • Create a simple index.html file inside a subfolder in static directory
  • In the Docusaurus config, add a NavBar item to link to this static html page
  • Try to do a production build using npm run build
  • Observe thrown errors during server compilation

Expected behavior

Does not throw errors for static page links (like in 3.0.1)

Actual behavior

Throw errors for static page links

Your environment

  • Public source code: None
  • Public site URL: None
  • Docusaurus version used:3.1.0
  • Environment name and version:Node 18.17
  • Operating system and version:MacOS 14.2.1 and Unbuntu 22.01

Self-service

  • [ ] I'd be willing to fix this bug myself.

jbltx avatar Jan 17 '24 17:01 jbltx

Thanks for reporting, I see where the issue is coming from, we'll fix it 👍

In the meantime, you can use this on your navbar item: 'data-noBrokenLinkCheck': true,

slorber avatar Jan 18 '24 11:01 slorber

Just to add that now we're moving to 3.1 we've noticed a similar thing. We have three different docs sources in our site and linking from one to another throws the warnings (was fine in 3.0.1 and earlier).

andrewgbell avatar Jan 23 '24 16:01 andrewgbell

@andrewgbell is your link having target: "_blank" too?

Because it wasn't mentioned in the original issue, but it's what matters to me here.

If the link is always opened with such target, then we never navigate through a "soft/SPA navigation" and it's always a full page reload, so it doesn't make sense to check for broken links for such links with targets.

If your link does not have a target, please share another repro so that I understand the case better

slorber avatar Jan 24 '24 10:01 slorber

@andrewgbell is your link having target: "_blank" too?

Because it wasn't mentioned in the original issue, but it's what matters to me here.

If the link is always opened with such target, then we never navigate through a "soft/SPA navigation" and it's always a full page reload, so it doesn't make sense to check for broken links for such links with targets.

If your link does not have a target, please share another repro so that I understand the case better

No, we don't have that. The repo is private, however I can grant you access if that works for you?

andrewgbell avatar Jan 24 '24 14:01 andrewgbell

@andrewgbell even if the repo is private, what prevents you from creating a smaller repro where I can clearly see which topology of link gets reported?

Giving me access to your repo is fine, but if I need to spend 2 hours figuring out a complex setup and identifying which link exactly we are talking about, I'm less likely to solve the problem.

I'll close this issue links with target are not checked by our broken link checker anymore. If you have another case, that's probably worth opening a different bug report with a repro so that we don't miss that case.

slorber avatar Jan 24 '24 15:01 slorber

This seems to have been resolved now by two things. 1, running the latest canary, 2, then adding a trailing slash after support below in docusaurus.config.js from to: "/support", to to: "/support/",

(it wasn't needed in earlier versions, but maybe we were just lucky): ` footer: {

  style: "dark",
  links: [
    {
      title: "Docs",
      items: [
        {
          label: "label1",
          to: "/label1/1/",
        },
        {
          label: "label2",
          to: "/label2/2/",
        },
        {
          label: "Support",
          to: "/support/",
        },
      ],
    }

,`

andrewgbell avatar Jan 25 '24 13:01 andrewgbell

@andrewgbell

This seems to have been resolved now by two things.

Great, but unfortunately I don't even know what the issue is in the first place.

What problem has upgrading to canary resolved?

1, running the latest canary, 2, then adding a trailing slash after support below in docusaurus.config.js from /support to /support/

I see where this trailing slash problem might come from.

But that would help to have a runnable repro to be sure we are talking about the same thing. Not seeing things such as your trailingSlash config and md docs creates an ambiguity, and I won't be sure to solve exactly the problem you encounter.


So:

    1. I still don't know what the original issue is about
    1. It's possible that my perf optimizations introduced a regression

These are 2 separate issues that both need to be solved.

Can you please create a smaller repro detailing both?

(I have no idea for case 1, and only have a guess for case 2)

slorber avatar Jan 25 '24 16:01 slorber

Going to close this issue as part of https://github.com/facebook/docusaurus/pull/9788


@andrewgbell I'm not able to understand case 2 either, so if there is a problem I don't see, please open another issue with a repro.

slorber avatar Jan 25 '24 17:01 slorber

The 2nd case (trailing slash url being reported) will probably be fixed in https://github.com/facebook/docusaurus/pull/9791

slorber avatar Jan 25 '24 18:01 slorber