history.block() fails intermittently, causing loss of unsaved changes

[octogonz]

Have you read the Contributing Guidelines on issues?

• I have read the Contributing Guidelines on issues.

Prerequisites

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

Description

The history.block() API fails unpredictably when used on a Docusaurus website. The reason is that this API is also used by some system components, but the API doesn't allow more than one callback to be registered simultaneously.

Why this matters: We need history.block() to prevent accidental navigation in situations such as unsaved changes. Otherwise the user might accidentally click on the navigation header and lose all their work. (It's the same problem usually solved by window.addEventListener('beforeunload', ...), but for the case of in-page navigation.)

What we could do above it: Docusaurus should provide a wrapper API to address this requirement, but does not seem to.

Steps to reproduce

Below is a simple repro. It's attempting to use the history.block() API from @docusaurus/router to prevent navigation:

import {useEffect} from 'react';
import { useHistory, useLocation } from '@docusaurus/router';

function HistoryTest() {
  const history = useHistory();
  useEffect(() => {
    console.log("+++ history.block()");
    const unblock = history.block((location, action) => {
      console.log("+++ callback was called");

      // Prevent navigation to other pages
      return false; 
    });

    return () => {
      unblock();
    };
  }, [history]);

  return <>REPRO</>;
}

Expected behavior

When the user clicks a navigation hyperlink, the callback should be invoked, giving the component a chance to reject the action by returning false.

Actual behavior

It occasionally works. But quite often the callback will NOT get called. The console logs show +++ history.block() but not +++ callback was called.

And this warning appears sometimes in the console:

Warning: A history supports only one prompt at a time

This warning is telling us that the history.block() API does not allow more than one callback to be registered simultaneously. Some debugging revealed that another callback is being registered inside useHistoryPopHandler() in this component:

docusaurus/packages/docusaurus-theme-common/src/contexts/navbarMobileSidebar.tsx

Lines 48 to 58 in fd51384

	function useContextValue(): ContextValue {
	const disabled = useIsNavbarMobileSidebarDisabled();
	const windowSize = useWindowSize();
	const shouldRender = !disabled && windowSize === 'mobile';
	const [shown, setShown] = useState(false);
	// Close mobile sidebar on navigation pop
	// Most likely firing when using the Android back button (but not only)
	useHistoryPopHandler(() => {

This conflict happens when setting up the NavbarMobileSidebarProvider context. The useHistoryPopHandler() conflict occurs even if we are not on a mobile site. Even if we have no use for NavbarMobileSidebar.

Proposed solution

Docusaurus reexports useHistory() from @docusaurus/router, implying that it is supported for use by the website. But given how React components get combined together from the theme and plugins, it seems unrealistic to expect components to somehow globally coordinate their access to history.block().

Instead, maybe we can provide an API that wraps history.block() in a way that allows multiple components to handle the event. Thinking about the design of this callback, I don't see any semantic problem with each component getting a turn to reject the navigation event. Once it is rejected, we can simply skip calling the remaining event handlers. (Actually, I don't understand why history.block() imposed this restriction in the first place, unless it was intended to be a low level facility to be wrapped a higher level system such as I am proposing.)

Your environment

• Docusaurus version used: 3.5.1 ... 3.7.0 and https://new.docusaurus.io/
• Environment name and version (e.g. Chrome 89, Node.js 16.4): Firefox 136, Node 20.9.0
• Operating system and version (e.g. Ubuntu 20.04.2 LTS): Linux/Windows

Self-service

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

[octogonz]

As a workaround, maybe someone could recommend a way to prevent NavbarMobileSidebarProvider from calling history.block().

The "seems desirable enough" comment from #5462 suggests that this functionality is nonessential:

docusaurus/packages/docusaurus-theme-common/src/contexts/navbarMobileSidebar.tsx

Lines 58 to 64 in fd51384

	useHistoryPopHandler(() => {
	if (shown) {
	setShown(false);
	// Prevent pop navigation; seems desirable enough
	// See https://github.com/facebook/docusaurus/pull/5462#issuecomment-911699846
	return false;
	}

[slorber]

Thanks for your analysis

The "seems desirable enough" comment from #5462 suggests that this functionality is nonessential:

The functionality being discussed here is the return false, not the closing of the drawer when using Android back button. Even if we removed return false, we'd still register the history.block() call and cause your issue.

---

I'm implementing a workaround in #10989, by only rendering our mobile drawer history blocker logic conditionally.

Preview:

• https://deploy-preview-10989--docusaurus-2.netlify.app/tests/pages/, then navigate to history-tests

This means that the "conflicting blockers" situation will only happen if you open the mobile drawer, which will override yours. This case can happen but it much less common.

This is not a complete fix, and there are edge cases: when opening the mobile drawer, and doing 2 back navigations, the drawer closes but the alert doesn't print and we still navigate back from the page.

A proper fix would probably be to have a provider that collects an array of history block handlers that we call successively until we get a false value, but this requires introducing a new API with a proper design. Honestly, I'm not sure it's worth implementing unless someone has a clear use case for this, which doesn't seem to be your case.

The workaround I implemented is good enough for now, unless proven otherwise

[slorber]

Note there's an extra workaround to be aware of, based on the fact that the last registered blocker "wins".

If you use the useNavbarMobileSidebar().shown in your history blocker, it always "re-render" on drawer state changes, and thus your blocker overrides the one registered by the mobile drawer.

import {useNavbarMobileSidebar} from '@docusaurus/theme-common/internal';


function BlockNavigation() {
  const history = useHistory();
  const {shown} = useNavbarMobileSidebar();
  useEffect(() => {
    return history.block(() => {
      alert('navigation blocked successfully');
      return false;
    });
  }, [history, shown]);
  return false;
}

In practice this means that your blocker will now be applied consistently everywhere, even when the drawer is open:

The tradeoff is that now it's your responsibility to eventually close the drawer on back navigation, in addition to whatever else you want to do in your handler function.

And of course, you still have the annoying History console warning.

---

Don't hesitate to close this issue if you think those workarounds are good enough. I think they are and it's probably not worth it implementing a new API.

[octogonz]

you still have the annoying History console warning.

This isn't okay for my app.

For now as a workaround, I used pnpm patch to remove that logic from the mobile drawer.

now it's your responsibility to eventually close the drawer on back navigation

It seems like this same problem will arise in many different ways, wherever a reusable component wants to interact with history. Philosophically one of the big selling points of Docusaurus is its fast/smooth in-page navigation while still delivering a completely statically rendered website. That's why it seems that @docusaurus/router would benefit by providing an API if react-router does not, if Docusaurus is intended for complex websites and not just tiny project sites.

If the React community has not already designed a history.block() wrapper API, and you are interested, I could propose something.

[slorber]

This isn't okay for my app.

You mean the warning is a problem? Or is there any UX problem?

---

If the React community has not already designed a history.block() wrapper API, and you are interested, I could propose something.

That could be interesting yes.

Note that you probably need to use a nested context provider structure so that you are able to preserve the "tree ordering" of the history block handlers. The order likely needs to follow the tree and the most deeply nested handler should probably always run first. A bit similar to how <Head> allows to override SEO metadata that's already declared upper in the tree

[octogonz]

You mean the warning is a problem? Or is there any UX problem?

The websites I work on represent people who purport to be web app experts. It's embarrassing if we have a pile of assertion failures in our own F12 console. 😆

[slorber]

This warning will only show if you open the mobile drawer, less likely to happen on desktop where you use devtools. Agree it doesn't look great but most devs opening dev tools will only see this warning if they use a small viewport and then open the drawer, not by default.

Note that history v5 should allow multiple blockers. I plan to upgrade to React Router v7 / History v5 for Docusaurus v4 (we have an existing PR that we'll revisit soon #6037).

Until then the workaround is probably good enough. It's probably not worth it to implement a custom solution that we later have to remove.