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

[V6] [Feature] Getting usePrompt and useBlocker back in the router #8139

Closed
callmeberzerker opened this issue Oct 16, 2021 · 173 comments
Closed

Comments

@callmeberzerker
Copy link

I think in general most people won't be able to upgrade to v6 since in the latest beta usePrompt and useBlocker are removed.

Most apps rely on the usePrompt or Prompt to prevent navigation in case the user has unsaved changes (aka the form is dirty).

With this issue maybe we can have some feedback on why they (usePrompt, Prompt) were removed (they worked fine in the previous beta version v6.0.0-beta.6) and what makes them problematic, what's the outlook for getting them back in the router and potentially userland solutions to this problem.

@openscript
Copy link

openscript commented Oct 16, 2021

I tried to find a workaround with useEffect with the location as a dependencies, but couldn't get it working. With many applications we are stuck on 6.beta.1 because of this and #8035.

@KubaOczko
Copy link

Is there any guidance on how to implement the same behavior? I didn't find any extension point within the current router implementation.
We can't continue with v6 without the possibility to block navigation based on some condition 😢

@fgatti675
Copy link
Contributor

Removing usePrompt and useBlocker in the 7th iteration of the beta was really unexpected.
It would be great to get an alternative way to do the same. Is there any way we can include the removed code in our own codebase? Thanks

@dohomi
Copy link

dohomi commented Oct 26, 2021

I also experienced this and just found out it happened very unexpected in beta7. +1 for bringing it back or at least outline some workaround.

@piecyk
Copy link

piecyk commented Oct 28, 2021

As react-router expose NavigationContext under UNSAFE_NavigationContext we can add useBlocker hook back using the same code.

256cad7#diff-b60f1a2d4276b2a605c05e19816634111de2e8a4186fe9dd7de8e344b65ed4d3L344-L381

import * as React from 'react'
import { UNSAFE_NavigationContext } from 'react-router-dom'
import type { History, Blocker, Transition } from 'history'

export function useBlocker(blocker: Blocker, when = true): void {
  const navigator = React.useContext(UNSAFE_NavigationContext).navigator as History

  React.useEffect(() => {
    if (!when) return

    const unblock = navigator.block((tx: Transition) => {
      const autoUnblockingTx = {
        ...tx,
        retry() {
          // Automatically unblock the transition so it can play all the way
          // through before retrying it. TODO: Figure out how to re-enable
          // this block if the transition is cancelled for some reason.
          unblock()
          tx.retry()
        },
      }

      blocker(autoUnblockingTx)
    })

    return unblock
  }, [navigator, blocker, when])
}

Don't exactly know why "block" method was omitted from History type? @mjackson maybe we could add it back?

@mjackson
Copy link
Member

If you really need blocking I'd recommend you remain on v5 until we have time to get it back into v6. v5 will continue to be supported for the foreseeable future. You shouldn't feel any pressure to upgrade to v6 immediately.

As for why it was removed in v6, we decided we'd rather ship with what we have than take even more time to nail down a feature that isn't fully baked. We will absolutely be working on adding this back in to v6 at some point in the near future, but not for our first stable release of 6.x.

@mjackson
Copy link
Member

I might add (for those of you who want to upgrade immediately to v6) that you could consider another type of user experience instead of blocking navigation away from the current page.

The canonical use case for blocking is so you can show the user a prompt to confirm navigation away from a page with unsaved data. Instead of blocking navigation, another valid way to handle this situation could be to just save the state of the form to local/session storage in the browser as the form values change. Then, when the user returns, repopulate the default values of all the form fields with the ones you saved from the last visit. This experience should be less jarring than throwing up a prompt in front of the user while still giving you the resilience you're looking for.

Again, we are planning on re-adding this functionality to v6 after our stable release. I only offer this suggestion as a possible alternative for those of you who may wish to upgrade immediately.

@piecyk
Copy link

piecyk commented Oct 29, 2021

As for why it was removed in v6, we decided we'd rather ship with what we have than take even more time to nail down a feature that isn't fully baked.

