Fixing layout in js output (use full paths rather than just the library name)

Fixing layout in js output (use full paths rather than just the library name)

R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/b22268905f792c72e9179b43900c4d4483cda5bf

[Siggi Cherem (dart-lang), 2015-03-02 17:34:06]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2015-03-02 17:39:24]

sigmund@google.com changed reviewers:
+ jmesserly@google.com

[Siggi Cherem (dart-lang), 2015-03-02 17:39:27]

Addresses #79 for js. I plan to address the Dart output separately

[Siggi Cherem (dart-lang), 2015-03-02 17:39:39]

Patchset #1 (id:10001) has been deleted

[Jennifer Messerly, 2015-03-02 18:40:51]

the code change looks great, I'm just wondering about the mapping.

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1770: filepath = 'packages/$filepath';
hmmm. I wonder about this. Does it make sense in an incremental compilation
world? If I point dev_compiler at lib/devc.dart, won't that file itself be
treated as outside of the package? In other words, how do I start compiling a
package.

Also, do we get conflicts with "packages/" symlink on disk, if present? The file
could end up being written into the pub cache.

I was expecting something like this, but without the "packages/" prefix.

It seems like interop is also challenging if "packages/" is hardcoded, e.g. how
does this play with ES6 packages?

[Siggi Cherem (dart-lang), 2015-03-02 18:57:29]

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1770: filepath = 'packages/$filepath';
On 2015/03/02 18:40:51, John Messerly wrote:
> hmmm. I wonder about this. Does it make sense in an incremental compilation
> world? If I point dev_compiler at lib/devc.dart, won't that file itself be
> treated as outside of the package? In other words, how do I start compiling a
> package.
> 
> Also, do we get conflicts with "packages/" symlink on disk, if present? The
file
> could end up being written into the pub cache.
> 
> I was expecting something like this, but without the "packages/" prefix.
> 
> It seems like interop is also challenging if "packages/" is hardcoded, e.g.
how
> does this play with ES6 packages?

Good point - this also brings a related question about what to do with respect
to entrypoints from `web/` and other folders.

One one hand, these are app's entry points, so it's not something I expect to
build separately. On the other hand, it might make the output easier to
understand if they are also within their package layout.

To be more precise, consider package:foo and the files lib/bar.dart,
web/index.dart, web/index.html:

Option A (like we have today, but addressing your point above):
- lib/bar.dart   => out/foo/bar.js
- web/index.dart => out/index.js
- web/index.html => out/index.html

Option B (everything in it's "package"):
- lib/bar.dart   => out/foo/lib/bar.js
- web/index.dart => out/foo/web/index.dart
- web/index.html => out/foo/web/index.html


I'm currently inclined to do Option A, but I'm curious about what you think

[Siggi Cherem (dart-lang), 2015-03-03 02:09:03]

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/968273002/diff/20001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1770: filepath = 'packages/$filepath';
On 2015/03/02 18:57:29, Siggi Cherem (dart-lang) wrote:
> On 2015/03/02 18:40:51, John Messerly wrote:
> > hmmm. I wonder about this. Does it make sense in an incremental compilation
> > world? If I point dev_compiler at lib/devc.dart, won't that file itself be
> > treated as outside of the package? In other words, how do I start compiling
a
> > package.
> > 
> > Also, do we get conflicts with "packages/" symlink on disk, if present? The
> file
> > could end up being written into the pub cache.
> > 
> > I was expecting something like this, but without the "packages/" prefix.
> > 
> > It seems like interop is also challenging if "packages/" is hardcoded, e.g.
> how
> > does this play with ES6 packages?
> 
> Good point - this also brings a related question about what to do with respect
> to entrypoints from `web/` and other folders.
> 
> One one hand, these are app's entry points, so it's not something I expect to
> build separately. On the other hand, it might make the output easier to
> understand if they are also within their package layout.
> 
> To be more precise, consider package:foo and the files lib/bar.dart,
> web/index.dart, web/index.html:
> 
> Option A (like we have today, but addressing your point above):
> - lib/bar.dart   => out/foo/bar.js
> - web/index.dart => out/index.js
> - web/index.html => out/index.html
> 
> Option B (everything in it's "package"):
> - lib/bar.dart   => out/foo/lib/bar.js
> - web/index.dart => out/foo/web/index.dart
> - web/index.html => out/foo/web/index.html
> 
> 
> I'm currently inclined to do Option A, but I'm curious about what you think

Done. I updated the CL to match Option A.

[Jennifer Messerly, 2015-03-03 02:12:46]

lgtm

[Siggi Cherem (dart-lang), 2015-03-03 17:05:14]

Committed patchset #2 (id:30001) manually as
b22268905f792c72e9179b43900c4d4483cda5bf (presubmit successful).