for discussion: support for zones in DOM events

for discussion: support for zones in DOM events

Not ready for checkin because of currentZoneExperimental, but I wanted to illustrate how zones can be used in DOM events with minimal overhead if zones aren't being used.

This would give us what we need for Polymer, and probably help other frameworks as well.

[Jennifer Messerly, 2013-08-13 21:24:15]

This change is only for discussion. What do y'all think about the changes in
EventStreamProvider?

(the dart:async changes are not right; I just punched through the API boundary
so to see if the dart:html changes could actually work.)

[Jennifer Messerly, 2013-08-13 21:28:31]

On 2013/08/13 21:24:15, John Messerly wrote:
> This change is only for discussion. What do y'all think about the changes in
> EventStreamProvider?
> 
> (the dart:async changes are not right; I just punched through the API boundary
> so to see if the dart:html changes could actually work.)

To add some motivation here:

Right now we hack around DOM events+zones in two places:
https://github.com/dart-lang/web-ui/blob/dc50c98deac426ebbfbbb0d10159baa2d095...
https://github.com/dart-lang/web-ui/blob/dc50c98deac426ebbfbbb0d10159baa2d095...

The overhead of zones is preferable to polling :)

[blois, 2013-08-13 21:43:36]

I like it better than going through StreamController for DOM events, though
Florian was averse to exposing zones publicly which is why this hasn't been done
yet.

It sounds like Angular is using similar hacks and mentioned a vague desire for
this- though never responded to my request for more info.

In general though zones feel like an expensive contract (a user-provided
callback after every async operation) which is being used to work around other
platform limitations. Given that zones are there I understand why people want to
use them, but I'd rather see a narrower solution to the problem.

[blois, 2013-08-13 22:01:46]

On 2013/08/13 21:43:36, blois wrote:
> I like it better than going through StreamController for DOM events, though
> Florian was averse to exposing zones publicly which is why this hasn't been
done
> yet.
> 
> It sounds like Angular is using similar hacks and mentioned a vague desire for
> this- though never responded to my request for more info.
> 
> In general though zones feel like an expensive contract (a user-provided
> callback after every async operation) which is being used to work around other
> platform limitations. Given that zones are there I understand why people want
to
> use them, but I'd rather see a narrower solution to the problem.

FYI- the change I made a while back when originally discussing with Florian-
https://codereview.chromium.org/16123029/

I can fix it up, but every time I've asked people for feedback on zones &
priority I've never gotten a firm 'zones plz'. I've also never seen a bug come
up on this.

[Jennifer Messerly, 2013-08-13 22:04:45]

On 2013/08/13 22:01:46, blois wrote:
> On 2013/08/13 21:43:36, blois wrote:
> > I like it better than going through StreamController for DOM events, though
> > Florian was averse to exposing zones publicly which is why this hasn't been
> done
> > yet.
> > 
> > It sounds like Angular is using similar hacks and mentioned a vague desire
for
> > this- though never responded to my request for more info.
> > 
> > In general though zones feel like an expensive contract (a user-provided
> > callback after every async operation) which is being used to work around
other
> > platform limitations. Given that zones are there I understand why people
want
> to
> > use them, but I'd rather see a narrower solution to the problem.
> 
> FYI- the change I made a while back when originally discussing with Florian-
> https://codereview.chromium.org/16123029/
> 
> I can fix it up, but every time I've asked people for feedback on zones &
> priority I've never gotten a firm 'zones plz'. I've also never seen a bug come
> up on this.

I've heard the exact opposite -- people want zones and are coming up with
increasingly crazy workarounds :)

FYI the main bug is here, note it is marked C1:
https://code.google.com/p/dart/issues/detail?id=11279
... and others like: https://code.google.com/p/dart/issues/detail?id=12215

[Jennifer Messerly, 2013-08-13 22:06:43]

On 2013/08/13 22:01:46, blois wrote:
> 
> FYI- the change I made a while back when originally discussing with Florian-
> https://codereview.chromium.org/16123029/

yeah, that works too. But it probably has a lot more overhead?
also what's going on there with the stream not being broadcast by default?

[justinfagnani, 2013-08-13 22:31:56]

On 2013/08/13 21:24:15, John Messerly wrote:
> This change is only for discussion. What do y'all think about the changes in
> EventStreamProvider?
> 
> (the dart:async changes are not right; I just punched through the API boundary
> so to see if the dart:html changes could actually work.)

I basically like this, and it would help Angular and C1 out a lot. I wouldn't
want to expose Zone until the powers that be reexamine the APIs though, so your
hiding idea would be necessary. I hope everyone using Zones knows this stuff
will change!

[Jennifer Messerly, 2013-08-13 22:41:27]

On 2013/08/13 22:31:56, justinfagnani wrote:
> On 2013/08/13 21:24:15, John Messerly wrote:
> > This change is only for discussion. What do y'all think about the changes in
> > EventStreamProvider?
> > 
> > (the dart:async changes are not right; I just punched through the API
boundary
> > so to see if the dart:html changes could actually work.)
> 
> I basically like this, and it would help Angular and C1 out a lot. I wouldn't
> want to expose Zone until the powers that be reexamine the APIs though, so
your
> hiding idea would be necessary. I hope everyone using Zones knows this stuff
> will change!

