Getters aren't observable #12473

Closed
sethladd opened this Issue Aug 14, 2013 · 16 comments

Comments

Projects
None yet
6 participants
Member

sethladd commented Aug 14, 2013

In the old Web UI days, a getter was observable. In the polymer days, getters aren't observable. That is, if a field that a getter changes, the system doesn't realize that the getter's value changed.

One idea: enable the concept of "observable computed properties". Maybe like this:

@observable('a', 'b', 'c')
int get foo => a + b + c;

Member

jmesserly commented Aug 14, 2013

TL;DR observables are never going to be as seamless as watchers.

I've certainly thought a lot about this. There are various problems. I'll start with the easy ones :)

@observable is a top level variable, not a constructor. If we switched it to a type, you'd need @­observable() with parens, and it would violate naming guidelines. It can't be uppercase Observable since that's used for something else. Also it would need to take Symbol not strings.

There are more fundamental issues:

  1. It duplicates information inside the getter, and there's no way we can check if it is correct. I can already see the bug reports: "compiler should warn me when I forgot to list a dependency..." ;-)
  2. It won't work at all for more complex cases like a.b + c.d. I can already see the endless feature requests. Before you know it we'll have invented a fully analyzable expression language and embedded it in an annotation inside Dart. :)
  3. It isn't clear the getter form has any hope of making it into the language, whereas the simple @­observable has a shot. Adding more bells & whistles might reduce the changes we get something really good in the long run.
  4. Polymer Expressions (formerly known as Fancy Syntax) address most of these use cases. Maybe what we really want here is some technique to get Polymer Expressions (which are analyzable) conveniently defined in Dart code.
  5. There are other patterns in code that you can use. There are top-level helper functions like bindProperty that make it easy to cause a change to one property to imply a change to another property. Alternatively, you can make the field mutable and recompute it when the dependencies change. We can try to improve on these techniques; they don't require new features from the observable system. Because we don't have convenient DartDoc, most people are probably not aware of these patterns.

I'm tempted to WontFix since I don't see a solution here. I guess we can leave the bug open for a while and see if any brilliant ideas come around. Personally, I'd like to push on the existing patterns that Dart and JS can already use.


Removed Priority-Unassigned label.
Added Priority-Low, Library-Observe labels.

Member

jmesserly commented Aug 14, 2013

oops, typo: "reduce the changes" should be "reduce the chances"

Member

jmesserly commented Aug 14, 2013

another thought, we haven't ported all of the Polymer convenience. they make it really easy to create a "fooChanged" method that is called whenever foo changes, letting you recompute properties. Maybe there's more magic too; we aren't caught up yet.

Member

sethladd commented Aug 15, 2013

Thanks John. Here's what I have which can work for now:

import 'dart:html';
import 'package:polymer/polymer.dart';

class App extends Object with ObservableMixin {
  @­observable
  bool blue = false;
  @­observable
  bool green = false;
  @­observable
  bool red = false;
  
  App() {
    bindProperty(this, const Symbol('red'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
    bindProperty(this, const Symbol('green'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
    bindProperty(this, const Symbol('blue'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
  }
  
  String get favoriteColor {
    String value = '';
    if (red) {
      value = 'red';
    } else if (green) {
      value = 'green';
    } else if (blue) {
      value = 'blue';
    }
    return value;
  }
}

main() {
  query('#tmpl').model = new App();
}

(BTW, for anyone reading, don't run this code with your editor set to break on all exceptions. There's some "perfectly fine" exception that's thrown here that we don't care about.)

Member

jmesserly commented Aug 15, 2013

(BTW, for anyone reading, don't run this code with your editor set to break on all exceptions. There's some "perfectly fine" exception that's thrown here that we don't care about.)

Is that int.parse? :(
I'm tempted to write my own int parse function because it's so annoying...

Member

sethladd commented Aug 15, 2013

yes! int.parse. Why are we parsing a path section as an int? I don't understand :)

Member

jmesserly commented Aug 15, 2013

its an MDV thing. in default syntax, this is a valid path: a.1.b.2.c

Member

sigmundch commented Aug 15, 2013

btw - I think the exception only hits when parsing fails (when onError is called),
so it might be fine to do a quick regex to check if it looks numeric, and call int.parse in that case.

Ideally this should be fixed in the debugging API: if you provide an onerror, the debugger shouldn't stop there.

Member

DartBot commented Sep 2, 2013

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


Need observable getter!!!

class App extends Object with ObservableMixin {
  @­observable
  bool blue = false;
  @­observable
  bool green = false;
  @­observable
  bool red = false;
  
  App() {
    bindProperty(this, const Symbol('red'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
    bindProperty(this, const Symbol('green'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
    bindProperty(this, const Symbol('blue'),
        () => notifyProperty(this, const Symbol('favoriteColor')));
  }
  
  String get favoriteColor {
    String value = '';
    if (red) {
      value = 'red';
    } else if (green) {
      value = 'green';
    } else if (blue) {
      value = 'blue';
    }
    return value;
  }
}

Too complicated, not as:

class App extends Object with ObservableMixin {
  @­observable
  bool blue = false;
  @­observable
  bool green = false;
  @­observable
  bool red = false;
  
  String favoriteColor(bool red, bool green, bool blue) {
    String value = '';
    if (red) {
      value = 'red';
    } else if (green) {
      value = 'green';
    } else if (blue) {
      value = 'blue';
    }
    return value;
  }
}
Polymer_expressions can run correctly.

Member

sethladd commented Sep 7, 2013

Could we make this a bit easier for developers?

observeGetter(#getterName, [#field, #field, #field])

Which is a convenience method for the three bindProperties.

Thoughts?


cc @jmesserly.

Member

sethladd commented Sep 21, 2013

See https://code.google.com/p/dart/issues/detail?id=13458 for enhancements to make observable getters easier.

Member

sigmundch commented Oct 7, 2013

Added this to the M8 milestone.

Member

sigmundch commented Oct 9, 2013

Added Duplicate label.
Marked as being merged into #13458.

Owner

anders-sandholm commented Feb 6, 2014

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

Member

DartBot commented May 7, 2014

This comment was originally written by William.full...@gmail.com


This problem is still present in v1.4.0. As far as I can tell, it only works on getter-s to give the value set in the constructor.

sethladd added this to the M8 milestone May 7, 2014

Member

DartBot commented Jun 5, 2015

This issue has been moved to dart-lang/observe#33.

This issue was closed.

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