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

Difficult to migrate .block() use case to v5 #921

Open
liamqma opened this issue Dec 22, 2021 · 5 comments
Open

Difficult to migrate .block() use case to v5 #921

liamqma opened this issue Dec 22, 2021 · 5 comments

Comments

@liamqma
Copy link

liamqma commented Dec 22, 2021

Hi,

I use history.block to check whether the next location can be transition to. If not, do a full page load. The pseudo code looks like

const unblock = history.block((tx) => {
  // canTransition is the function that checks if next location can be SPA transition to
  // If false, do a full page load on next location's path
  if (!canTransition()) {
    return window.location.assign(nextPath);
  }
  unblock();
  tx.retry();
});

Full code example: https://codesandbox.io/s/distracted-river-8yczz?file=/src/index.js

So depending on the result of canTransition(), it should either unblock the blocker or do a full page reload. Users shouldn't notice much difference. This is fine except for the navigation that reloads the page. Users see something like this (Chrome):
image
It doesn't make sense to our users as there isn't any unsaved changes.
I've read the documentation and understand the library registers a beforeunload handler to prevent the navigation. Maybe v5's .block is not designed for my use case.

But I am able to achieve this use case via history v4. I've also tried history.listen but .listen is too late as it happens at the end of applyTx.

Any suggestion is much appreciated. 🙂
The solution I can think of is to remove window.addEventListener(BeforeUnloadEventType, promptBeforeUnload); from the library and let consumer of the library to handle it.

@liamqma
Copy link
Author

liamqma commented Jan 7, 2022

I've found a hacky workaround 😬

window.addEventListener('beforeunload', () => {
        unBlock();
});

Register our own BeforeUnloadEvent and clean up our transition blockers before history runs its check

@dubzzz
Copy link

dubzzz commented Feb 3, 2022

@liamqma Thank you so much for the workaround it works perfectly well in my case too. I just had to register my own event listener before calling block which I did the other way initially.

Meanwhile I am wondering if calling the provided blockers on "beforeunload" could be something directly implemented inside history itself. I came up with this code on my side (highly inspired from react-router beta.6):

// location being undefined when beforeunload cases
export type BlockerFunction = (tx: { location: Location | undefined; retry: () => void }) => void;

export function useBlocker(blocker: BlockerFunction): void {
  const navigationContext = useContext(UNSAFE_NavigationContext);
  const navigator = navigationContext.navigator as History;

  useEffect(() => {
    const beforeUnloadCallback = () => {
      let closeAuthorized = false;
      blocker({
        location: undefined,
        retry: () => {
          closeAuthorized = true;
        },
      });
      if (closeAuthorized) {
        unblock();
      }
    };
    window.addEventListener('beforeunload', beforeUnloadCallback);

    const unblock = navigator.block((tx) => {
      const autoUnblockingTx = {
        ...tx,
        retry() {
          unblock();
          tx.retry();
        },
      };
      blocker(autoUnblockingTx);
    });

    return () => {
      unblock();
      window.removeEventListener('beforeunload', beforeUnloadCallback);
    };
  }, [block, blocker]);
}

The beforeUnloadCallback is basically doing the trick you talked about plus calling the underlying blocker to check whether or not we should prevent the default behaviour of the browser. If we should, then we let the beforeunload handler registered by history do the blocking, otherwise we unplug it (your code).

I'm wondering if such behaviour should be the default for block in history. It would made users able to say whether or not they really want to lock the tab closing. Is there a reason not to call blockers when receiving things on beforeunload?

@liamqma
Copy link
Author

liamqma commented Feb 3, 2022

@dubzzz this approach sounds like a legit improvement.

@karlshea
Copy link

I'm trying to migrate to this version, and we were using a similar approach (checking to see if a route should be handled by react-router in our SPA or if we needed to fully reload the page to get a server-rendered version), and blockers are completely not working for us to the point where we may have to fork this library.

The big issue is the beforeUnload event listener that history is registering. It keeps popping a prompt when reloading the page or when navigating in and out of our SPA pages using the back/forward browser buttons, even when we register our own listener and unblock() first.

I think what we really need is a persistent listener that we don't need to keep registering on every route change that can check the same transaction but doesn't register the beforeUnload listener.

This will all be moot when we finish moving the whole app to a SPA, but for anyone that has some routes in a SPA and some routes outside, you really cannot move to history 5 or react-router 6 at this point.

@karlshea
Copy link

I was able to get this working in my case by making the changes in PR #949.

Then in our app I set up the blocker like this:

const registerBlock = () => {
  const unblock = history.noUnloadBlock((tx) => {
    if (!isSpaLocation(tx.location)) {
      window.location.href = createPath(tx.location);
      return;
    }

    unblock();
    tx.retry();
  });
};

history.listen(() => {
  // Register a new block on location changes.
  registerBlock();
});

// Register initial block.
registerBlock();

Normal blockers are looked at first (to support prompts), and then if they all allow the transaction the no-unload blockers are checked afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants