Reducing the amount of code we generate in the compiler: We still continue

Reducing the amount of code we generate in the compiler: We still continue
generating all files like before, especially for observable, but this reduces
what we need to generate further. In particular, I moved template pieces back to
the html page, moved initialization code out of the user's main file, and moved
css code out of the webcomponent class

[Siggi Cherem (dart-lang), 2013-07-18 20:12:00]

I'm not planning on submitting this until we fix bug:
https://code.google.com/p/dart/issues/detail?id=11885

but wanted to give you an early look at the changes in the compiler.

[Jennifer Messerly, 2013-07-18 23:56:04]

LGTM

https://codereview.chromium.org/19497002/diff/2001/lib/custom_element.dart
File lib/custom_element.dart (right):

https://codereview.chromium.org/19497002/diff/2001/lib/custom_element.dart#ne...
lib/custom_element.dart:630: void _initCustomElement(Element node, String
localName, CustomElement ctor()) {
do we need this arg?

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart
File lib/polymer_element.dart (right):

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart#n...
lib/polymer_element.dart:74: List<String> _localNames = [];
final?

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart#n...
lib/polymer_element.dart:128: // have Shadow DOM support? In that case, styles
won't have proper
I think this TODO is obsolete :)

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart#n...
lib/polymer_element.dart:130: if (root is ShadowRoot &&
root is always ShadowRoot now, right?

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart#n...
lib/polymer_element.dart:131: declaration.attributes['apply-author-styles'] !=
null) {
does polymer have apply-author-styles?

https://codereview.chromium.org/19497002/diff/2001/lib/polymer_element.dart#n...
lib/polymer_element.dart:140: var syntax = templateNode.attributes['syntax'];
mind adding a TODO? we should treat syntax like Polymer does, which probably
means defaulting to fancy-syntax.

https://codereview.chromium.org/19497002/diff/2001/lib/src/html_css_fixup.dart
File lib/src/html_css_fixup.dart (right):

https://codereview.chromium.org/19497002/diff/2001/lib/src/html_css_fixup.dar...
lib/src/html_css_fixup.dart:22: // TODO(terry): Replace with enum when
supported.
remove TODO?

[Siggi Cherem (dart-lang), 2013-07-19 22:52:52]

PTAL

Thanks John - indeed I verified that running bind-model synchornously was part
of the problem after all. I added the fix here, and fixes in the tests to wait
for the rendering to be done before querying and finding data in the DOM.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/custom_element....
File lib/custom_element.dart (right):

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/custom_element....
lib/custom_element.dart:630: void _initCustomElement(Element node, String
localName, CustomElement ctor()) {
On 2013/07/18 23:56:04, John Messerly wrote:
> do we need this arg?

oops, not anymore =) I removed the use of it, but forgot to remove it. Thanks.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
File lib/polymer_element.dart (right):

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
lib/polymer_element.dart:74: List<String> _localNames = [];
On 2013/07/18 23:56:04, John Messerly wrote:
> final?

Done.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
lib/polymer_element.dart:128: // have Shadow DOM support? In that case, styles
won't have proper
On 2013/07/18 23:56:04, John Messerly wrote:
> I think this TODO is obsolete :)

Done.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
lib/polymer_element.dart:130: if (root is ShadowRoot &&
On 2013/07/18 23:56:04, John Messerly wrote:
> root is always ShadowRoot now, right?

I think so. removed.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
lib/polymer_element.dart:131: declaration.attributes['apply-author-styles'] !=
null) {
On 2013/07/18 23:56:04, John Messerly wrote:
> does polymer have apply-author-styles?

turns out they have it as a property in the prototype, but not in the html, not
sure why. To match them, I removed it from here and turned it into a property in
PolymerElement.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/polymer_element...
lib/polymer_element.dart:140: var syntax = templateNode.attributes['syntax'];
On 2013/07/18 23:56:04, John Messerly wrote:
> mind adding a TODO? we should treat syntax like Polymer does, which probably
> means defaulting to fancy-syntax.

Done.

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/src/html_css_fi...
File lib/src/html_css_fixup.dart (right):

https://chromiumcodereview.appspot.com/19497002/diff/2001/lib/src/html_css_fi...
lib/src/html_css_fixup.dart:22: // TODO(terry): Replace with enum when
supported.
On 2013/07/18 23:56:04, John Messerly wrote:
> remove TODO?

Done.

https://chromiumcodereview.appspot.com/19497002/diff/43001/lib/polymer_elemen...
File lib/polymer_element.dart (right):

https://chromiumcodereview.appspot.com/19497002/diff/43001/lib/polymer_elemen...
lib/polymer_element.dart:142: Timer.run(() => mdv.bindModel(root, this,
so this was the fix after all. Seems to work ok for most cases, but there are 2
tests that still puzzle me: doing a Timer.run to observe the changes is not
enough, sometimes is necessary to do a longer delay (for instance, 200ms).

Any ideas? I'm ok submitting as it is anyways, but let me know what you think.

[Jennifer Messerly, 2013-07-19 23:16:24]

lgtm

https://chromiumcodereview.appspot.com/19497002/diff/43001/lib/polymer_elemen...
File lib/polymer_element.dart (right):

https://chromiumcodereview.appspot.com/19497002/diff/43001/lib/polymer_elemen...
lib/polymer_element.dart:142: Timer.run(() => mdv.bindModel(root, this,
On 2013/07/19 22:52:52, Siggi Cherem (dart-lang) wrote:
> so this was the fix after all. Seems to work ok for most cases, but there are
2
> tests that still puzzle me: doing a Timer.run to observe the changes is not
> enough, sometimes is necessary to do a longer delay (for instance, 200ms).
> 
> Any ideas? I'm ok submitting as it is anyways, but let me know what you think.

not sure... it depends on how Timer.run is implemented (is it equivalent to
setTimeout 0?) ... what could be happening is that this delay changes the order
that MutationObservers are noticing the changes, and whether they're noticing it
before or after observers fire. Alas, async makes it hard to write predictable
code :|

[Siggi Cherem (dart-lang), 2013-07-22 17:02:44]

submitted in:
https://github.com/dart-lang/web-ui/commit/3b6866bd8dcd29fa7aa76eccb59046d904...