dart-archive / observe

Support for marking objects as observable, and getting notifications when those objects are mutated
https://pub.dartlang.org/packages/observe
BSD 3-Clause "New" or "Revised" License
13 stars 6 forks source link

Open discussion about the current API #9

Closed matanlurey closed 7 years ago

matanlurey commented 9 years ago

Some questions below.

  1. Why is deliverChanges part of the API? I understand why it's part of the HTML-based implementation, but why does a receiver of an Observable object need access to it?
  2. Dirty checking-based observables is not presently something I've used, mostly because we already have a dirty-checking based component, Angular Dart, and the @observe annotation goes unused in every project I work with. Does this need to be part of the core interface?
  3. Are there plans (as far as you know) for dart2js to switch to using JS's Object.observe internally? If not it doesn't really make sense (in my opinion) to adhere closely to the standards when that means more API bloat. If anything, it seems like it makes sense to have two public libraries:

    package:observe/observe.dart Core API, no dependencies on smoke, browser specs, mirrors, etc. Just interfaces (and implementations of those interfaces, like ObservableMap) that can be used

    package:observe/html.dart API that mirrors the HTML/JS spec for people that'd like to use that. Maybe one day in the future it will use Object.observe behind the scenes.

  4. Last I checked, there was some significant code size costs to using mixins in dart2js. Is that no longer the case? This has prevented me from advocating use of with Observable/ChangeNotifier.
  5. When I pub build or serve any application that uses package:observe, it in turn throws a nasty warning that mirrors are being used. Ideally in the static version of any application, no library or dependency would import 'dart:mirrors' at all. Unfortunately that isn't possible when importing observe.dart.
jmesserly commented 9 years ago
  1. Why is deliverChanges part of the API? I understand why it's part of the HTML-based implementation, but why does a receiver of an Observable object need access to it?

it's to ensure you can get synchronous change delivery. This is sometimes a requirement. There is a bug, though: https://github.com/dart-lang/observe/issues/5

  1. Dirty checking-based observables is not presently something I've used, mostly because we already have a dirty-checking based component, Angular Dart, and the @observe annotation goes unused in every project I work with. Does this need to be part of the core interface?

Fascinating. Can you give an example of what this looks like? If you're using a dirty check system, do you need change delivery at all? It's a lot simpler to write a "watcher" API that just polls everything.

In some sense, the entire point of Observable is to be able to optimize away the dirty-checking for deployment.

  1. Are there plans (as far as you know) for dart2js to switch to using JS's Object.observe internally?

That was the intent, yes. That we use Object.observe when available.

Keep in mind Object.observe is not a "browser spec". EcmaScript is a programming language standard, just like Dart. It works on the server & command line (see npm).

  1. Last I checked, there was some significant code size costs to using mixins in dart2js. Is that no longer the case?

I've never heard this before. Maybe they meant "mirrors" instead of "mixins"?

  1. When I pub build or serve any application that uses package:observe, it in turn throws a nasty warning that mirrors are being used.

Similar to Angular2 tranformers, we have https://github.com/dart-lang/smoke to eliminate mirrors in deployment. That said opened: https://github.com/dart-lang/observe/issues/10

matanlurey commented 9 years ago

it's to ensure you can get synchronous change delivery.

Acknowledged. I guess you could always throw if an implementation does not want to support it.

Fascinating. Can you give an example of what this looks like?

https://github.com/angular/angular. I can reference to you a copy of their Change Detector API, but basically they already have a tree-pruning based dirty checking on all controllers. We're hoping to pipe Observable.changes into their Change Detector API when available.

That we use Object.observe when available

Any ETA or experiments I can know about? (Not blocking, just interested)

I've never heard this before. Maybe they meant "mirrors" instead of "mixins"?

I can upload a .tar of the same class with and without a mixin and what the dart2js looks like.

That said opened: #10

I really appreciate that, thank you. I've primarily been consuming package:observe for the interfaces and for ObservableList/Map, and it's been really helpful. Let me know if there is anything I can personally do to help.

jmesserly commented 9 years ago

BTW, some of the issues with change records are over in https://github.com/dart-lang/observe/issues/3

Acknowledged. I guess you could always throw if an implementation does not want to support it.

yeah, it would be fine to throw notsupported from deliverChanges if it can't be supported.

We're hoping to pipe Observable.changes into their Change Detector API when available.

Ah, I get where you're coming from now. That'd be awesome! Yeah let me know how we can fix the API. Sounds like #10 is the first step. Basically you'd never want dirty checking, only manual notification?

Any ETA or experiments I can know about? (Not blocking, just interested)

Well I'd experiment first in https://github.com/dart-lang/dev_compiler, since it's our place for experimental ES6+ stuff. If it works we can do something similar in dart2js. Opened https://github.com/dart-lang/dev_compiler/issues/119

I can upload a .tar of the same class with and without a mixin and what the dart2js looks like.

Ah, no worries. Your first message sounded like speculation. I believe you if you've done the analysis. Is there a bug to track this problem? If not, would be very good to file it so dart2js team is aware. http://dartbug.com/new

(we're working on https://github.com/dart-lang/dev_compiler in hopes of finding fixes to that sort of issue. That's a longer term thing though.)

You can certainly use it as a base class:

class Monster extends ChangeNotifier {
  // manual notification
}

would that work?

I mean in general, we can't really avoid mixins. Even Iterable is a mixin ... the core libraries use them all over... As a library designer I don't know how to process "don't use this language feature" especially if the style advice so far has been "use this language feature". But I'm happy to try and find a pragmatic balance if we can strike one. If you avoid mixins, what pattern would you use instead for sharing code between classes?

jmesserly commented 9 years ago

how about we fork the angular discussion into another bug? it sounds like a neat topic. opening one now...

jmesserly commented 9 years ago

filed #11 ... let me know if there are more issues we can open based on this thread.

jmesserly commented 7 years ago

woooohoooooooo 🎉