fix(animation): remove throw and fix promise handling - #5861
vakrilov merged 1 commit into
Conversation
Sorry, something went wrong.
|
@NathanWalker There is another place which should be fixed ( imho) :
Stack also shows it :
|
Sorry, something went wrong.
|
@RoyiNamir in your case above, anytime you use animation.play() it'd be best practice to wire up the success and error callbacks, ie: animation.play(options).then(success => {
//...
}, error => {
//...
});
|
Sorry, something went wrong.
|
@NathanWalker Not only. In my case ^ it was when I added a class ( which contains as single animation-name with 1 iteration) and then removed that class. Something like : [ngClass]='{loadingClass:isLoading}'
So basically when isLoadingLogic finished , it removed that class , and caused this error. |
Sorry, something went wrong.
|
@RoyiNamir good to know. Nice one and valid use case. Will update PR to handle that as well, thank you for clarifying. |
Sorry, something went wrong.
|
@RoyiNamir There really should never be a rejection in keyframe animation handling since there's no proper way to handle it therefore this removes rejections altogether. Will solve your case and many others that have been reported over time around this. You may try the changes seen in this PR in your project and report back the results for you. |
Sorry, something went wrong.
|
test |
Sorry, something went wrong.
| animation.play().then(() => { | ||
| this.animate(view, index + 1, iterations); | ||
| }, (error: any) => { | ||
| traceWrite(typeof error === "string" ? error : error.message, traceCategories.Animation, traceType.warn); |
There was a problem hiding this comment.
Just wondering - do you need both error callback and .catch()?
Sorry, something went wrong.
There was a problem hiding this comment.
@vakrilov Why not ? if this.animate(view, index + 1, iterations); will throw ( for some reason) , it won't be cached in the callback of , (error: any) =>.
But - it will be catched in the catch.
Sorry, something went wrong.
There was a problem hiding this comment.
Due to number of reports around instability here I was keeping for assurance it was stopped.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch
Technically the error callback appears preferred as catch internally calls rejected. I would presume an error callback is better everywhere instead of a catch based on way docs have it stated.
Sorry, something went wrong.
There was a problem hiding this comment.
Although as @RoyiNamir points out I believe there are others cases that having both may provide utmost resiliency against.
Sorry, something went wrong.
|
test |
Sorry, something went wrong.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sorry, something went wrong.
PR Checklist
What is the current behavior?
Usage of throw destabilizes apps during animation handling.
What is the new behavior?
Promise rejection is now removed around keyframe handling since there was no valid way to handle. With tracing enabled developers can now have meaningful and productive debug sessions.
NativeScript/nativescript-angular#431