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

Consider making all custom event callbacks asynchronous #108

Closed
andybryant opened this Issue May 12, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@andybryant

andybryant commented May 12, 2013

For request/response event pairs, it's currently possible for replies to be either synchronous or asynchronous. Consider the following snippet.

this.trigger('cakeRequested');
this.on('cakeServed', eatCake);

Assuming a component CakeMaker exists that listens for a 'cakeRequested' event and generates a 'cakeServed' event, the eatCake function would only be called if CakeMaker is asynchronous.

For the following example with the two calls flipped around (harder to read IMO), eatCake will now always be called. However depending on whether CakeMaker is asynchronous or not, cleanPlate could be called before or after eatCake.

this.on('cakeServed', eatCake);
this.trigger('cakeRequested');
cleanPlate();

I'd recommend always ensuring callbacks are asynchronous. Wait until the next tick before calling listeners after an event is triggered, much like how Q provides the same guarantee for promises. See https://github.com/kriskowal/q/blob/master/q.js#L85

@maxkueng

This comment has been minimized.

Show comment
Hide comment
@maxkueng

maxkueng commented May 16, 2013

+1

@danwrong

This comment has been minimized.

Show comment
Hide comment
@danwrong

danwrong May 16, 2013

Contributor

Couple of reasons why we don't do this:

  1. We use jQuery's event system. It's out of scope of Flight to adjust how events work at a low level.
  2. In practice (in other projects) I've found that this often results in sluggish performance. setTimeout 0 is more like setTimeout 20 in practice and when there are large amounts of events flying around it can get very noticeable.
Contributor

danwrong commented May 16, 2013

Couple of reasons why we don't do this:

  1. We use jQuery's event system. It's out of scope of Flight to adjust how events work at a low level.
  2. In practice (in other projects) I've found that this often results in sluggish performance. setTimeout 0 is more like setTimeout 20 in practice and when there are large amounts of events flying around it can get very noticeable.
@andybryant

This comment has been minimized.

Show comment
Hide comment
@andybryant

andybryant May 16, 2013

Hi Dan,

You shouldn't need to switch away from using jQuery.trigger() - just add the async nature before calling it.

If you look at the Q code, it tries a bunch of techniques to make this fast as possible (setTimeout is a last resort for old browsers). On Chrome for instance it uses MessageChannel, which in ad-hoc testing adds delay of less than 1ms.

andybryant commented May 16, 2013

Hi Dan,

You shouldn't need to switch away from using jQuery.trigger() - just add the async nature before calling it.

If you look at the Q code, it tries a bunch of techniques to make this fast as possible (setTimeout is a last resort for old browsers). On Chrome for instance it uses MessageChannel, which in ad-hoc testing adds delay of less than 1ms.

@danwrong

This comment has been minimized.

Show comment
Hide comment
@danwrong

danwrong May 17, 2013

Contributor

Ah, that's interesting. Didn't know about the async hacks.

I know we wouldn't need to alter jquery code specifically but I meant that I think that Flight should not change how events work. Setting an event handler via Flight should act the same as setting an event handler directly with jquery. I think diverging would cause confusion.

Contributor

danwrong commented May 17, 2013

Ah, that's interesting. Didn't know about the async hacks.

I know we wouldn't need to alter jquery code specifically but I meant that I think that Flight should not change how events work. Setting an event handler via Flight should act the same as setting an event handler directly with jquery. I think diverging would cause confusion.

@maxkueng

This comment has been minimized.

Show comment
Hide comment
@andybryant

This comment has been minimized.

Show comment
Hide comment
@andybryant

andybryant May 19, 2013

Your call - although I'm guessing jquery made the decision to allow synchronous event handlers back when there wasn't an efficient way of forcing asynchronous behaviour.

andybryant commented May 19, 2013

Your call - although I'm guessing jquery made the decision to allow synchronous event handlers back when there wasn't an efficient way of forcing asynchronous behaviour.

@danwrong danwrong closed this Jul 4, 2013

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