Skip to content
This repository has been archived by the owner on Sep 15, 2021. It is now read-only.

Open discussion about the current API #9

Closed
matanlurey opened this issue Apr 1, 2015 · 6 comments
Closed

Open discussion about the current API #9

matanlurey opened this issue Apr 1, 2015 · 6 comments

Comments

@matanlurey
Copy link
Contributor

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
Copy link
Contributor

  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: #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: #10

@matanlurey
Copy link
Contributor Author

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
Copy link
Contributor

BTW, some of the issues with change records are over in #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 dart-archive/dev_compiler#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
Copy link
Contributor

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

@jmesserly
Copy link
Contributor

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

@jmesserly
Copy link
Contributor

woooohoooooooo 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants