facebook / facebook/docusaurus

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

Ouverte
#10,988 8 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
bug
Langage dominant
TypeScript
Étoiles
66.2k
Forks
10k
Merge moyen
1 j 3 h
PR mergées (30 j)
52

Description

### Have you read the Contributing Guidelines on issues?

- [x] I have read the [Contributing Guidelines on issues](https://github.com/facebook/docusaurus/blob/main/CONTRIBUTING.md#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

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:

```ts
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:

https://github.com/facebook/docusaurus/blob/fd51384cab565bf32c3965f2aca1854b8d79248f/packages/docusaurus-theme-common/src/contexts/navbarMobileSidebar.tsx#L48-L58

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

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

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.