-
Notifications
You must be signed in to change notification settings - Fork 159
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
Don't fire 'error' for already-aborted transition #87
Conversation
👍 And winning the Internet for the hour starting: now. |
I don't feel comfortable merging in such a heavy code change |
Don't fire 'error' for already-aborted transition
@ef4 this code change may be a hazard, as assertions which fire during |
@stefanpenner - Please open an issue with a repro so we have the ability to address, I do not fully understand the scenario you are describing... |
model: function() { | ||
return new Promise(function(res, rej){ | ||
router.transitionTo('good'); | ||
rej(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue the fact that the resulting syntax error from this line is totally lost is the hazard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno, maybe I'm missing something obvious but I don't see a syntax error here....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way though, opening an issue with a repro is the best (possibly only?) way to ensure we actually fix it...
This closes emberjs/ember.js#4573