Sunday, September 29, 2013

Kicking the Tires on the Proposed new Polymer.dart


After a few days trying, I apply the proposed patch to Polymer.dart in a rather tedious way. So that I can use it in my ICE Code Editor project, I point the project's pubspec.yaml to my local copy of Dart + Packages using a path dependency:
name: ice_code_editor
# ...
dependencies:
  # ...
  polymer: {path: /home/chris/repos/dart/dart/pkg/polymer}
Then, I make similar updates to the core packages that are impacted by the patch. For each, I replace the “any” constraint with a relative path constraint. The diff ends up being longish:
diff --git a/dart/pkg/custom_element/pubspec.yaml b/dart/pkg/custom_element/pubspec.yaml
index e52b080..3e1cf52 100644
--- a/dart/pkg/custom_element/pubspec.yaml
+++ b/dart/pkg/custom_element/pubspec.yaml
@@ -9,6 +9,6 @@ dependencies:
   meta: any
   mutation_observer: any
   # TODO(jmesserly): fix this, we should not depend on MDV.
-  mdv: any
+  mdv: {path: ../mdv}
 dev_dependencies:
   unittest: any
diff --git a/dart/pkg/mdv/pubspec.yaml b/dart/pkg/mdv/pubspec.yaml
index 937a8cc..85c7ff5 100644
--- a/dart/pkg/mdv/pubspec.yaml
+++ b/dart/pkg/mdv/pubspec.yaml
@@ -9,6 +9,6 @@ description: >
 homepage: https://www.dartlang.org/polymer-dart/
 dependencies:
   logging: any
-  observe: any
+  observe: {path: ../observe}
 dev_dependencies:
   unittest: any
diff --git a/dart/pkg/polymer/pubspec.yaml b/dart/pkg/polymer/pubspec.yaml
index acd745f..38206af 100644
--- a/dart/pkg/polymer/pubspec.yaml
+++ b/dart/pkg/polymer/pubspec.yaml
@@ -11,15 +11,15 @@ dependencies:
   barback: any
   browser: any
   csslib: any
-  custom_element: any
+  custom_element: {path: ../custom_element}
   html5lib: any
   html_import: any
   logging: any
-  mdv: any
+  mdv: {path: ../mdv}
   meta: any
-  observe: any
+  observe: {path: ../observe}
   path: any
-  polymer_expressions: any
+  polymer_expressions: {path: ../polymer_expressions}
   shadow_dom: any
   source_maps: any
   yaml: any
diff --git a/dart/pkg/polymer_expressions/pubspec.yaml b/dart/pkg/polymer_expressions/pubspec.yaml
index 3a1d563..f6641b7 100644
--- a/dart/pkg/polymer_expressions/pubspec.yaml
+++ b/dart/pkg/polymer_expressions/pubspec.yaml
@@ -4,7 +4,7 @@ description: An expressive custom binding syntax for MDV templates
 homepage: http://www.dartlang.org/polymer-dart/
 dependencies:
   browser: any
-  mdv: any
-  observe: any
+  mdv: {path: ../mdv}
+  observe: {path: ../observe}
 dev_dependencies:
   unittest: any
The patch affected the polymer, observe, and custom_element core packages. These, along with the other core packages in Dart (the ones maintained by the Dart developers), all reside in the same directory in the main Dart source code repository. So I had to manually update my local copy of polymer so that it depended on observe and custom_element, using relative paths. Unfortunately, both the mdv and polymer_element packages also depended on observe, so they got dragged into the pubspec.yaml fun as well.

This was not horrible, but hopefully there will be easier ways to pin packages in the future. Anyhow, with the preliminaries out of the way, pub install works:
➜  ice-code-editor git:(polymer) ✗ pub install
Resolving dependencies....................
Dependencies installed!
And the new versions of the code are served up via pub serve when I try my code out in Dartium. So how does the patch work with my Polymer.dart code?

