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

errors in transition handlers are swallowed #21

Closed
ryanflorence opened this issue Jun 10, 2014 · 6 comments
Closed

errors in transition handlers are swallowed #21

ryanflorence opened this issue Jun 10, 2014 · 6 comments

Comments

@ryanflorence
Copy link
Member

I hate promises ... can we not use promises?

willTransitionFrom: function() {
  undefined(); // ignored :(
}
@KidkArolis
Copy link
Contributor

well.. the rule is quite simple - promises should always be returned

here many promises are being created in a loop, but only the last one is getting returned
https://github.com/rpflorence/react-router/blob/7fc55834e9f607f553c96774c99c8cc7785b1c26/lib/router.js#L522

replace with smth like

return when.map(reversedArray(matches), function () {
  ... make your promise ...
  return promise;
});

@andreypopp
Copy link
Contributor

I'd also suggest using bluebird promise library which logs uncaught errors in browser console.

@KidkArolis
Copy link
Contributor

Or when.

@ryanflorence
Copy link
Member Author

"just do promises right" :P

Anyway, not actually interested in a promise conversation (I was just bugging Michael about them before this commit, talking about how people always mess them up and then app code errors get swallowed, so it was fun for me to come back and say "see ... pain in the neck").

I have two requirements though:

  1. We don't swallow user's code in our promise usage
  2. We support a simple callback in transition hooks so people don't have to use promises in application code because half of us have had nothing but bad experiences with them.

I do not want to constantly be having promise conversations with people using this library.

@mjackson
Copy link
Member

I'll take a look at this today. @rpflorence I agree that the last thing I want is to have endless conversations with people trying to track down swallowed errors, so we'll just fix it and not swallow them. If we can't, we'll remove support for them.

@mjackson
Copy link
Member

BTW, I'd be happy to discuss support for callbacks in transition hooks in a separate issue. Promises give us guarantees that callbacks don't, and they present a really clean interface for dealing with asynchronous code.

@andreypopp I used es6-promise because it's just a shim instead of a full-blown library with lots of extra bells and whistles. Using es6-promise today means we can easily remove it at some point in the future when more runtimes support a native Promise.

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