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

Support asynchronous blocking #545

Closed
kirill-konshin opened this issue Nov 22, 2017 · 7 comments
Closed

Support asynchronous blocking #545

kirill-konshin opened this issue Nov 22, 2017 · 7 comments

Comments

@kirill-konshin
Copy link

kirill-konshin commented Nov 22, 2017

It's really hard to implement asynchronous procedure that should delay navigation.

It would be great if blocker function could return Promise (if it's resolved navigation should proceed, if rejected — stopped) or a function that can accept an argument callback so that callback(true) will resume nav.

In this case result in https://github.com/ReactTraining/history/blob/master/modules/createTransitionManager.js#L26 treatment should be a bit different.

Change is super small and should not break anything. I propose 2 options (I have made a pull request #546 with scenario 2 because it's more user friendly):

1. Less changes: use getUserConfirmation to handle string and any other object results

  var confirmTransitionTo = function confirmTransitionTo(location, action, getUserConfirmation, callback) {
      console.log('Bar', prompt, getUserConfirmation);
    // TODO: If another transition starts while we're still confirming
    // the previous one, we may end up in a weird state. Figure out the
    // best way to handle this.
    if (prompt != null) {
      var result = typeof prompt === 'function' ? prompt(location, action) : prompt;

// NEW STUFF
      if (typeof result === 'string' || typeof result === 'object' || typeof result === 'function') {
// NEW STUFF
        if (typeof getUserConfirmation === 'function') {
          getUserConfirmation(result, callback);
        } else {
          warning(false, 'A history needs a getUserConfirmation function in order to use a prompt message');

          callback(true);
        }
      } else {
        // Return false from a transition hook to cancel the transition.
        callback(result !== false);
      }
    } else {
      callback(true);
    }
  };

2. More user friendly: handle Promise right there in transition confirmation

  var confirmTransitionTo = function confirmTransitionTo(location, action, getUserConfirmation, callback) {
      console.log('Bar', prompt, getUserConfirmation);
    // TODO: If another transition starts while we're still confirming
    // the previous one, we may end up in a weird state. Figure out the
    // best way to handle this.
    if (prompt != null) {
      var result = typeof prompt === 'function' ? prompt(location, action) : prompt;

      if (typeof result === 'string') {
        if (typeof getUserConfirmation === 'function') {
          getUserConfirmation(result, callback);
        } else {
          warning(false, 'A history needs a getUserConfirmation function in order to use a prompt message');

          callback(true);
        }
// NEW STUFF
      } else if (typeof result === 'function') {
        result(callback);
      } else if ('then' in result && 'catch' in result) { // duck typing here for simplicity
        result
          .then(function() { callback(true); })
          .catch(function() { callback(false; });
// NEW STUFF
      } else {
        // Return false from a transition hook to cancel the transition.
        callback(result !== false);
      }
    } else {
      callback(true);
    }
  };

I personally would prefer option 2 because in this case all handling can be done locally in component w/o need to configure extra getUserConfirmation in some other files:

const blocker = (({pathname}, action) => {
  return asyncFigureOutWhatToDoWith(pathname, action); // returns promise
};

or

const blocker = (({pathname}, action) => {
  return (callback) => {
    asyncFigureOutWhatToDoWith(pathname, action)
      .then(function() { callback(true); })
      .catch(function() { callback(false; });    
  };
};
kirill-konshin added a commit to kirill-konshin/history that referenced this issue Nov 22, 2017
@idrm
Copy link

idrm commented Dec 27, 2017

You can pass a custom getUserConfirmation prop to your <Router/> component? E.g.

<Router
  getUserConfirmation={(message, callback) => {
    console.log('will async confirm transition', message);
    setTimeout(() => {
      callback(true);
    }, 1500);
  }}
>
...
</Router>

where you'd replace setTimeout with your custom async logic.

The message param is equal to what you pass inside <Prompt />.

If you need to pass a more complicated object inside message you will run into issues with type checking in, say, TypeScript, since the original signature for that prop is string | (Location => (string | boolean)).

My preferred async callback approach is to change the state inside the component that contains <Router/>, and display a modal dialog when the state changes. This keeps everything declarative. E.g.

class App extends Component {

  state = {
    transitioning: false
  }

  render = () => (
    <Router
      getUserConfirmation={(message, callback) => {
        this.setState({
          transitioning: true,
          transitionMessage: message,
          transitionCallback: callback,
        });
      }}
    >
      {
        this.state.transitioning ? (
         <ModalConfirmationDialog
           onConfirm={() => {
             const { transitionCallback } = this.state;
             this.setState({
               transitioning: false,
               transitionMessage: undefined,
               transitionCallback: undefined,
             }, () => transitionCallback(true));
           })
           onReject={() => {
             const { transitionCallback } = this.state;
             this.setState({
               transitioning: false,
               transitionMessage: undefined,
               transitionCallback: undefined,
             }, () => transitionCallback(false));
            }}
          />
        ) : null
      }
      ...
    </Router>
  )
}

where <ModalConfirmationDialog/> is a custom modal confirmation dialog implementation.

@kirill-konshin
Copy link
Author

This seems to be an overly global solution to me... I need something more local, say, block transition for one page only. Moreover it could be UI-free thing, for example, do XHR and do not allow user to leave the page while this XHR is pending.

@mjackson
Copy link
Member

The problem with a general async blocking function is that it doesn't work for the beforeunload event, which is what I was trying to generalize when I wrote the current implementation. beforeunload is synchronous. You only have one shot to return a string that will be displayed in the dialog.

The current API needs to be reworked to accommodate both use cases:

  1. You want to prevent navigation in the same app
  2. You want to prevent navigation to another site (or closing the tab)

@mjackson mjackson mentioned this issue Jan 12, 2018
@kirill-konshin
Copy link
Author

@mjackson I see now from where it is all coming :) makes sense. I was solving first case, so maybe for that we can have separate Promise-based API?

@kirill-konshin
Copy link
Author

Any news about this?

@mufumbo
Copy link

mufumbo commented Sep 17, 2018

The proposed solution doesn't work? any work around this?

@mjackson
Copy link
Member

You should not rely on history.block for anything at this point because the API has problems that I don't know how to resolve (see #690 for more info). I'm planning on removing it in the next major version. There is probably a much more straightforward way to accomplish what you need. I'd be happy to discuss in #690 if you can tell me your use case.

@lock lock bot locked as resolved and limited conversation to collaborators May 26, 2019
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

4 participants