fix #626, add AMD module format and make it default

lib/src/compiler/compiler.dart

 // Copyright (c) 2016, 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.
 
 import 'dart:collection' show HashSet, Queue;
 import 'package:analyzer/dart/element/element.dart' show LibraryElement;
 import 'package:analyzer/analyzer.dart'
     show AnalysisError, CompilationUnit, ErrorSeverity;
 import 'package:analyzer/file_system/file_system.dart' show ResourceProvider;
 import 'package:analyzer/src/generated/engine.dart' show AnalysisContext;
@@ skipping 125 matching lines @@
     if (!options.unsafeForceCompile &&
         errors.any((e) => errorSeverity(context, e) == ErrorSeverity.ERROR)) {
       return new JSModuleFile.invalid(unit.name, messages);
     }
 
     var codeGenerator = new CodeGenerator(context, options, _extensionTypes);
     return codeGenerator.compile(unit, trees, messages);
   }
 }
 
-enum ModuleFormat { es6, legacy, node }
+enum ModuleFormat { es6, commonjs, requirejs, legacy }
 
 ModuleFormat parseModuleFormat(String s) => {
       'es6': ModuleFormat.es6,
-      'node': ModuleFormat.node,
+      'commonjs': ModuleFormat.commonjs,
+      'requirejs': ModuleFormat.requirejs,
+      // Deprecated:
+      'node': ModuleFormat.commonjs,
       'legacy': ModuleFormat.legacy
     }[s];
 
 class CompilerOptions {
   /// Whether to emit the source mapping file.
   ///
   /// This supports debugging the original source code instead of the generated
   /// code.
   final bool sourceMap;
 
@@ skipping 54 matching lines @@
 
   const CompilerOptions(
       {this.sourceMap: true,
       this.sourceMapComment: true,
       this.summarizeApi: true,
       this.summaryExtension: 'sum',
       this.unsafeForceCompile: false,
       this.emitMetadata: false,
       this.closure: false,
       this.destructureNamedParams: false,
-      this.moduleFormat: ModuleFormat.legacy,
+      this.moduleFormat: ModuleFormat.requirejs,
       this.hoistInstanceCreation: true,
       this.hoistSignatureTypes: false,
       this.nameTypeTests: true,
       this.hoistTypeTests: true,
       this.useAngular2Whitelist: false});
 
   CompilerOptions.fromArguments(ArgResults args)
       : sourceMap = args['source-map'],
         sourceMapComment = args['source-map-comment'],
         summarizeApi = args['summarize'],
@@ skipping 15 matching lines @@
       ..addOption('summary-extension',
           help: 'file extension for Dart summary files',
           defaultsTo: 'sum',
           hide: true)
       ..addFlag('source-map', help: 'emit source mapping', defaultsTo: true)
       ..addFlag('source-map-comment',
           help: 'adds a sourceMappingURL comment to the end of the JS,\n'
               'disable if using X-SourceMap header',
           defaultsTo: true,
           hide: true)
-      ..addOption('modules',
-          help: 'module pattern to emit',
-          allowed: ['es6', 'legacy', 'node'],
-          allowedHelp: {
-            'es6': 'es6 modules',
-            'legacy': 'a custom format used by dartdevc, similar to AMD',
-            'node': 'node.js modules (https://nodejs.org/api/modules.html)'
-          },
-          defaultsTo: 'legacy')
+    ..addOption('modules',
+        help: 'module pattern to emit',
+        allowed: [
+          'es6',
+          'commonjs',
+          'requirejs',
+          /*deprecated*/ 'legacy',
+          /*renamed to commonjs*/ 'node'
+        ],
+        allowedHelp: {
+          'es6': 'es6 modules',
+          'commonjs': 'commonjs/node.js modules',
+          'requirejs': 'requirejs/amd modules'

[nweiz, 2016/08/16 22:14:53]

Nit: add capitalization to the help strings here.

[Jennifer Messerly, 2016/08/24 22:39:51]

Done.

+        },
+        defaultsTo: 'requirejs')

[vsm, 2016/08/16 21:23:23]

We should keep the default until we fix downstream uses (which probably don't
set this explicitly right now).

[Jennifer Messerly, 2016/08/16 21:29:27]

it's really easy to tweak our internal build rules, if that's what you're
worried about. We can set the formal explicitly long before this flows in.

[Jennifer Messerly, 2016/08/16 21:34:45]

I just sent you a CL that pins internal build rules to --modules=legacy

[Jennifer Messerly, 2016/08/24 22:39:51]

update: and this was landed.

       ..addFlag('emit-metadata',
           help: 'emit metadata annotations queriable via mirrors',
           defaultsTo: false)
       ..addFlag('closure-experimental',
           help: 'emit Closure Compiler-friendly code (experimental)',
           defaultsTo: false)
       ..addFlag('destructure-named-params',
           help: 'Destructure named parameters', defaultsTo: false, hide: true)
       ..addFlag('unsafe-force-compile',
           help: 'Compile code even if it has errors. ಠ_ಠ\n'
@@ skipping 87 matching lines @@
 
     var map = new Map.from(this.sourceMap);
     List list = new List.from(map['sources']);
     map['sources'] = list;
     for (int i = 0; i < list.length; i++) {
       list[i] = path.relative(list[i], from: dir);
     }
     return map;
   }
 }