Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Bind to object eclosed in if template broken in polymer expressions 0.10.0-pre.2 #18792

Closed
DartBot opened this Issue May 13, 2014 · 7 comments

Comments

Projects
None yet
5 participants

DartBot commented May 13, 2014

This issue was originally filed by @tomaszkubacki


When you press select currency button in 0.10.0-pre.1 correct currency will be selected (PLN) but in 0.10.0-pre.2 after press USD will be shown.

Full project in attachment

<polymer-element name="binding-selector-issue">
  <template>

<template if="{{showSelector}}">
  <select value="{{document.currencyId | asInteger}}">
     <option template repeat="{{currency in model.currencies}}" value="{{currency.id}}">{{currency.name}}</option>
  </select>
</template>

<button on-click="{{selectCurrency}}">select currency</button>
   
  </template>
  <script type="application/dart;component=1" >
  import 'package:polymer/polymer.dart';
  import 'package:polymer_expressions/filter.dart';
 
  @­CustomTag('binding-selector-issue')
  class BindingSlectorIssue extends PolymerElement {
    
    @­observable MyModel model = new MyModel();
    
    @­observable bool showSelector = false;
    
    final Transformer asInteger = new StringToInt();

    BindingSlectorIssue.created() : super.created() {
    }
    
    void selectCurrency(){
             
      document = new Document()
      ..currencyId = 2; ;
      
      showSelector = true;
    }
    
    @­observable Document document = new Document();
    
    @­observable int currencyId = 0;
  }

  class Document extends Observable {
    @­observable int currencyId = 0;
  }

  class MyModel extends Observable{
    @­observable List currencies = toObservable([
                                                new Currency()
                                                ..id = 1
                                                ..name = "USD",
                                                new Currency()
                                                ..id = 2
                                                ..name = "PLN",
                                                ]);
  }

  class StringToInt extends Transformer<String, int> {
    final int radix;
    StringToInt({this.radix: 10});
    String forward(int i) => '$i';
    int reverse(String s) => s == null ? null : int.parse(s, radix: radix, onError: (s) => null);
  }

  class Currency extends Observable{
    
    @­observable int id = 0;
    
    @­observable String name = '';
    
  }
  
  
  </script>
</polymer-element>

Member

floitschG commented May 13, 2014

Added Area-Polymer, Triaged labels.

Member

sigmundch commented May 29, 2014

Ok, got more progress on this. It seems that my "fix" in r35868 for losing the cursor position in two-way bindings, created this issue.

Here is a smaller repro without using transformers or observable lists:
    <link rel=import href="packages/polymer/polymer.html">
    
    <polymer-element name="binding-selector-issue">
      <template>
        <select value="{{x}}">
        <option template repeat="{{item in items}}" value="{{item}}">{{item}}</option>
        </select>
      </template>
    </polymer-element>
    
    <script type="application/dart">
      import 'package:polymer/polymer.dart';
      export 'package:polymer/init.dart';
      import 'dart:async';
     
      @­CustomTag('binding-selector-issue')
      class BindingSlectorIssue extends PolymerElement {
        final List items = ["a", "b"];
        @­observable String x = 'b';
    
        BindingSlectorIssue.created() : super.created();
      }
    
    </script>
    <binding-selector-issue></binding-selector-issue>

And here is another repro that uses only polymer_expressions, but doesn't use polymer elements:
    
    <script type="application/dart">
    import 'dart:html';
    import 'dart:async';
    import 'package:observe/observe.dart';
    import 'package:observe/src/dirty_check.dart' show dirtyCheckZone;
    import 'package:polymer_expressions/polymer_expressions.dart';
    import 'package:template_binding/template_binding.dart' show templateBind;
    
    main() => dirtyCheckZone().run(() {
      var list = const ['a', 'b'];
      var template = new Element.html(
        '<template>'
        '<select value="{{x}}">'
          '<option template repeat="{{i in y}}" value="{{i}}">item {{i}}'
          '</option></select>'
        '</template>', treeSanitizer: new NullTreeSanitizer());
      var model = new NotifyModel('b', list);
      model.changes.listen((c) => print("change: ${model.x}"));
      var div = new DivElement();
      document.body.append(div);
      div.append(templateBind(template)
          .createInstance(model, new PolymerExpressions()));
    
      new Future(() {
        print('model: ${model.x}');
        print('value: ${div.querySelector('select').value}');
      });
    });
    
    @­reflectable
    class NotifyModel extends ChangeNotifier {
      var _x;
      var _y;
      NotifyModel([this._x, this._y]);
    
      get x => _x;
      set x(value) {
        _x = notifyPropertyChange(#x, _x, value);
      }
      get y => _y;
      set y(value) {
        _y = notifyPropertyChange(#y, _y, value);
      }
    }
    
    class NullTreeSanitizer implements NodeTreeSanitizer {
      void sanitizeTree(Node node) {}
    }
    </script>

Member

sigmundch commented May 30, 2014

I've been looking more into it and I am more inclined to believe that my fixes in polymer-expressions were wrong. In fact, I can reproduce the 2-way binding bug if I replicate the same test, but use the default delegate based on path observers.

It might be a good idea to revert the previous fix to bring things back to a consistent state between the two kind of delegates for the time being.


Added Started label.

Member

sigmundch commented May 30, 2014

Looking more closely, the fixes we did in polymer-expressions made it match much closer to the original binding delegate, so I'm not a fan of reverting the old fix either. I opened issue #19105 to separately investigate the two-way binding problem in the default syntax.

I discovered today that if you use PolymerExpressions with the default delegate syntax, you can work around this bug. For example, replace:

'<option template repeat="{{i in y}}" value="{{i}}">item {{i}}'

with:
'<option template repeat="{{y}}" value="{{}}">item {{}}'

and the problem will be avoided.

The issue is that polymer-expressions is creating scope objects multiple times for the "x in y" comprehension. These objects are not comparable, so the system thinks it needs to notify about it as if it was a change. The earlier fix that eliminated the second notification on two-way bindings exposed this problem because it removed an extra notification that hid this underlying problem (2 wrongs make it right?).

Here is a proposed fix for the issue with comprehensions: https://codereview.chromium.org/304223004/

Member

jmesserly commented May 31, 2014

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

Member

sigmundch commented May 31, 2014

fixed in r36848


Added Fixed label.

DartBot commented Jun 5, 2015

This issue has been moved to dart-lang/polymer-dart#180.

This issue was closed.

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