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

pkg:observe -- implement dirty-checking fallback for non-observable objects #58

Closed DartBot closed 9 years ago

DartBot commented 9 years ago

Issue by jmesserly Originally opened as dart-lang/sdk#16342


After landing https://codereview.chromium.org/132403010/ we'll have most of the infrastructure to do this.

Ideal:

* Ideal: @­observable transform does not require ChangeNotifier base class.

* Need a way of marking an object @­observable in another library.

* Do we need an object to be a ChangeNotifier and have some properties dirty-checked? If so, we need a way of attempting to subscribe, and determining if such a property is not available for subscription. I think we need this to make computed properties really easy to use. Alternatively we can throw an error in this case so user at least knows that they forgot to make a property @­observable.

* remove Observable base class -- objects can be observed via PathObserver/CompoundObserver. Add ObjectObserver from observe-js

* ObservableList still provided as the "fast version"

* Add logging/tracing diagnostics about dirty checking to provide developers a way to assess performance impact of dirty checking -- how many objects, which objects, how often was dirty checking run and how long did it take?

* For best performance, this should be combined with an approach that generates fast-paths for expressions found in HTML

* For now, the @­observable transform will not be run in dev mode, meaning those objects will be treated as dirty-checked. If using "pub serve" the transform will happen automatically.

Any flaws in this plan? We've tried this before and not succeeded. I think the big changes since we last looked at this:

DartBot commented 9 years ago

Comment by anders-sandholm


Removed Library-Observe label. Added Pkg-Observe label.

DartBot commented 9 years ago

Comment by jmesserly


if we don't do this, we'll need to fix computed properties.

DartBot commented 9 years ago

Comment by jmesserly


s/fix/make easier ...

DartBot commented 9 years ago

Comment by jmesserly


Issue #40 has been merged into this issue.

DartBot commented 9 years ago

This comment was originally written by joo....@gmail.com


yes, list and map work in web_ui fine, but on polymer has not reached the expected results, never.

DartBot commented 9 years ago

Comment by jmesserly


Removed Area-Polymer label. Added Area-Pkg label.

DartBot commented 9 years ago

Comment by sigmundch


Added this to the Later milestone. Removed Priority-Unassigned label. Added Priority-Medium label.

DartBot commented 9 years ago

Comment by sigmundch


Added Polymer-P-2 label.

DartBot commented 9 years ago

Comment by sigmundch


Removed this from the Later milestone.

DartBot commented 9 years ago

Comment by jmesserly


IMO, should be higher priority... it's a big usability disadvantage compared with other systems.


Removed Priority-Medium, Polymer-P-2 labels. Added Priority-High label.

DartBot commented 9 years ago

Comment by sethladd


High Pri means we have a milestone assigned. Does this mean you're signing up to make it happen in 1.6? :)

DartBot commented 9 years ago

Comment by jmesserly


@Seth -- Sadly no, am not working on this feature area anymore.

DartBot commented 9 years ago

Comment by sigmundch


Since packages have an independent release cycle than the SDK, I don't want to assign any Milestone (it just confuses everyone).

This is important, there are many other things with higher priority for now, so I doubt we'll get to this that soon.


Added Polymer-P-2 label.

DartBot commented 9 years ago

Comment by sigmundch


Removed Polymer-P-2 label. Added Polymer-Milestone-Later label.

DartBot commented 9 years ago

Comment by sigmundch


Removed Polymer-Milestone-Later label. Added PolymerMilestone-Later label.

DartBot commented 9 years ago

Comment by sethladd


Bumping Priority.


Removed Priority-High label. Added Priority-Medium label.

DartBot commented 9 years ago

Comment by blois


It would be great if there were some way to ensure that dirty-checking does not creep into a codebase unexpectedly. From experience in large XAML apps which had a similar slow-path for non-observables we had to have a strict 'must never use the slowpath' rule, but did not have a decent way of enforcing it.

It's a bit tricky because the final component of a binding path may support observing while intermediate steps do not (or the reverse).

This could potentially be mitigated by tools for inspecting binding state of a DOM tree, but as a dev I'd prefer to be able to prevent this entirely.

DartBot commented 9 years ago

Comment by jmesserly


Jake/Siggi, does this seem worth it? if so I can move this to the github tracker, otherwise let's close


cc @jakemac53. cc @sigmundch.

DartBot commented 9 years ago

Comment by jakemac53


lets close imo