Yes, that's completely understandable. Looking forward for v6 release, great work 👏

My question was about this change, with omitting 'block' from Navigator

Screenshot 2021-10-29 at 06 51 28

@mjackson
Copy link
Member

mjackson commented Oct 29, 2021

@piecyk You can always create your own type to add that method back. I simply removed all the methods from the History interface that aren't being used anywhere in the router.

@yanickrochon
Copy link

yanickrochon commented Nov 4, 2021

A version with breaking changes notice would've been nice prior to a major release. Right now, the project recommends to upgrade, yet the migration guide is partial and application critical features have been removed without notice.

(After updating to v6, I am currently debugging a few errors with react-router-dom : "TypeError: pathname.match is not a function", and "TypeError: meta.relativePath.startsWith is not a function". The migration is anything but smooth. Compared to this, @material-ui, now @mui, made an excellent job at facilitating the upgrade process, starting a few months ago.)

@frankalbenesius
Copy link

I also ran into this omission while trying to migrate from v5 to v6. My migration was going so well! I love the changes that come in v6. They are super intuitive. Really great job!!! Thank you! I will wait patiently for useBlocker or another recommended way of blocking history in v6 before doing our migration.

I will also attempt to convince my company's product owners to use this pattern when possible. Thanks for the suggestion!

@steveoh
Copy link

steveoh commented Nov 16, 2021

We need a way to cancel a long running process if the page is navigated away from in an electron app. There are more use cases than form data or unsaved changes.

@rmorse
Copy link

rmorse commented Nov 24, 2021

As react-router expose NavigationContext under UNSAFE_NavigationContext we can add useBlocker hook back using the same code.

Thanks @piecyk !

I made a gist of this for react-router-dom for easier access:
https://gist.github.com/rmorse/426ffcc579922a82749934826fa9f743

@jamesdh
Copy link

jamesdh commented Dec 2, 2021

Nowhere is the removal of this mentioned anywhere in the v5 to v6 upgrade docs. I just burned an hour trying to figure out why I couldn't resolve Prompt until I stumbled upon this issue.

@timdorr
Copy link
Member

timdorr commented Dec 2, 2021

A note has been added in #8436

@vvakame
Copy link

vvakame commented Dec 6, 2021

@storybook/router uses v6 already.
this dependency introduce react-router native .d.ts instead of @types/react-router-dom. this changes will occur tsc compile error.
so, If we can update to v6. I'm very glad 😉

@matsgm
Copy link

matsgm commented Dec 8, 2021

@storybook/router uses v6 already. this dependency introduce react-router native .d.ts instead of @types/react-router-dom. this changes will occur tsc compile error. so, If we can update to v6. I'm very glad 😉

@storybook/router 6.2.9 uses @reach/router.

The following (and probably some newer versions) is working with React Router v5:

"@storybook/addon-actions": "6.2.9",
"@storybook/addon-docs": "6.2.9",
"@storybook/addons": "6.2.9",
"@storybook/react": "6.2.9",

@Haraldson
Copy link

@mjackson Is there a roadmap for when this will be prioritized and worked on? Got 97% of the way with my upgrade, and the remaining bits are comprised of <Prompt /> and optional route parameters.

@onionhammer

This comment was marked as duplicate.

@chazlm
Copy link

chazlm commented Dec 14, 2022

are these features still not implemented? do we have a timeframe?

@chaance
Copy link
Collaborator

chaance commented Dec 14, 2022

There is a draft PR open at #9709. This is a work-in-progress but keep an eye on the activity there to see where things stand.

@JWo1F
Copy link

JWo1F commented Dec 21, 2022

Hi there. If anyone is also waiting for the usePrompt and useBlocker hooks to appear, in version 6.5.0 you can override the navigate function of the router this way:

const router = createBrowserRouter(routesArray);
const _navigate = router.navigate.bind(router);

type Listener = () => boolean | Promise<boolean>;
const listeners: Listener[] = [];

router.navigate = async (...args) => {
  const params = args as [any];

  if (listeners.length > 0) {
    const promises = listeners.map((fn) => fn());
    const values = await Promise.all(promises);
    const allowed = values.every(Boolean);

    if (!allowed) return;
  }

  return _navigate(...params);
};

const beforeUnload = (e: BeforeUnloadEvent) => {
  // Cancel the event.
  e.preventDefault();
  // Chrome (and legacy IE) requires returnValue to be set.
  e.returnValue = '';
};

function blockNavigation(fn: Listener) {
  if (listeners.length === 0) {
    addEventListener('beforeunload', beforeUnload, { capture: true });
  }

  listeners.push(fn);

  return () => {
    const index = listeners.indexOf(fn);
    listeners.splice(index, 1);
    if (listeners.length === 0) {
      removeEventListener('beforeunload', beforeUnload, { capture: true });
    }
  };
}

You can then use the blockNavigation function to block the navigation:

function useBlocker(dirty: boolean, blocker: Listener) {
  useEffect(() => {
    if (!dirty) return;
    return blockNavigation(blocker);
  }, [blocker, dirty]);
}

And use it in the project (you should change the code to suit your needs, this is just an example):

function usePrompt(message: ReactNode, dirty = true) {
  useBlocker(
    dirty,
    useCallback(
      // async function:
      async () => await confirmUnsavedChanges(message),
      // or just confirm
      // () => confirm('Unsaved changes!'), // boolean
      // or just boolean function:
      // () => doWeNeedToGo(), // boolean
      [message]
    )
  );
}

tillprochaska added a commit to alephdata/aleph that referenced this issue Jan 3, 2023
This bug was likely introduced when upgrading to React Router 6. The `UpdateStatus` component needed access to the router's state in order to block navigation (and show a confirmation dialog to the user) when there are unsaved changes. With the upgrade to React Router 6, this required wrapping class components in a custom `withRouter` HOC. The `UpdateStatus` component exposes the different states (`SUCCESS`, `ERROR`, `IN_PROGRESS`) as static class fields (e.g. `UpdateStatus.IN_PROGRESS`). When wrapping the component in `withRouter`, those static class fields aren't forwarded, i.e. `UpdateStatus.IN_PROGRESS` is undefined.

However, at the moment, blocking navigation isn't possible anyway (at least temporarily), as React Router v6 has removed support for relevant features required to implement this (although it will hopefully return soon [1]). So for now, I have simply removed `withRouter` altogether. Once blocking navigation is possible again, we should refactor this component to a function component and use the hooks provided by React Router.

[1] remix-run/react-router#8139
@Messa1

This comment was marked as off-topic.

Rosencrantz pushed a commit to alephdata/aleph that referenced this issue Jan 9, 2023
This bug was likely introduced when upgrading to React Router 6. The `UpdateStatus` component needed access to the router's state in order to block navigation (and show a confirmation dialog to the user) when there are unsaved changes. With the upgrade to React Router 6, this required wrapping class components in a custom `withRouter` HOC. The `UpdateStatus` component exposes the different states (`SUCCESS`, `ERROR`, `IN_PROGRESS`) as static class fields (e.g. `UpdateStatus.IN_PROGRESS`). When wrapping the component in `withRouter`, those static class fields aren't forwarded, i.e. `UpdateStatus.IN_PROGRESS` is undefined.

However, at the moment, blocking navigation isn't possible anyway (at least temporarily), as React Router v6 has removed support for relevant features required to implement this (although it will hopefully return soon [1]). So for now, I have simply removed `withRouter` altogether. Once blocking navigation is possible again, we should refactor this component to a function component and use the hooks provided by React Router.

[1] remix-run/react-router#8139
@develra
Copy link

develra commented Jan 9, 2023

Thanks for all the awesome progress on this!

I really love the loaders feature in v6.4+ and wanted to be able to keep using that while being able to move forward with a migration that relies on usePrompt - so implemented a version pretty similar to previous react-router versions here:
#9821

