Bring back initPolymer, allow boot.js only if using "polymer_experimental.html".

Bring back initPolymer, allow boot.js only if using "polymer_experimental.html".

R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36578

[Siggi Cherem (dart-lang), 2014-05-22 21:10:41]

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp
File pkg/pkg.gyp (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp#newcode21
pkg/pkg.gyp:21: '"polymer/e2e_test/"])',
we were mixing tests and examples together. I moved the tests to "e2e_test" in
the process of cleaning up here.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/import_inliner.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/import_inliner.dart:147: _extractScripts(doc, id);
FYI: I first started with a naive revert, but to support both the new and old
style with polymer_experimental I had to change how things work for initPolymer.


Here there would be a call to _removeScripts, and in _removeScripts there would
be additional warnings that could only be reasonable if you call it after every
inline. (For reference, this is what the original diff looked like:
https://code.google.com/p/dart/source/diff?spec=svn34753&r=34753&format=side&...)

There were a few warnings that cannot be reported in _extractScripts for the
same reason. I made sure that those warnings are still reported in the linter
tough.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
File pkg/polymer/lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
pkg/polymer/lib/src/mirror_loader.dart:26: import 'package:polymer/boot.dart'
show discoverScripts, ScriptInfo;
Q: I wonder if I should just move the code that is left in boot.dart here. I
kept it there to make the review simpler too.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:15: html5lib: '>=0.11.0-dev <0.12.0'
not sure if I could just expand between 0.10 and 0.12. What was the breaking
change to 0.11? does it require fixing here?

I for a moment used the better query syntax, but reverted those changes so I
could keep this as it was.

[Jennifer Messerly, 2014-05-23 07:20:05]

lgtm

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp
File pkg/pkg.gyp (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp#newcode21
pkg/pkg.gyp:21: '"polymer/e2e_test/"])',
On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> we were mixing tests and examples together. I moved the tests to "e2e_test" in
> the process of cleaning up here.

much better :)

out of curiosity could it be under test/e2e?

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/load...
File pkg/polymer/lib/src/loader.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/load...
pkg/polymer/lib/src/loader.dart:27: ///   * Include some style to prevent flash
of unstyled content (FOUC)
just curious, is the FOUC code still there? I thought it was mostly gone now
(well, it's there but handled purely in JS). I could be wrong tho.... maybe I'm
thinking of my ported changes

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
File pkg/polymer/lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
pkg/polymer/lib/src/mirror_loader.dart:26: import 'package:polymer/boot.dart'
show discoverScripts, ScriptInfo;
On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> Q: I wonder if I should just move the code that is left in boot.dart here. I
> kept it there to make the review simpler too.

I guess the main reason is if you want it to be public? Otherwise I would move
it here.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:15: html5lib: '>=0.11.0-dev <0.12.0'
On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> not sure if I could just expand between 0.10 and 0.12. What was the breaking
> change to 0.11? does it require fixing here?
> 
> I for a moment used the better query syntax, but reverted those changes so I
> could keep this as it was. 

+1 for depending on the newer stuff. I think html5lib can be released:
https://codereview.chromium.org/281613003/diff/1/pkg/third_party/html5lib/pub...
:)

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/custom_...
File pkg/polymer/test/custom_event_test.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/custom_...
pkg/polymer/test/custom_event_test.dart:46: initPolymer();
I wonder if this should use the initPolymer.run(...) pattern?

(our old tests were kind of inconsistent -- and it often doesn't matter because
the test isn't testing data binding -- but might be nice to keep things zoned,
as @initMethod was doing)

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/event_p...
File pkg/polymer/test/event_path_test.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/event_p...
pkg/polymer/test/event_path_test.dart:37: initPolymer();
hmmm, I wonder if this should be sooner? Not sure the use case of initPolymer
after Polymer.register ... but it is interesting that it works :). I'm guessing
we had this previously? I wonder if it was on purpose.

https://codereview.chromium.org/293023008/diff/40001/samples/third_party/todo...
File samples/third_party/todomvc/pubspec.yaml (right):

https://codereview.chromium.org/293023008/diff/40001/samples/third_party/todo...
samples/third_party/todomvc/pubspec.yaml:5: polymer: ">=0.10.0-pre.1 <0.11.0"
leave this at .7 ?

[Siggi Cherem (dart-lang), 2014-05-23 17:56:32]

thanks John!

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp
File pkg/pkg.gyp (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/pkg.gyp#newcode21
pkg/pkg.gyp:21: '"polymer/e2e_test/"])',
On 2014/05/23 07:20:06, John Messerly wrote:
> On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> > we were mixing tests and examples together. I moved the tests to "e2e_test"
in
> > the process of cleaning up here.
> 
> much better :)
> 
> out of curiosity could it be under test/e2e?

I think yes. I'm happy to change it, but I'm slighly inclined to keep it as it
is. Like 'examples/', 'test/' will have a packages folder, but 'e2e_test' will
not. By naming it e2e_test we could more easily run these tests by actually
calling pub-get on the e2e test and not be worried about packages symlinks that
may be created from the pubspec in polymer.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/load...
File pkg/polymer/lib/src/loader.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/load...
pkg/polymer/lib/src/loader.dart:27: ///   * Include some style to prevent flash
of unstyled content (FOUC)
On 2014/05/23 07:20:06, John Messerly wrote:
> just curious, is the FOUC code still there? I thought it was mostly gone now
> (well, it's there but handled purely in JS). I could be wrong tho.... maybe
I'm
> thinking of my ported changes

Good point. This commment came with the revert, but I didn't check if it was
accurate still. Done.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
File pkg/polymer/lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/lib/src/mirr...
pkg/polymer/lib/src/mirror_loader.dart:26: import 'package:polymer/boot.dart'
show discoverScripts, ScriptInfo;
On 2014/05/23 07:20:06, John Messerly wrote:
> On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> > Q: I wonder if I should just move the code that is left in boot.dart here. I
> > kept it there to make the review simpler too.
> 
> I guess the main reason is if you want it to be public? Otherwise I would move
> it here.

Done. Moved it here. At least now it is structured in a way that it can be
imported more easily if we rewrite the boot.js as boot.dart in the future.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:15: html5lib: '>=0.11.0-dev <0.12.0'
On 2014/05/23 07:20:06, John Messerly wrote:
> On 2014/05/22 21:10:41, Siggi Cherem (dart-lang) wrote:
> > not sure if I could just expand between 0.10 and 0.12. What was the breaking
> > change to 0.11? does it require fixing here?
> > 
> > I for a moment used the better query syntax, but reverted those changes so I
> > could keep this as it was. 
> 
> +1 for depending on the newer stuff. I think html5lib can be released:
>
https://codereview.chromium.org/281613003/diff/1/pkg/third_party/html5lib/pub...
> :)

yeah, I pushed a -dev version to test it out :). Are we ready to push 0.11? I'm
happy to do it, but wanted to double check.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/custom_...
File pkg/polymer/test/custom_event_test.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/custom_...
pkg/polymer/test/custom_event_test.dart:46: initPolymer();
On 2014/05/23 07:20:06, John Messerly wrote:
> I wonder if this should use the initPolymer.run(...) pattern?
> 
> (our old tests were kind of inconsistent -- and it often doesn't matter
because
> the test isn't testing data binding -- but might be nice to keep things zoned,
> as @initMethod was doing)

