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

lib/src/codegen/js_codegen.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library ddc.src.codegen.js_codegen;
 
 import 'dart:io' show Directory, File;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 1698 matching lines @@
   final JSCodeOptions options;
 
   JSGenerator(String outDir, Uri root, TypeRules rules, this.options)
       : super(outDir, root, rules);
 
   void generateLibrary(Iterable<CompilationUnit> units, LibraryInfo info,
       CheckerReporter reporter) {
     JS.Block jsTree =
         new JSCodegenVisitor(info, rules).generateLibrary(units, reporter);
 
-    var outputPath = path.join(outDir, jsOutputPath(info));
+    var outputPath = path.join(outDir, jsOutputPath(info, root));
     new Directory(path.dirname(outputPath)).createSync(recursive: true);
 
     if (options.emitSourceMaps) {
       var outFilename = path.basename(outputPath);
       var printer = new srcmaps.Printer(outFilename);
       var context =
           new SourceMapPrintingContext(printer, path.dirname(outputPath));
       _writeLibrary(context, jsTree);
       printer.add('//# sourceMappingURL=$outFilename.map');
       // Write output file and source map
@@ skipping 21 matching lines @@
   new JS.Printer(opts, context).visit(node);
   // Write output file and source map
   return context.getText();
 }
 
 /// Choose a canonical name from the library element.
 /// This never uses the library's name (the identifier in the `library`
 /// declaration) as it doesn't have any meaningful rules enforced.
 String jsLibraryName(LibraryElement library) => canonicalLibraryName(library);
 
-/// Path to file that will be generated for [info].
-// TODO(jmesserly): library directory should be relative to its package
-// root. For example, "package:dev_compiler/src/codegen/js_codegen.dart" would be:
-// "ddc/src/codegen/js_codegen.js" under the output directory.
-String jsOutputPath(LibraryInfo info) => '${info.name}/${info.name}.js';
+/// Path to file that will be generated for [info]. In case it's url is a
+/// `file:` url, we use [root] to determine the relative path from the entry
+/// point file.
+String jsOutputPath(LibraryInfo info, Uri root) {
+  var uri = info.library.source.uri;
+  var filepath = '${path.withoutExtension(uri.path)}.js';
+  if (uri.scheme == 'dart') {
+    filepath = 'dart/$filepath';
+  } else if (uri.scheme == 'package') {
+    filepath = 'packages/$filepath';

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

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]

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]

Done. I updated the CL to match Option A.

+  } else if (uri.scheme == 'file') {
+    filepath = path.relative(filepath, from: path.dirname(root.path));
+  }
+  return filepath;
+}
 
 class SourceMapPrintingContext extends JS.JavaScriptPrintingContext {
   final srcmaps.Printer printer;
   final String outputDir;
 
   CompilationUnit unit;
   Uri uri;
 
   SourceMapPrintingContext(this.printer, this.outputDir);
 
@@ skipping 45 matching lines @@
 
     // TODO(jmesserly): in many cases marking the end will be unncessary.
     printer.mark(_location(node.end));
   }
 
   String _getIdentifier(AstNode node) {
     if (node is SimpleIdentifier) return node.name;
     return null;
   }
 }