I am still trying to read two attributes from my custom <ice-code-editor> element, which is declared as:
<polymer-element name="ice-code-editor" attributes="src,lineNumber">
  <template>
    <h1>The src is "{{src}}" ({{lineNumber}})</h1>
    <content></content>
  </template>
  <script type="application/dart" src="ice_polymer.dart"></script>
</polymer-element>
Just for debugging purposes, I am also including the values of src and lineNumber inside an <h1> tag in this template.

The backing ice_polymer.dart code declares the same src and lineNumber attributes as instance variables in the IceCodeEditorElement class:
import 'package:polymer/polymer.dart';
// ICE-related imports here...
@CustomTag('ice-code-editor')
class IceCodeEditorElement extends PolymerElement with ObservableMixin {
  @observable String src = '';
  @observable int lineNumber = 0;

  void created() {
    super.created();
    // Setup ICE here...

    HttpRequest.getString(src).then((response) {
      editor.content = response;
      editor.editorReady.then((_)=> editor.lineNumber = lineNumber);
    });

  }
}
The src attribute is used to request the content for the editor (it will be located in a separate file). The lineNumber attribute is used to scroll the <ice-code-editor> widget to the correct location in the buffer.

Finally, I use the custom element as:
<head>
  <link rel="import" href="ice_code_editor.html">
  <script src="packages/polymer/boot.js"></script>
</head>
<body>
<!-- other HTML here... -->
<ice-code-editor src="embed_a.html" line-number="49"></ice-code-editor>
<!-- ... -->
And, once all of the pieces—the patch and my local code—are together, it works. Kind of…

The src attribute is working, because when I view the page, the contents are correctly placed in the editor and the <h1> element is correctly updated:



But the line-number attribute / lineNumber instance variable are not working.

Unlike pre-patch, this does not appear to be a problem with types. I can get this working if I use the same name for both the attribute and the instance variable. If, for example, I replace every instance of line-number and lineNumber with line_number in the custom element, its template, and the backing code, it works:



Interestingly, this even works if the instance variables in the backing class are declared without defaults (something that did not work pre-patch):
@CustomTag('ice-code-editor')
class IceCodeEditorElement extends PolymerElement with ObservableMixin {
  @observable String src;
  @observable int line_number;
  // ...
}
I dig through the patched code a bit and it seems that there really is no support for camel-cased attribute mapping as there used to be. Some of the code has survived (it's just no longer used), so hopefully that will make its way back before or after the patch is ready.

While digging around the patched packages, I find that the @observable annotations look to be getting replaced with @published, which has a nicer, semantic feel:
@CustomTag('ice-code-editor')
class IceCodeEditorElement extends PolymerElement with ObservableMixin {
  @published String src;
  @published int line_number;
  // ...
}
So in the end, the patch was a pain to apply, but it seems well worth it. The confusion with type information seems nicely resolved by the patch. I am not too fussed with the lack of hyphenated / camel-case conversion. It seems an easy enough thing to add back at some point in the future and, until then, attributes like line_number are fine by me.


Day #889

2 comments:

  1. Unfortunately, based on John's comments in this discussion thread: https://groups.google.com/a/dartlang.org/d/msg/web-ui/6PZLryVYY_k/UibWZq3sWSIJ It doesn't appear that dash-separated to camelCase will be returning. In order to match closer with Polymer and less with Web UI the mapping process won't be happening again.

    That said, it should still be possible for you to use camelCase in your dart code, and just declare it as 'linenumber' in the polymer attributes. I'm a little bummed by this myself but as you mention it's relatively minor annoyance. I'm looking forward to that patch landing myself as it will correct most of the issues holding me back from porting over my apps to Polymer.dart

    ReplyDelete
    Replies
    1. Thanks for dashing my last hopes against the proverbial rocks! I kid, I kid. It's not a big deal, especially if I know not to expect it to return. That said...

      Using linenumber as an attribute isn't too horrible, but what if I need to further distinguish the scroll-to line number and the line number to place the active cursor in my polymer element. Attributes named scrolltolinenumber and activelinenumber kinda stink. Then again, maybe I would need to use scrollto and activeline instead. I suppose I'll have to keep playing with this stuff to find out more :)

      Delete