yeah, currentZoneExperimental is a little crazy :)
Some maybe less crazy ideas:

* expose only the 4 pieces needed (with different names, of course):
isDefaultZone, expectCallback, executePeriodicCallbackGuarded,
cancelCallbackExpectation.

* add an API like this to dart:async:

abstract class EventStreamSubscription<T extends Event> extends
StreamSubscription<T> {
  // Note: DOM implements these to call add/removeEventListener, and supply
appropriate event name and capture flag
  void registerListener(void handleData());
  void unregisterListener(void handleData());
}

[blois, 2013-08-13 23:07:36]

On 2013/08/13 22:41:27, John Messerly wrote:
> On 2013/08/13 22:31:56, justinfagnani wrote:
> > On 2013/08/13 21:24:15, John Messerly wrote:
> > > This change is only for discussion. What do y'all think about the changes
in
> > > EventStreamProvider?
> > > 
> > > (the dart:async changes are not right; I just punched through the API
> boundary
> > > so to see if the dart:html changes could actually work.)
> > 
> > I basically like this, and it would help Angular and C1 out a lot. I
wouldn't
> > want to expose Zone until the powers that be reexamine the APIs though, so
> your
> > hiding idea would be necessary. I hope everyone using Zones knows this stuff
> > will change!
> 
> yeah, currentZoneExperimental is a little crazy :)
> Some maybe less crazy ideas:
> 
> * expose only the 4 pieces needed (with different names, of course):
> isDefaultZone, expectCallback, executePeriodicCallbackGuarded,
> cancelCallbackExpectation.
> 
> * add an API like this to dart:async:
> 
> abstract class EventStreamSubscription<T extends Event> extends
> StreamSubscription<T> {
>   // Note: DOM implements these to call add/removeEventListener, and supply
> appropriate event name and capture flag
>   void registerListener(void handleData());
>   void unregisterListener(void handleData());
> }

Our recent addition of custom subclasses of Stream with additional APIs makes it
not-quite-straightforward to use StreamControllers, without cloning
_ForwardingStream & all dependencies from dart:async.

[Jennifer Messerly, 2013-09-04 03:41:02]

On 2013/08/13 23:07:36, blois wrote:
> On 2013/08/13 22:41:27, John Messerly wrote:
> > On 2013/08/13 22:31:56, justinfagnani wrote:
> > > On 2013/08/13 21:24:15, John Messerly wrote:
> > > > This change is only for discussion. What do y'all think about the
changes
> in
> > > > EventStreamProvider?
> > > > 
> > > > (the dart:async changes are not right; I just punched through the API
> > boundary
> > > > so to see if the dart:html changes could actually work.)
> > > 
> > > I basically like this, and it would help Angular and C1 out a lot. I
> wouldn't
> > > want to expose Zone until the powers that be reexamine the APIs though, so
> > your
> > > hiding idea would be necessary. I hope everyone using Zones knows this
stuff
> > > will change!
> > 
> > yeah, currentZoneExperimental is a little crazy :)
> > Some maybe less crazy ideas:
> > 
> > * expose only the 4 pieces needed (with different names, of course):
> > isDefaultZone, expectCallback, executePeriodicCallbackGuarded,
> > cancelCallbackExpectation.
> > 
> > * add an API like this to dart:async:
> > 
> > abstract class EventStreamSubscription<T extends Event> extends
> > StreamSubscription<T> {
> >   // Note: DOM implements these to call add/removeEventListener, and supply
> > appropriate event name and capture flag
> >   void registerListener(void handleData());
> >   void unregisterListener(void handleData());
> > }
> 
> Our recent addition of custom subclasses of Stream with additional APIs makes
it
> not-quite-straightforward to use StreamControllers, without cloning
> _ForwardingStream & all dependencies from dart:async.

Friendly ping. Florian, any thoughts on this approach?

[floitsch, 2013-09-19 08:59:55]

I'm making the zones public soon (https://codereview.chromium.org/23875032).
Once that Zone CL landed it should be possible to land this CL with few
modifications.

[floitsch, 2013-09-23 17:15:04]

On 2013/09/19 08:59:55, floitsch wrote:
> I'm making the zones public soon (https://codereview.chromium.org/23875032).
> Once that Zone CL landed it should be possible to land this CL with few
> modifications.

The zone CL just went green. I will submit it tomorrow morning. If nobody beats
me to it I will then take this CL and adapt it.

[floitsch, 2013-09-25 14:28:23]

Wrote an adapted version: https://codereview.chromium.org/24509004/

[Jennifer Messerly, 2013-09-25 18:14:33]

On 2013/09/25 14:28:23, floitsch wrote:
> Wrote an adapted version: https://codereview.chromium.org/24509004/

awesome! closing this one