Done.

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/event_p...
File pkg/polymer/test/event_path_test.dart (right):

https://codereview.chromium.org/293023008/diff/40001/pkg/polymer/test/event_p...
pkg/polymer/test/event_path_test.dart:37: initPolymer();
On 2014/05/23 07:20:06, John Messerly wrote:
> hmmm, I wonder if this should be sooner? Not sure the use case of initPolymer
> after Polymer.register ... but it is interesting that it works :). I'm
guessing
> we had this previously? I wonder if it was on purpose.

good catch. I am as curious. It seems to be working fine moving this to the top,
so I did.

https://codereview.chromium.org/293023008/diff/40001/samples/third_party/todo...
File samples/third_party/todomvc/pubspec.yaml (right):

https://codereview.chromium.org/293023008/diff/40001/samples/third_party/todo...
samples/third_party/todomvc/pubspec.yaml:5: polymer: ">=0.10.0-pre.1 <0.11.0"
On 2014/05/23 07:20:06, John Messerly wrote:
> leave this at .7 ?

.7 specifically is a problem because it requires the ';component=1', I was
thinking that by setting it as .1 it was OK because it is back how it used to be
before. Thinking mroe about it, I think I should just use the latest. I assigned
it to .13

[Siggi Cherem (dart-lang), 2014-05-23 18:32:13]

Committed patchset #5 manually as r36578 (presubmit successful).