I know that ends up being sort of ugly and hacky - but is working well for my current use case. I really appreciate the direction the current PR is going, but have two requests for the final version.

  1. I'm using useBlocker that returns false to report some metrics before react-router handles a navigation. I would love for the final implementation to either allow multiple blockers or to expose a new useBeforeNavigation hook for this purpose.

  2. My impression with the current state of the PR is that react-router doesn't want to deal with the ugly hacks to deal with the "what happens when you click back again with a prompt open" use case. I totally understand that desire, but maybe it would make sense to expose init.history.listen and the history stack (init.history.state.idx) to allow custom implementations of usePrompt that deal with the hacki-ness, without giving too much exposure to history and the foot-guns that entails.

Thanks for all the hard work on resolving this important use-case in a deterministic and easy to maintain way - y'all rock!

@chaance
Copy link
Collaborator

chaance commented Jan 9, 2023

I totally understand that desire, but maybe it would make sense to expose init.history.listen and the history stack (init.history.state.idx) to allow custom implementations of usePrompt that deal with the hacki-ness, without giving too much exposure to history and the foot-guns that entails.

Unfortunately I think both of those suggestions would be leaking hacky implementation details and foot-guns. history.listen is only used for POP navigations in the data routers, and it's only there because of browser API limitations. If the future navigation API settles and gives us better primitives, we could revisit our implementation at any time (another reason for not exposing history directly). The index, too, is a hacky workaround and isn't reliable.

We will likely opt to do as we did before: give you the best effort attempt to block navigation without breaking the back button in most cases, but outline the tradeoffs.

@chaance
Copy link
Collaborator

chaance commented Jan 13, 2023

unstable_useBlocker has been shipped in v6.7.0-pre.3, and I threw up a Gist showing how you can use it to recreate usePrompt (from the v6 beta releases) and <Prompt> from v5. The gist explains the limitations of our approach, so we're leaving it up to end users (you) to own these implementations so you can decide exactly which tradeoffs you'd like to accept for your own use-case.

unstable_useBlocker will be shipped in v6.7.0, but I'd encourage you all to try out the pre-release and offer any feedback you might have. We're confident in the implementation but marked it unstable for now to make sure we're comfortable with the API.

@vladutclp
Copy link

Does anybody know why I'm getting this erro while running tests when using useBlocker hook in my react app?

image

I'm using createBrowserRouter to create the router that is passed to RouterProvider component.

In my test file I render the component like this:

const renderComponent = () =>
		render(<ContactDetailsPage />, { wrapper: BrowserRouter })

@brophdawg11
Copy link
Contributor

Hey folks! unstable_useBlocker and unstable_usePrompt are now released in 6.7.0. Docs should be up on reactrouter.com in the coming days. Until then, there's an example of useBlocker usage in examples/navigation-blocking in the repo.

@dennisoelkers
Copy link

I have the same issue as @vladutclp, with both useBlocker and usePrompt. Both are executed in a component which is nested below a BrowserRouter. What am I doing wrong?

@machour
Copy link
Contributor

machour commented Jan 19, 2023

@dennisoelkers Please open a new Q&A discussion, this is a closed issue.

@brophdawg11
Copy link
Contributor

useBlocker and usePrompt are new data-router features, so you will need to use them inside a RouterProvider, not a BrowserRouter.

@esetnik
Copy link

esetnik commented Jan 19, 2023

useBlocker and usePrompt are new data-router features, so you will need to use them inside a RouterProvider, not a BrowserRouter.

As a point of feedback, the documentation could be significantly improved for how to transition the previous setup using a BrowserRouter to the new RouterProvider. I spent an hour trying to get my v6.4.0 -> v6.7.0 transitioned using

createBrowserRouter(
  createRoutesFromElements(
  ...

but was ultimately unsuccessful and had to roll back.

@chaance
Copy link
Collaborator

chaance commented Jan 19, 2023

We are working on documentation as we speak, but yes it should (and will) definitely be improved.

Locking the issue now that we've wrapped up the core work. Feel free to open new issues if you run into usage problems, and stay tuned for docs!

@remix-run remix-run locked as resolved and limited conversation to collaborators Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests