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

Make sure cancelPrevious is a function before trying to call it. #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make sure cancelPrevious is a function before trying to call it. #53

wants to merge 1 commit into from

Conversation

thibaultzanini
Copy link

No description provided.

@kriskowal
Copy link
Member

Perhaps make this check earlier, and throw an error into the caller that provides it. cancelPrevious should be undefined or a function at this point.

@marchant
Copy link
Member

marchant commented Sep 9, 2016

Thanks Kris, wouldn't throwing risk slowing down code by being deoptimized?

On Sep 8, 2016, at 15:53, Kris Kowal notifications@github.com wrote:

Perhaps make this check earlier, and throw an error into the caller that provides it. cancelPrevious should be undefined or a function at this point.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@kriskowal
Copy link
Member

V8 does not penalize throw, but it does penalize try/catch, and that
problem can be mitigated by only using try/catch in very small functions
that delegate all the real work. The try/catch does not penalize the entire
stack.

On Thu, Sep 8, 2016 at 5:20 PM Benoit Marchant notifications@github.com
wrote:

Thanks Kris, wouldn't throwing risk slowing down code by being deoptimized?

On Sep 8, 2016, at 15:53, Kris Kowal notifications@github.com wrote:

Perhaps make this check earlier, and throw an error into the caller that
provides it. cancelPrevious should be undefined or a function at this point.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#53 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADrhvQCCe8AvRa3Q2Q0zzrdH7BDDbcKks5qoKZAgaJpZM4J4iCY
.

@thibaultzanini
Copy link
Author

PR updated

@hthetiot
Copy link
Contributor

@thibaultzanini check Travis

@hthetiot
Copy link
Contributor

@thibaultzanini could you rebase on master. Thx.

@hthetiot hthetiot added this to the 4.0.x milestone Dec 28, 2017
@hthetiot hthetiot self-requested a review December 28, 2017 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants