Only pass an AssetId to isPrimary and declareOutputs.

Only pass an AssetId to isPrimary and declareOutputs.

BUG=16442
R=blois@google.com, efortuna@google.com, rnystrom@google.com, sigmund@google.com

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

[nweiz, 2014-04-03 21:03:48]

I wanted to send this out for review quickly so that I could start working on
patches that build on top of it, but I'm going to hold off on committing it
until Siggi releases the next version of Polymer. Once I commit, I'll release
barback 0.13.0 immediately.

Bob: To keep this patch small, I avoided any internal changes to barback that
would take advantage of the new transformer API. I'll include those in a future
CL.

Siggi/Pete: I updated the version of code_transformers since I changed the
pubspec, please make sure I updated it to the right thing.

[blois, 2014-04-03 21:16:36]

https://codereview.chromium.org/223553008/diff/20001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transformer.dart:62: Future<bool> isPrimary(AssetId id) {
Make it not return a future pretty please!! (or at least make it be a union type
Future<bool>|bool (hah!))

[nweiz, 2014-04-03 21:34:17]

https://codereview.chromium.org/223553008/diff/20001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transformer.dart:62: Future<bool> isPrimary(AssetId id) {
On 2014/04/03 21:16:36, blois wrote:
> Make it not return a future pretty please!! (or at least make it be a union
type
> Future<bool>|bool (hah!))

I think we're planning to do the union type thing, but not in this CL.

[Bob Nystrom, 2014-04-03 22:28:28]

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transformer.dart:46: /// Returns `true` if [inputId] can be
a primary input for this transformer.
[id]

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/package...
File pkg/barback/test/package_graph/transform/pass_through_test.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/package...
pkg/barback/test/package_graph/transform/pass_through_test.dart:249:
//expectAssetDoesNotComplete("app|foo.txt");
?

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/transfo...
File pkg/barback/test/transformer/bad.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/transfo...
pkg/barback/test/transformer/bad.dart:25: Future<bool> doIsPrimary(_) => new
Future.value(true);
I'd prefer the full "AssetId id" here even though it isn't used. Here and
elsewhere.

https://codereview.chromium.org/223553008/diff/40001/samples/third_party/drom...
File samples/third_party/dromaeo/pubspec.yaml (right):

https://codereview.chromium.org/223553008/diff/40001/samples/third_party/drom...
samples/third_party/dromaeo/pubspec.yaml:9: barback: ">=0.13.0-dev <0.14.0"
Why "-dev" here and not on any of the other modified pubspecs?

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:69: //
TODO(nweiz): If/when barback starts reporting what assets were modified,
Reference tracking bug.

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:82: var provider
= new _BarbackCompilerProvider(_environment, transform,
The indentation is a bit much here. How about breaking from here on out into a
separate method too?

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:106: "packages"),
Does this need to be wrapped?

[nweiz, 2014-04-03 23:03:50]

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transformer.dart:46: /// Returns `true` if [inputId] can be
a primary input for this transformer.
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> [id]

Done.

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/package...
File pkg/barback/test/package_graph/transform/pass_through_test.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/package...
pkg/barback/test/package_graph/transform/pass_through_test.dart:249:
//expectAssetDoesNotComplete("app|foo.txt");
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> ?

Leftover from debugging, removed.

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/transfo...
File pkg/barback/test/transformer/bad.dart (right):

https://codereview.chromium.org/223553008/diff/40001/pkg/barback/test/transfo...
pkg/barback/test/transformer/bad.dart:25: Future<bool> doIsPrimary(_) => new
Future.value(true);
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> I'd prefer the full "AssetId id" here even though it isn't used. Here and
> elsewhere.

Done.

https://codereview.chromium.org/223553008/diff/40001/samples/third_party/drom...
File samples/third_party/dromaeo/pubspec.yaml (right):

https://codereview.chromium.org/223553008/diff/40001/samples/third_party/drom...
samples/third_party/dromaeo/pubspec.yaml:9: barback: ">=0.13.0-dev <0.14.0"
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> Why "-dev" here and not on any of the other modified pubspecs?

