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

Promise/observable returns in validate/transform/process hooks #136

Open
bisubus opened this issue Dec 2, 2018 · 2 comments
Open

Promise/observable returns in validate/transform/process hooks #136

bisubus opened this issue Dec 2, 2018 · 2 comments

Comments

@bisubus
Copy link

bisubus commented Dec 2, 2018

It appears that hooks don't fully benefit from promise/observable returns. The suggestion is to use promise and observable returns and make callback API optional where possible.

The library already relies on hook functions arity, this could be used to change their behaviour based on function parameters.

This validate hook

async validate({ action }, resolve, reject) {
  try {
    await ...;
    resolve(action);
  } catch (err) {
    reject(err);
  }
}

could become

async validate({ action }) {
  await ...;
  return action;
}

validateOptions/transformOptions could possibly be useful with an option similar to dispatchReturn, just in case the behaviour needs to be unambiguous.

And this multi-dispatch process hook

async process({ action }, dispatch, done) {
  dispatch(...);
  await ...;
  dispatch(...);
  done();
}

could become

async process({ action }, dispatch) {
  dispatch(...);
  await ...;
  dispatch(...);
}

Currently single-dispatch mode is deprecated, call done when finished dispatching warning appears in case process function hook has 2 parameters. Additional doneReturn option (defaults to true for multi-dispatch) could possibly be useful for non-ending logics. I believe this approach supplements #93.

@jeffbski
Copy link
Owner

@bisubus Thanks for the suggestions. I like your ideas. I'll give it some thought.

@thorn0
Copy link

thorn0 commented Aug 7, 2019

Yeah, let's get rid of done, please. It looks so 2016. :)

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

No branches or pull requests

3 participants