docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

404 page not rendered when `routeBasePath` is `"/"`, and no version uses that path

Open thejcannon opened this issue 1 year ago • 13 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

Combining routeBasePath: '/', with having each version at a specific path (the version itself) results in unknown pages rendering "blank" content (instead of the "Page not found" content).

Reproducible demo

https://stackblitz.com/edit/github-py3g6d?file=docusaurus.config.js

Steps to reproduce

From a fresh project:

$ npm run docusaurus docs:version 1.1.0

And add the following to the docs config:

          routeBasePath: '/',
          versions: {
            current: { path: '/current' },
            '1.1.0': { path: '/1.1.0' },
          },

then visit /foo route. No "Page Not Found" content :cry:

Expected behavior

Render the "Page not found" content

Actual behavior

Empty page

Your environment

No response

Self-service

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

thejcannon avatar Jan 02 '24 20:01 thejcannon

Related to of https://github.com/facebook/docusaurus/issues/9688

Not really a duplicate. However the problem/solution is likely to be the same: make react-router-config render top-level 404 if no subroute matches current path

slorber avatar Jan 03 '24 10:01 slorber

Related to of #9688

Not really a duplicate. However the problem/solution is likely to be the same: make react-router-config render top-level 404 if no subroute matches current path

That issue number is this issue. Did you mean to link to something else?

thejcannon avatar Jan 03 '24 12:01 thejcannon

Also, any suggestions on a workaround?

thejcannon avatar Jan 03 '24 21:01 thejcannon

Sorry 🤪 https://github.com/facebook/docusaurus/issues/9665

I think a workaround could be to create a 404.md doc with slug /, but didn't try. If this doesn't work I don't see any other workaround.

slorber avatar Jan 04 '24 00:01 slorber

However the problem/solution is likely to be the same: make react-router-config render top-level 404 if no subroute matches current path

I'd love to help pitch in if possible! Feel free to thoughtdump how someone would go about doing this :smile:

thejcannon avatar Jan 04 '24 13:01 thejcannon

There's some bits of code that seem to have the intention of showing the <NotFound> 404 component when there's no route match... but apparently not triggering?

  • fallback handling: https://github.com/facebook/docusaurus/blob/ca09f238f3d809641614309cd8118a4e42475c1b/packages/docusaurus/src/client/exports/ComponentCreator.tsx#L28-L45
  • which syncs with the generated routes: https://github.com/facebook/docusaurus/blob/ca09f238f3d809641614309cd8118a4e42475c1b/packages/docusaurus/src/server/routes.ts#L312-L315
  • these are pulled together by: https://github.com/facebook/docusaurus/blob/ca09f238f3d809641614309cd8118a4e42475c1b/packages/docusaurus/src/client/App.tsx#L29

huonw avatar Jan 05 '24 03:01 huonw

@huonw You have found out how we show 404 "in general". However, the docs plugin is weirder: it declares a wildcard subpath match (like /{routeBasePath}/*) so that the sidebar never re-renders on navigation. In this case, a "not found" would not be caught by the root wildcard, but by /docs/*.

Josh-Cena avatar Jan 05 '24 03:01 Josh-Cena

Ah, right, okay, so with routeBasePath == / as we configured, the docs plugin catch-all route is /* and presumably is in the route list before that final catch-all, and thus wins.

huonw avatar Jan 05 '24 03:01 huonw

Yes, exactly :D

Josh-Cena avatar Jan 05 '24 03:01 Josh-Cena

Curious, why is the not found working on our own website? 🤔 https://docusaurus.io/docs/notFound Why does it only happen when routeBasePath is /, I found this surprising.


There are a few ways we could handle it.

  1. Automatically add a * not found route to all subroutes, similar to what is done at the root here:
CleanShot 2024-01-05 at 12 18 44@2x
  1. Fix React-Router-Config route rendering logic

Currently we render routes with a package:

export {renderRoutes as default} from 'react-router-config';

Problem: if a parent route match the path, but no child route match the path, then it does not "escape" the parent route and try to enter another parent route that also matches, nor does it try to render the default * 404 route.

This is IMHO what we should fix.

This would also enable us to have 2 plugins use routeBasePath as / (even if I wouldn't recommend this due to potential conflicts between plugins, it should technically be possible)

  1. Have a way for plugins to declare their custom 404 page

This can be interesting because some users might have a dedicated 404 @theme/DocPageNotFound to say that the doc does not exist, with a link to the root of the version.

That's probably overkill (and requires CDN config to route /docs/* 404 to /docs/notFound.html) but I already saw someone requesting that kind of thing.

Not a priority IMHO


I would go with solution 2, but that's probably not easy. We'd need to fix that code: https://github.com/remix-run/react-router/blob/v5/packages/react-router-config/modules/renderRoutes.js

Note this code doesn't even exist anymore in React-Router v6 (and we'll probably want to upgrade later so need to make sure our solution will be compatible)

slorber avatar Jan 05 '24 11:01 slorber

Note this code doesn't even exist anymore in React-Router v6

I wonder if we're ought to try and see if this bug exists after upgrading. You might be able to feed two birds with one scone.

And if not, we likely ought to file an issue so it's made known to the maintainer(s)

thejcannon avatar Jan 05 '24 12:01 thejcannon

Okie dokie, I know it wasn't your first choice, but decided to take a stab at the "easy" solution here of 1.

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

I'm relatively noviced when it comes to JS (and furthermore TS), so feel free to tear me a new one with suggestions/questions/etc... I try and learn through osmosis.

thejcannon avatar Jan 08 '24 21:01 thejcannon

@slorber there is a workaround, although it's not for the faint of heart :laughing:

https://github.com/pantsbuild/pantsbuild.org/pull/84

thejcannon avatar Jan 10 '24 20:01 thejcannon