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

Prefer calling then and catch separately to combining them into a single then #217

Closed
ghost opened this issue Aug 4, 2021 · 4 comments · Fixed by #525
Closed

Prefer calling then and catch separately to combining them into a single then #217

ghost opened this issue Aug 4, 2021 · 4 comments · Fixed by #525

Comments

@ghost
Copy link

ghost commented Aug 4, 2021

Essentially, Promise.prototype.then allows taking in two arguments to respond to both resolved and rejected promises. Essentially, p.then(f, g) is equivalent to p.catch(g).then(f); note that the catch comes first since errors thrown by the then handler are not caught by the catch handler.

It would be nice to automatically replace instances like this since it makes it easier to visually scan for catch to check if an error is handled. It also helps emphasise the fact that the then is not handled by the catch, which isn't as apparent when it's ordered visually after the then handler.

@xjamundx
Copy link
Contributor

Are you proposing to keep them separate AND to order them. Maybe that would be a config option. I'm not necessarily sold on the ordering thing.

@ghost
Copy link
Author

ghost commented Oct 27, 2021

To clarify, I mean that then(f, g) is equivalent to catch(g).then(f) always, so it can be auto-applied. But the exact ordering of then and catch is up to the user. So, if any automatic combining or splitting is done, it has to be done for things in that order, because otherwise it might change the functionality.

@fisker

This comment has been minimized.

@fisker

This comment has been minimized.

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

Successfully merging a pull request may close this issue.

3 participants