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

[Bug]: useBlocker fails when blocking function is quick to proceed #11613

Closed
Artur- opened this issue Jun 5, 2024 · 3 comments · Fixed by #11930
Closed

[Bug]: useBlocker fails when blocking function is quick to proceed #11613

Artur- opened this issue Jun 5, 2024 · 3 comments · Fixed by #11930
Labels
awaiting release This issue have been fixed and will be released soon bug

Comments

@Artur-
Copy link

Artur- commented Jun 5, 2024

What version of React Router are you using?

6.23.1

Steps to Reproduce

Take the example from https://github.com/remix-run/react-router/blob/main/examples/navigation-blocking/src/app.tsx

and remove the user input based confirmation and replace it with an immediate confirmation, e.g. something like

  // Reset the blocker if the user cleans the form
  React.useEffect(() => {
    if (blocker.state === 'blocked' && value === '') {
      blocker.reset();
    } else if (blocker.state === 'blocked') {
      blocker.proceed();
    }
  }, [blocker, value]);

Full code in https://stackblitz.com/edit/github-qwtgqt

Expected Behavior

When you go to view "One", then to the form view, enter "123" in the field and press back in the browser, you should end up on the "One" view as the blocker allows proceeding.

Actual Behavior

You remain on the form view (this might be dependent on the machine and the browser, tested with Chrome on a M1 mac). In another test case where the proceed function call takes a very short time, the back navigation succeeds around 80% of the time.

I think this is because the blocker functionality that in this case immediately after the user's back navigation will navigate forward and then back again. This does not work because history.go is asynchronous and you are supposed to wait for a popstate before calling it again: https://developer.mozilla.org/en-US/docs/Web/API/History/go

Similarly, if you do something like this in JS

for (let i=1; i <= 5; i++) {
  history.pushState( { myState: "url"+i+"state" },"","url" + i);
}

window.addEventListener("popstate", e => console.log("popstate with url",window.location.pathname,"and state",e.state));

history.go(-1);
history.go(1);
history.go(-1);

You will only see two popstate events, and for paths 4 and 3.

If you use short delay instead, like

setTimeout(()=> history.go(-1),100);
setTimeout(()=> history.go(1),200);
setTimeout(()=> history.go(-1),300);

you see the expected three events and actually end up on the correct history entry

@Artur- Artur- added the bug label Jun 5, 2024
@Artur-
Copy link
Author

Artur- commented Jun 5, 2024

So if I understand it correctly, the first "oops, let's go back" navigation is fine because that is a reaction to a popstate event but then you would need to wait for the popstate event for that navigation before redoing the original navigation

@taylorkline
Copy link

taylorkline commented Jul 30, 2024

I haven't tested the fix, but came across this after noticing an issue where a simple .proceed() after a window.confirm(message) results in a no-op.

Both of the following reproduce the issue:

	useEffect(() => {
		if (blocker.state === "blocked") {
			if (window.confirm(unsavedMessage)) {
				console.log("User wants to leave");
				// setTimeout(() => {
				blocker.proceed();
				// }, 0);
			} else {
				console.log("User wants to stay");
				blocker.reset();
			}
		}
		console.log(blocker.state);
	}, [blocker]);
	useEffect(() => {
		if (blocker.state === "blocked") {
			blocker.proceed();
		}
		console.log(blocker.state);
	}, [blocker]);

This seems to be confirmed by the fact that the proceed in usePrompt has a setTimeout in #10718 from @brophdawg11.

This also seems to be discussed in discussion item #10489.

@brophdawg11
Copy link
Contributor

Resolved by #11930 and will be include din the next release 👍

@brophdawg11 brophdawg11 added the awaiting release This issue have been fixed and will be released soon label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release This issue have been fixed and will be released soon bug
Projects
None yet
3 participants