Leftover from before I decided to just release. Removed.

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:69: //
TODO(nweiz): If/when barback starts reporting what assets were modified,
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> Reference tracking bug.

Done.

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:82: var provider
= new _BarbackCompilerProvider(_environment, transform,
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> The indentation is a bit much here. How about breaking from here on out into a
> separate method too?

Done.

https://codereview.chromium.org/223553008/diff/40001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:106: "packages"),
On 2014/04/03 22:28:29, Bob Nystrom wrote:
> Does this need to be wrapped?

Nope.

[Bob Nystrom, 2014-04-03 23:06:12]

lgtm

[blois, 2014-04-04 00:31:38]

On 2014/04/03 23:06:12, Bob Nystrom wrote:
> lgtm

code_transformers changes lgtm

[Emily Fortuna, 2014-04-04 15:40:26]

dromaeo changes lgtm

[Siggi Cherem (dart-lang), 2014-04-04 16:02:31]

https://codereview.chromium.org/223553008/diff/60001/pkg/barback/pubspec.yaml
File pkg/barback/pubspec.yaml (right):

https://codereview.chromium.org/223553008/diff/60001/pkg/barback/pubspec.yaml...
pkg/barback/pubspec.yaml:34: sdk: ">=1.0.1 <2.0.0"
should this include a 1.4 dependency?

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
File pkg/observe/lib/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
pkg/observe/lib/transformer.dart:57: return;
I believe in this case we want to pass the asset through (like in line 68
below?)

[nweiz, 2014-04-04 17:59:35]

https://codereview.chromium.org/223553008/diff/60001/pkg/barback/pubspec.yaml
File pkg/barback/pubspec.yaml (right):

https://codereview.chromium.org/223553008/diff/60001/pkg/barback/pubspec.yaml...
pkg/barback/pubspec.yaml:34: sdk: ">=1.0.1 <2.0.0"
On 2014/04/04 16:02:31, Siggi Cherem (dart-lang) wrote:
> should this include a 1.4 dependency?

I don't believe barback is using anything 1.4 specific, although older SDKs will
de facto avoid installing this version due to pub's constraint.

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
File pkg/observe/lib/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
pkg/observe/lib/transformer.dart:57: return;
On 2014/04/04 16:02:31, Siggi Cherem (dart-lang) wrote:
> I believe in this case we want to pass the asset through (like in line 68
> below?)

Assets are now passed through by default.

[nweiz, 2014-04-08 22:42:24]

I've added compatibility with older versions of barback to other packages.
Siggi, Pete, PTAL

[Siggi Cherem (dart-lang), 2014-04-08 22:56:19]

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
File pkg/observe/lib/transformer.dart (right):

https://codereview.chromium.org/223553008/diff/60001/pkg/observe/lib/transfor...
pkg/observe/lib/transformer.dart:57: return;
On 2014/04/04 17:59:36, nweiz wrote:
> On 2014/04/04 16:02:31, Siggi Cherem (dart-lang) wrote:
> > I believe in this case we want to pass the asset through (like in line 68
> > below?)
> 
> Assets are now passed through by default.

cool, i'll do a cleanup on this afterwards.

https://codereview.chromium.org/223553008/diff/80001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/223553008/diff/80001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/script_compactor.dart:52: Future<bool>
isPrimary(AssetId id) =>
we seem to be missing the fix here

[nweiz, 2014-04-08 22:58:07]

https://codereview.chromium.org/223553008/diff/80001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/223553008/diff/80001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/script_compactor.dart:52: Future<bool>
isPrimary(AssetId id) =>
On 2014/04/08 22:56:20, Siggi Cherem (dart-lang) wrote:
> we seem to be missing the fix here

Oops, done.

[Siggi Cherem (dart-lang), 2014-04-08 22:58:45]

lgtm, thanks!

[nweiz, 2014-04-08 23:46:43]

Committed patchset #6 manually as r34853 (presubmit successful).