From 55cc3aa987159bcdb491550d864115c1e8daeebb Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Tue, 2 Feb 2016 15:18:34 -0800
Subject: [PATCH 01/13] WIP.

---
 js/commonjs_export.js                         |  10 +
 js/gulpfile.js                                |  39 ++-
 js/jasmine_commonjs.json                      |  11 +
 js/package.json                               |   6 +-
 .../protobuf/compiler/js/js_generator.cc      | 268 ++++++++++++++----
 .../protobuf/compiler/js/js_generator.h       |  18 +-
 6 files changed, 287 insertions(+), 65 deletions(-)
 create mode 100644 js/commonjs_export.js
 create mode 100644 js/jasmine_commonjs.json

diff --git a/js/commonjs_export.js b/js/commonjs_export.js
new file mode 100644
index 000000000..44e632965
--- /dev/null
+++ b/js/commonjs_export.js
@@ -0,0 +1,10 @@
+/**
+ * @fileoverview Export symbols needed by generated code in CommonJS style.
+ */
+
+exports = {
+  Message: jspb.Message,
+  BinaryReader: jspb.BinaryReader,
+  BinaryWriter: jspb.BinaryWriter,
+  ExtensionFieldInfo: jspb.ExtensionFieldInfo,
+};
diff --git a/js/gulpfile.js b/js/gulpfile.js
index 79095d657..a548a826f 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -1,25 +1,45 @@
 var gulp = require('gulp');
 var exec = require('child_process').exec;
 
-gulp.task('genproto', function (cb) {
+gulp.task('genproto_closure', function (cb) {
   exec('../src/protoc --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
     cb(err);
   });
-})
+});
 
-gulp.task('deps', ['genproto'], function (cb) {
+gulp.task('genproto_commonjs', function (cb) {
+  exec('mkdir -p commonjs_out && ../src/protoc --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('dist', function (cb) {
+  // TODO(haberman): minify this more aggressively.
+  // Will require proper externs/exports.
+  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('deps', ['genproto_closure'], function (cb) {
   exec('./node_modules/google-closure-library/closure/bin/build/depswriter.py *.js binary/*.js > deps.js',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
     cb(err);
   });
-})
+});
 
-gulp.task('test', ['genproto', 'deps'], function (cb) {
+gulp.task('test_closure', ['genproto_closure', 'deps'], function (cb) {
   exec('JASMINE_CONFIG_PATH=jasmine.json ./node_modules/.bin/jasmine',
        function (err, stdout, stderr) {
     console.log(stdout);
@@ -27,3 +47,12 @@ gulp.task('test', ['genproto', 'deps'], function (cb) {
     cb(err);
   });
 });
+
+gulp.task('test_commonjs', ['genproto_commonjs', 'dist'], function (cb) {
+  exec('JASMINE_CONFIG_PATH=jasmine.json cp jasmine_commonjs.json commonjs_out/jasmine.json && cd commonjs_out && ../node_modules/.bin/jasmine',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
diff --git a/js/jasmine_commonjs.json b/js/jasmine_commonjs.json
new file mode 100644
index 000000000..05444550d
--- /dev/null
+++ b/js/jasmine_commonjs.json
@@ -0,0 +1,11 @@
+{
+    "spec_dir": "",
+    "spec_files": [
+        "*_test.js"
+    ],
+    "helpers": [
+        "node_modules/google-closure-library/closure/goog/bootstrap/nodejs.js",
+        "node_loader.js",
+        "deps.js"
+    ]
+}
diff --git a/js/package.json b/js/package.json
index be93286f1..8c8f434b0 100644
--- a/js/package.json
+++ b/js/package.json
@@ -2,13 +2,15 @@
   "name": "google-protobuf",
   "version": "3.0.0-alpha.5",
   "description": "Protocol Buffers for JavaScript",
-  "main": "debug.js",
+  "main": "google-protobuf.js",
   "dependencies": {
     "google-closure-library": "~20160125.0.0",
     "gulp": "~3.9.0",
     "jasmine": "~2.4.1"
   },
-  "devDependencies": {},
+  "devDependencies": {
+    "google-closure-compiler": "~20151216.2.0"
+  },
   "scripts": {
     "test": "./node_modules/gulp/bin/gulp.js test"
   },
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc
index e6c3b36aa..31938b11a 100755
--- a/src/google/protobuf/compiler/js/js_generator.cc
+++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -134,12 +134,37 @@ bool IsReserved(const string& ident) {
 
 // Returns a copy of |filename| with any trailing ".protodevel" or ".proto
 // suffix stripped.
+// TODO(robinson): Unify with copy in compiler/cpp/internal/helpers.cc.
 string StripProto(const string& filename) {
   const char* suffix = HasSuffixString(filename, ".protodevel")
       ? ".protodevel" : ".proto";
   return StripSuffixString(filename, suffix);
 }
 
+// Given a filename like foo/bar/baz.proto, returns the correspoding JavaScript
+// file foo/bar/baz.js.
+string GetJSFilename(const string& filename) {
+  const char* suffix = HasSuffixString(filename, ".protodevel")
+      ? ".protodevel" : ".proto";
+  return StripSuffixString(filename, suffix) + "_pb.js";
+}
+
+// Returns the alias we assign to the module of the given .proto filename
+// when importing.
+string ModuleAlias(const string& filename) {
+  // This scheme could technically cause problems if a file includes any 2 of:
+  //   foo/bar_baz.proto
+  //   foo_bar_baz.proto
+  //   foo_bar/baz.proto
+  //
+  // We'll worry about this problem if/when we actually see it.  This name isn't
+  // exposed to users so we can change it later if we need to.
+  string basename = StripProto(filename);
+  StripString(&basename, "-", '$');
+  StripString(&basename, "/", '_');
+  return basename + "_pb";
+}
+
 // Returns the fully normalized JavaScript path for the given
 // file descriptor's package.
 string GetPath(const GeneratorOptions& options,
@@ -215,6 +240,26 @@ string GetPath(const GeneratorOptions& options,
       value_descriptor->type()) + "." + value_descriptor->name();
 }
 
+string MaybeCrossFileRef(const GeneratorOptions& options,
+                         const FileDescriptor* from_file,
+                         const Descriptor* to_message) {
+  if (options.import_style == GeneratorOptions::IMPORT_COMMONJS &&
+      from_file != to_message->file()) {
+    // Cross-file ref in CommonJS needs to use the module alias instead of
+    // the global name.
+    return ModuleAlias(to_message->file()->name()) + "." + to_message->name();
+  } else {
+    // Within a single file we use a full name.
+    return GetPath(options, to_message);
+  }
+}
+
+string SubmessageTypeRef(const GeneratorOptions& options,
+                         const FieldDescriptor* field) {
+  GOOGLE_CHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE);
+  return MaybeCrossFileRef(options, field->file(), field->message_type());
+}
+
 // - Object field name: LOWER_UNDERSCORE -> LOWER_CAMEL, except for group fields
 // (UPPER_CAMEL -> LOWER_CAMEL), with "List" (or "Map") appended if appropriate,
 // and with reserved words triggering a "pb_" prefix.
@@ -952,11 +997,13 @@ string RelativeTypeName(const FieldDescriptor* field) {
 }
 
 string JSExtensionsObjectName(const GeneratorOptions& options,
+                              const FileDescriptor* from_file,
                               const Descriptor* desc) {
   if (desc->full_name() == "google.protobuf.bridge.MessageSet") {
+    // TODO(haberman): fix this for the IMPORT_COMMONJS case.
     return "jspb.Message.messageSetExtensions";
   } else {
-    return GetPath(options, desc) + ".extensions";
+    return MaybeCrossFileRef(options, from_file, desc) + ".extensions";
   }
 }
 
@@ -1113,19 +1160,24 @@ void Generator::GenerateHeader(const GeneratorOptions& options,
                  "\n");
 }
 
+void Generator::FindProvidesForFile(const GeneratorOptions& options,
+                                    io::Printer* printer,
+                                    const FileDescriptor* file,
+                                    std::set<string>* provided) const {
+  for (int i = 0; i < file->message_type_count(); i++) {
+    FindProvidesForMessage(options, printer, file->message_type(i), provided);
+  }
+  for (int i = 0; i < file->enum_type_count(); i++) {
+    FindProvidesForEnum(options, printer, file->enum_type(i), provided);
+  }
+}
+
 void Generator::FindProvides(const GeneratorOptions& options,
                              io::Printer* printer,
                              const vector<const FileDescriptor*>& files,
                              std::set<string>* provided) const {
   for (int i = 0; i < files.size(); i++) {
-    for (int j = 0; j < files[i]->message_type_count(); j++) {
-      FindProvidesForMessage(options, printer, files[i]->message_type(j),
-                                 provided);
-    }
-    for (int j = 0; j < files[i]->enum_type_count(); j++) {
-      FindProvidesForEnum(options, printer, files[i]->enum_type(j),
-                              provided);
-    }
+    FindProvidesForFile(options, printer, files[i], provided);
   }
 
   printer->Print("\n");
@@ -1204,38 +1256,45 @@ void Generator::GenerateRequires(const GeneratorOptions& options,
                                  io::Printer* printer,
                                  const vector<const FileDescriptor*>& files,
                                  std::set<string>* provided) const {
-  std::set<string> required;
-  std::set<string> forwards;
-  bool have_extensions = false;
-  bool have_message = false;
+  if (options.import_style == GeneratorOptions::IMPORT_BROWSER) {
+    return;
+  } else if (options.import_style == GeneratorOptions::IMPORT_CLOSURE) {
+    // For Closure imports we need to import every message type individually.
+    std::set<string> required;
+    std::set<string> forwards;
+    bool have_extensions = false;
+    bool have_message = false;
 
-  for (int i = 0; i < files.size(); i++) {
-    for (int j = 0; j < files[i]->message_type_count(); j++) {
-      FindRequiresForMessage(options,
-                             files[i]->message_type(j),
-                             &required, &forwards, &have_message);
-    }
-    if (!have_extensions && HasExtensions(files[i])) {
-      have_extensions = true;
+    for (int i = 0; i < files.size(); i++) {
+      for (int j = 0; j < files[i]->message_type_count(); j++) {
+        FindRequiresForMessage(options,
+                               files[i]->message_type(j),
+                               &required, &forwards, &have_message);
+      }
+      if (!have_extensions && HasExtensions(files[i])) {
+        have_extensions = true;
+      }
+
+      for (int j = 0; j < files[i]->extension_count(); j++) {
+        const FieldDescriptor* extension = files[i]->extension(j);
+        if (IgnoreField(extension)) {
+          continue;
+        }
+        if (extension->containing_type()->full_name() !=
+            "google.protobuf.bridge.MessageSet") {
+          required.insert(GetPath(options, extension->containing_type()));
+        }
+        FindRequiresForField(options, extension, &required, &forwards);
+        have_extensions = true;
+      }
     }
 
-    for (int j = 0; j < files[i]->extension_count(); j++) {
-      const FieldDescriptor* extension = files[i]->extension(j);
-      if (IgnoreField(extension)) {
-        continue;
-      }
-      if (extension->containing_type()->full_name() !=
-          "google.protobuf.bridge.MessageSet") {
-        required.insert(GetPath(options, extension->containing_type()));
-      }
-      FindRequiresForField(options, extension, &required, &forwards);
-      have_extensions = true;
-    }
+    GenerateRequiresImpl(options, printer, &required, &forwards, provided,
+                         /* require_jspb = */ have_message,
+                         /* require_extension = */ have_extensions);
+  } else if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+    // CommonJS imports are based on files
   }
-
-  GenerateRequiresImpl(options, printer, &required, &forwards, provided,
-                       /* require_jspb = */ have_message,
-                       /* require_extension = */ have_extensions);
 }
 
 void Generator::GenerateRequires(const GeneratorOptions& options,
@@ -1406,6 +1465,19 @@ void Generator::GenerateClass(const GeneratorOptions& options,
     if (IsExtendable(desc) && desc->full_name() != "google.protobuf.bridge.MessageSet") {
       GenerateClassExtensionFieldInfo(options, printer, desc);
     }
+
+    if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+      printer->Print(
+          "exports.$name$ = $fullName$;\n",
+          "name", desc->name(),
+          "fullName", GetPath(options, desc));
+    }
+
+    if (options.import_style != GeneratorOptions:: IMPORT_CLOSURE) {
+      for (int i = 0; i < desc->extension_count(); i++) {
+        GenerateExtension(options, printer, desc->extension(i));
+      }
+    }
   }
 
   // Recurse on nested types.
@@ -1623,7 +1695,7 @@ void Generator::GenerateClassToObject(const GeneratorOptions& options,
         "obj,\n"
         "      $extObject$, $class$.prototype.getExtension,\n"
         "      includeInstance);\n",
-        "extObject", JSExtensionsObjectName(options, desc),
+        "extObject", JSExtensionsObjectName(options, desc->file(), desc),
         "class", GetPath(options, desc));
   }
 
@@ -1652,13 +1724,13 @@ void Generator::GenerateClassFieldToObject(const GeneratorOptions& options,
         printer->Print("jspb.Message.toObjectList(msg.get$getter$(),\n"
                        "    $type$.toObject, includeInstance)",
                        "getter", JSGetterName(field),
-                       "type", GetPath(options, field->message_type()));
+                       "type", SubmessageTypeRef(options, field));
       }
     } else {
       printer->Print("(f = msg.get$getter$()) && "
                      "$type$.toObject(includeInstance, f)",
                      "getter", JSGetterName(field),
-                     "type", GetPath(options, field->message_type()));
+                     "type", SubmessageTypeRef(options, field));
     }
   } else {
     // Simple field (singular or repeated).
@@ -1723,7 +1795,7 @@ void Generator::GenerateClassFieldFromObject(
             "      }));\n",
             "name", JSObjectFieldName(field),
             "index", JSFieldIndex(field),
-            "fieldclass", GetPath(options, field->message_type()));
+            "fieldclass", SubmessageTypeRef(options, field));
       }
     } else {
       printer->Print(
@@ -1731,7 +1803,7 @@ void Generator::GenerateClassFieldFromObject(
           "      msg, $index$, $fieldclass$.fromObject(obj.$name$));\n",
           "name", JSObjectFieldName(field),
           "index", JSFieldIndex(field),
-          "fieldclass", GetPath(options, field->message_type()));
+          "fieldclass", SubmessageTypeRef(options, field));
     }
   } else {
     // Simple (primitive) field.
@@ -1815,7 +1887,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
                                       /* always_singular = */ false),
         "rpt", (field->is_repeated() ? "Repeated" : ""),
         "index", JSFieldIndex(field),
-        "wrapperclass", GetPath(options, field->message_type()),
+        "wrapperclass", SubmessageTypeRef(options, field),
         "required", (field->label() == FieldDescriptor::LABEL_REQUIRED ?
                      ", 1" : ""));
     printer->Print(
@@ -2043,7 +2115,7 @@ void Generator::GenerateClassDeserializeBinary(const GeneratorOptions& options,
         "        $class$.prototype.getExtension,\n"
         "        $class$.prototype.setExtension);\n"
         "      break;\n",
-        "extobj", JSExtensionsObjectName(options, desc),
+        "extobj", JSExtensionsObjectName(options, desc->file(), desc),
         "class", GetPath(options, desc));
   } else {
     printer->Print(
@@ -2073,7 +2145,7 @@ void Generator::GenerateClassDeserializeBinaryField(
         "      var value = new $fieldclass$;\n"
         "      reader.read$msgOrGroup$($grpfield$value,"
         "$fieldclass$.deserializeBinaryFromReader);\n",
-        "fieldclass", GetPath(options, field->message_type()),
+        "fieldclass", SubmessageTypeRef(options, field),
         "msgOrGroup", (field->type() == FieldDescriptor::TYPE_GROUP) ?
                       "Group" : "Message",
         "grpfield", (field->type() == FieldDescriptor::TYPE_GROUP) ?
@@ -2149,7 +2221,7 @@ void Generator::GenerateClassSerializeBinary(const GeneratorOptions& options,
     printer->Print(
         "  jspb.Message.serializeBinaryExtensions(this, writer, $extobj$,\n"
         "    $class$.prototype.getExtension);\n",
-        "extobj", JSExtensionsObjectName(options, desc),
+        "extobj", JSExtensionsObjectName(options, desc->file(), desc),
         "class", GetPath(options, desc));
   }
 
@@ -2222,7 +2294,7 @@ void Generator::GenerateClassSerializeBinaryField(
     printer->Print(
         ",\n"
         "      $submsg$.serializeBinaryToWriter\n",
-        "submsg", GetPath(options, field->message_type()));
+        "submsg", SubmessageTypeRef(options, field));
   } else {
     printer->Print("\n");
   }
@@ -2290,9 +2362,9 @@ void Generator::GenerateExtension(const GeneratorOptions& options,
       "index", SimpleItoa(field->number()),
       "name", JSObjectFieldName(field),
       "ctor", (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ?
-               GetPath(options, field->message_type()) : string("null")),
+               SubmessageTypeRef(options, field) : string("null")),
       "toObject", (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ?
-                   (GetPath(options, field->message_type()) + ".toObject") :
+                   (SubmessageTypeRef(options, field) + ".toObject") :
                    string("null")),
       "repeated", (field->is_repeated() ? "1" : "0"));
 
@@ -2308,11 +2380,11 @@ void Generator::GenerateExtension(const GeneratorOptions& options,
         "binaryWriterFn", JSBinaryWriterMethodName(field),
         "binaryMessageSerializeFn",
         (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) ?
-        (GetPath(options, field->message_type()) +
+        (SubmessageTypeRef(options, field) +
          ".serializeBinaryToWriter") : "null",
         "binaryMessageDeserializeFn",
         (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) ?
-        (GetPath(options, field->message_type()) +
+        (SubmessageTypeRef(options, field) +
          ".deserializeBinaryFromReader") : "null",
         "isPacked", (field->is_packed() ? "true" : "false"));
   } else {
@@ -2324,7 +2396,8 @@ void Generator::GenerateExtension(const GeneratorOptions& options,
       "// toObject() will function correctly.\n"
       "$extendName$[$index$] = $class$.$name$;\n"
       "\n",
-      "extendName", JSExtensionsObjectName(options, field->containing_type()),
+      "extendName", JSExtensionsObjectName(options, field->file(),
+                                           field->containing_type()),
       "index", SimpleItoa(field->number()),
       "class", extension_scope,
       "name", JSObjectFieldName(field));
@@ -2364,6 +2437,19 @@ bool GeneratorOptions::ParseFromOptions(
       namespace_prefix = options[i].second;
     } else if (options[i].first == "library") {
       library = options[i].second;
+    } else if (options[i].first == "import_style") {
+      if (options[i].second == "closure") {
+        import_style = IMPORT_CLOSURE;
+      } else if (options[i].second == "commonjs") {
+        import_style = IMPORT_COMMONJS;
+      } else if (options[i].second == "browser") {
+        import_style = IMPORT_BROWSER;
+      } else if (options[i].second == "es6") {
+        import_style = IMPORT_ES6;
+      } else {
+        *error = "Unknown import style " + options[i].second + ", expected " +
+                 "one of: closure, commonjs, browser, es6.";
+      }
     } else {
       // Assume any other option is an output directory, as long as it is a bare
       // `key` rather than a `key=value` option.
@@ -2375,6 +2461,11 @@ bool GeneratorOptions::ParseFromOptions(
     }
   }
 
+  if (!library.empty() && import_style != IMPORT_CLOSURE) {
+    *error = "The library option should only be used for "
+             "import_style=closure";
+  }
+
   return true;
 }
 
@@ -2418,6 +2509,47 @@ void Generator::GenerateFileAndDeps(
   }
 }
 
+void Generator::GenerateFile(const GeneratorOptions& options,
+                             io::Printer* printer,
+                             const FileDescriptor* file) const {
+  GenerateHeader(options, printer);
+
+  // Generate "require" statements.
+  if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+    for (int i = 0; i < file->dependency_count(); i++) {
+      const std::string& name = file->dependency(i)->name();
+      printer->Print(
+          "var $alias$ = require('$file$');\n",
+          "alias", ModuleAlias(name),
+          "file", GetJSFilename(name));
+    }
+  }
+
+  // We aren't using Closure's import system, but we use goog.exportSymbol()
+  // to construct the expected tree of objects, eg.
+  //
+  //   goog.exportSymbol('foo.bar.Baz', null, this);
+  //
+  //   // Later generated code expects foo.bar = {} to exist:
+  //   foo.bar.Baz = function() { /* ... */ }
+  std::set<std::string> provided;
+  FindProvidesForFile(options, printer, file, &provided);
+  //FindProvidesForFields(options, printer, extensions, &provided);
+  for (std::set<string>::iterator it = provided.begin();
+       it != provided.end(); ++it) {
+    printer->Print("goog.exportSymbol('$name$', null, this);\n",
+                   "name", *it);
+  }
+
+  GenerateClassesAndEnums(options, printer, file);
+
+  // Extensions nested inside messages are emitted inside
+  // GenerateClassesAndEnums().
+  for (int i = 0; i < file->extension_count(); i++) {
+    GenerateExtension(options, printer, file->extension(i));
+  }
+}
+
 bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,
                             const string& parameter,
                             GeneratorContext* context,
@@ -2430,10 +2562,14 @@ bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,
   }
 
 
-  // We're either generating a single library file with definitions for message
-  // and enum types in *all* FileDescriptor inputs, or we're generating a single
-  // file for each type.
-  if (options.library != "") {
+  // There are three schemes for where output files go:
+  //
+  // - import_style = IMPORT_CLOSURE, library non-empty: all output in one file
+  // - import_style = IMPORT_CLOSURE, library empty: one output file per type
+  // - import_style != IMPORT_CLOSURE: one output file per .proto file
+  if (options.import_style == GeneratorOptions::IMPORT_CLOSURE &&
+      options.library != "") {
+    // All output should go in a single file.
     string filename = options.output_dir + "/" + options.library + ".js";
     google::protobuf::scoped_ptr<io::ZeroCopyOutputStream> output(context->Open(filename));
     GOOGLE_CHECK(output.get());
@@ -2469,7 +2605,7 @@ bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,
     if (printer.failed()) {
       return false;
     }
-  } else {
+  } else if (options.import_style == GeneratorOptions::IMPORT_CLOSURE) {
     // Collect all types, and print each type to a separate file. Pull out
     // free-floating extensions while we make this pass.
     map< string, vector<const FieldDescriptor*> > extensions_by_namespace;
@@ -2611,6 +2747,24 @@ bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,
         }
       }
     }
+  } else {
+    // Generate one output file per input (.proto) file.
+
+    for (int i = 0; i < files.size(); i++) {
+      const google::protobuf::FileDescriptor* file = files[i];
+
+      string filename = options.output_dir + "/" + GetJSFilename(file->name());
+      google::protobuf::scoped_ptr<io::ZeroCopyOutputStream> output(
+          context->Open(filename));
+      GOOGLE_CHECK(output.get());
+      io::Printer printer(output.get(), '$');
+
+      GenerateFile(options, &printer, file);
+
+      if (printer.failed()) {
+        return false;
+      }
+    }
   }
 
   return true;
diff --git a/src/google/protobuf/compiler/js/js_generator.h b/src/google/protobuf/compiler/js/js_generator.h
index db2dceb34..db9178d34 100755
--- a/src/google/protobuf/compiler/js/js_generator.h
+++ b/src/google/protobuf/compiler/js/js_generator.h
@@ -67,6 +67,13 @@ struct GeneratorOptions {
   bool error_on_name_conflict;
   // Enable binary-format support?
   bool binary;
+  // What style of imports should be used.
+  enum ImportStyle {
+    IMPORT_CLOSURE,    // goog.require()
+    IMPORT_COMMONJS,   // require()
+    IMPORT_BROWSER,    // no import statements
+    IMPORT_ES6,        // import { member } from ''
+  } import_style;
 
   GeneratorOptions()
       : add_require_for_enums(false),
@@ -75,7 +82,8 @@ struct GeneratorOptions {
         namespace_prefix(""),
         library(""),
         error_on_name_conflict(false),
-        binary(false) {}
+        binary(false),
+        import_style(IMPORT_CLOSURE) {}
 
   bool ParseFromOptions(
       const vector< pair< string, string > >& options,
@@ -111,6 +119,10 @@ class LIBPROTOC_EXPORT Generator : public CodeGenerator {
                     io::Printer* printer,
                     const vector<const FileDescriptor*>& file,
                     std::set<string>* provided) const;
+  void FindProvidesForFile(const GeneratorOptions& options,
+                           io::Printer* printer,
+                           const FileDescriptor* file,
+                           std::set<string>* provided) const;
   void FindProvidesForMessage(const GeneratorOptions& options,
                               io::Printer* printer,
                               const Descriptor* desc,
@@ -168,6 +180,10 @@ class LIBPROTOC_EXPORT Generator : public CodeGenerator {
                                 std::set<string>* required,
                                 std::set<string>* forwards) const;
 
+  void GenerateFile(const GeneratorOptions& options,
+                    io::Printer* printer,
+                    const FileDescriptor* file) const;
+
   // Generate definitions for all message classes and enums in all files,
   // processing the files in dependence order.
   void GenerateFilesInDepOrder(const GeneratorOptions& options,

From e9f31ee3d7cf7c0f370607e54dbea01ba7240a77 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 4 Feb 2016 10:29:27 -0800
Subject: [PATCH 02/13] CommonJS tests are now passing.

---
 js/binary/proto_test.js                       |  2 +
 js/binary/reader_test.js                      |  2 +
 js/binary/utils_test.js                       |  2 +
 js/commonjs_export.js                         | 13 +++---
 js/commonjs_export_asserts.js                 | 27 +++++++++++
 js/debug_test.js                              |  5 ++-
 js/gulpfile.js                                | 38 ++++++++++++++--
 js/jasmine_commonjs.json                      |  6 +--
 js/message_test.js                            | 17 +++++--
 js/package.json                               |  5 ++-
 js/proto3_test.js                             |  4 ++
 js/rewrite_tests_for_commonjs.js              | 45 +++++++++++++++++++
 .../protobuf/compiler/js/js_generator.cc      |  6 ++-
 13 files changed, 152 insertions(+), 20 deletions(-)
 create mode 100644 js/commonjs_export_asserts.js
 create mode 100644 js/rewrite_tests_for_commonjs.js

diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js
index 1cb7ff0e9..106ee243d 100644
--- a/js/binary/proto_test.js
+++ b/js/binary/proto_test.js
@@ -31,6 +31,8 @@
 // Test suite is written using Jasmine -- see http://jasmine.github.io/
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: testbinary_pb
 goog.require('proto.jspb.test.ExtendsWithMessage');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignMessage');
diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js
index a6482610e..c0f127021 100644
--- a/js/binary/reader_test.js
+++ b/js/binary/reader_test.js
@@ -42,6 +42,8 @@
  */
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google_protobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryDecoder');
 goog.require('jspb.BinaryReader');
diff --git a/js/binary/utils_test.js b/js/binary/utils_test.js
index 5c330791a..37bab080c 100644
--- a/js/binary/utils_test.js
+++ b/js/binary/utils_test.js
@@ -38,6 +38,8 @@
 
 goog.require('goog.crypt.base64');
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google_protobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryWriter');
 goog.require('jspb.utils');
diff --git a/js/commonjs_export.js b/js/commonjs_export.js
index 44e632965..ffbeb9dba 100644
--- a/js/commonjs_export.js
+++ b/js/commonjs_export.js
@@ -2,9 +2,10 @@
  * @fileoverview Export symbols needed by generated code in CommonJS style.
  */
 
-exports = {
-  Message: jspb.Message,
-  BinaryReader: jspb.BinaryReader,
-  BinaryWriter: jspb.BinaryWriter,
-  ExtensionFieldInfo: jspb.ExtensionFieldInfo,
-};
+exports.Message = jspb.Message;
+exports.BinaryReader = jspb.BinaryReader;
+exports.BinaryWriter = jspb.BinaryWriter;
+exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo;
+
+exports.exportSymbol = goog.exportSymbol;
+exports.inherits = goog.inherits;
diff --git a/js/commonjs_export_asserts.js b/js/commonjs_export_asserts.js
new file mode 100644
index 000000000..16abdd7de
--- /dev/null
+++ b/js/commonjs_export_asserts.js
@@ -0,0 +1,27 @@
+/**
+ * @fileoverview Description of this file.
+ */
+
+goog.require('goog.testing.asserts');
+
+var global = Function('return this')();
+
+// The Google Closure assert functions start with assert, eg.
+//   assertThrows
+//   assertNotThrows
+//   assertTrue
+//   ...
+//
+// The one exception is the "fail" function.
+function shouldExport(str) {
+  return str.lastIndexOf('assert') === 0 || str == 'fail';
+}
+
+for (var key in global) {
+  if ((typeof key == "string") && global.hasOwnProperty(key) &&
+      shouldExport(key)) {
+    exports[key] = global[key];
+  }
+}
+
+exports.COMPILED = COMPILED
diff --git a/js/debug_test.js b/js/debug_test.js
index 615fc7c68..d7bf3768f 100644
--- a/js/debug_test.js
+++ b/js/debug_test.js
@@ -31,13 +31,16 @@
 goog.setTestOnly();
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google-protobuf
 goog.require('jspb.debug');
+
+// CommonJS-LoadFromFile: test_pb
 goog.require('proto.jspb.test.HasExtensions');
 goog.require('proto.jspb.test.IsExtension');
 goog.require('proto.jspb.test.Simple1');
 
 
-
 describe('debugTest', function() {
   it('testSimple1', function() {
     if (COMPILED) {
diff --git a/js/gulpfile.js b/js/gulpfile.js
index a548a826f..22f8f3fd5 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -1,5 +1,6 @@
 var gulp = require('gulp');
 var exec = require('child_process').exec;
+var glob = require('glob');
 
 gulp.task('genproto_closure', function (cb) {
   exec('../src/protoc --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
@@ -22,7 +23,38 @@ gulp.task('genproto_commonjs', function (cb) {
 gulp.task('dist', function (cb) {
   // TODO(haberman): minify this more aggressively.
   // Will require proper externs/exports.
-  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -i commonjs_export.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('commonjs_asserts', function (cb) {
+  exec('mkdir -p commonjs_out && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs_export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/closure_asserts_commonjs.js',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts'], function (cb) {
+  // TODO(haberman): minify this more aggressively.
+  // Will require proper externs/exports.
+  var cmd = "mkdir -p commonjs_out/binary && ";
+  function addTestFile(file) {
+    cmd += 'nodejs rewrite_tests_for_commonjs.js < ' + file +
+           ' > commonjs_out/' + file + '&& ';
+  }
+
+  glob.sync('*_test.js').forEach(addTestFile);
+  glob.sync('binary/*_test.js').forEach(addTestFile);
+
+  exec(cmd +
+       'cp jasmine_commonjs.json commonjs_out/jasmine.json && ' +
+       'cp google-protobuf.js commonjs_out',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -48,8 +80,8 @@ gulp.task('test_closure', ['genproto_closure', 'deps'], function (cb) {
   });
 });
 
-gulp.task('test_commonjs', ['genproto_commonjs', 'dist'], function (cb) {
-  exec('JASMINE_CONFIG_PATH=jasmine.json cp jasmine_commonjs.json commonjs_out/jasmine.json && cd commonjs_out && ../node_modules/.bin/jasmine',
+gulp.task('test_commonjs', ['make_commonjs_out'], function (cb) {
+  exec('cd commonjs_out && JASMINE_CONFIG_PATH=jasmine.json NODE_PATH=. ../node_modules/.bin/jasmine',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
diff --git a/js/jasmine_commonjs.json b/js/jasmine_commonjs.json
index 05444550d..666b8edbf 100644
--- a/js/jasmine_commonjs.json
+++ b/js/jasmine_commonjs.json
@@ -1,11 +1,9 @@
 {
     "spec_dir": "",
     "spec_files": [
-        "*_test.js"
+        "*_test.js",
+        "binary/proto_test.js"
     ],
     "helpers": [
-        "node_modules/google-closure-library/closure/goog/bootstrap/nodejs.js",
-        "node_loader.js",
-        "deps.js"
     ]
 }
diff --git a/js/message_test.js b/js/message_test.js
index 971ea4f42..63da8532f 100644
--- a/js/message_test.js
+++ b/js/message_test.js
@@ -34,17 +34,25 @@ goog.setTestOnly();
 
 goog.require('goog.json');
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google-protobuf
 goog.require('jspb.Message');
+
+// CommonJS-LoadFromFile: test5_pb
 goog.require('proto.jspb.exttest.beta.floatingStrField');
+
+// CommonJS-LoadFromFile: test3_pb
 goog.require('proto.jspb.exttest.floatingMsgField');
+
+// CommonJS-LoadFromFile: test4_pb
 goog.require('proto.jspb.exttest.floatingMsgFieldTwo');
+
+// CommonJS-LoadFromFile: test_pb
 goog.require('proto.jspb.test.CloneExtension');
 goog.require('proto.jspb.test.Complex');
 goog.require('proto.jspb.test.DefaultValues');
 goog.require('proto.jspb.test.Empty');
 goog.require('proto.jspb.test.EnumContainer');
-goog.require('proto.jspb.test.ExtensionMessage');
-goog.require('proto.jspb.test.floatingMsgField');
 goog.require('proto.jspb.test.floatingStrField');
 goog.require('proto.jspb.test.HasExtensions');
 goog.require('proto.jspb.test.IndirectExtension');
@@ -56,13 +64,16 @@ goog.require('proto.jspb.test.Simple1');
 goog.require('proto.jspb.test.Simple2');
 goog.require('proto.jspb.test.SpecialCases');
 goog.require('proto.jspb.test.TestClone');
-goog.require('proto.jspb.test.TestExtensionsMessage');
 goog.require('proto.jspb.test.TestGroup');
 goog.require('proto.jspb.test.TestGroup1');
 goog.require('proto.jspb.test.TestMessageWithOneof');
 goog.require('proto.jspb.test.TestReservedNames');
 goog.require('proto.jspb.test.TestReservedNamesExtension');
 
+// CommonJS-LoadFromFile: test2_pb
+goog.require('proto.jspb.test.ExtensionMessage');
+goog.require('proto.jspb.test.TestExtensionsMessage');
+goog.require('proto.jspb.test.floatingMsgField');
 
 
 
diff --git a/js/package.json b/js/package.json
index 8c8f434b0..d37da1ddc 100644
--- a/js/package.json
+++ b/js/package.json
@@ -9,10 +9,11 @@
     "jasmine": "~2.4.1"
   },
   "devDependencies": {
-    "google-closure-compiler": "~20151216.2.0"
+    "google-closure-compiler": "~20151216.2.0",
+    "glob": "~6.0.4"
   },
   "scripts": {
-    "test": "./node_modules/gulp/bin/gulp.js test"
+    "test": "./node_modules/gulp/bin/gulp.js test_closure"
   },
   "repository": {
     "type": "git",
diff --git a/js/proto3_test.js b/js/proto3_test.js
index 8102bab6d..e4116cb9c 100644
--- a/js/proto3_test.js
+++ b/js/proto3_test.js
@@ -29,7 +29,11 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: testbinary_pb
 goog.require('proto.jspb.test.ForeignMessage');
+
+// CommonJS-LoadFromFile: proto3_test_pb
 goog.require('proto.jspb.test.Proto3Enum');
 goog.require('proto.jspb.test.TestProto3');
 
diff --git a/js/rewrite_tests_for_commonjs.js b/js/rewrite_tests_for_commonjs.js
new file mode 100644
index 000000000..4dc0a22d0
--- /dev/null
+++ b/js/rewrite_tests_for_commonjs.js
@@ -0,0 +1,45 @@
+/**
+ * @fileoverview Description of this file.
+ */
+
+var lineReader = require('readline').createInterface({
+  input: process.stdin,
+  output: process.stdout
+});
+
+var module = null;
+lineReader.on('line', function(line) {
+  var is_require = line.match(/goog\.require\('([^']*\.)([^'.]+)'\)/);
+  var is_loadfromfile = line.match(/CommonJS-LoadFromFile: (.*)/);
+  var is_settestonly = line.match(/goog.setTestOnly()/);
+  if (is_settestonly) {
+    // Remove this line.
+  } else if (is_require) {
+    if (module) {  // Skip goog.require() lines before the first directive.
+      var pkg = is_require[1];
+      var sym = is_require[2];
+      console.log("google_protobuf.exportSymbol('" + pkg + sym + "', " + module + "." + sym + ', global);');
+    }
+  } else if (is_loadfromfile) {
+    if (!module) {
+      console.log("var asserts = require('closure_asserts_commonjs');");
+      console.log("var global = Function('return this')();");
+      console.log("");
+      console.log("// Bring asserts into the global namespace.");
+      console.log("for (var key in asserts) {");
+      console.log("  if (asserts.hasOwnProperty(key)) {");
+      console.log("    global[key] = asserts[key];");
+      console.log("  }");
+      console.log("}");
+      console.log("");
+      console.log("var google_protobuf = require('google-protobuf');");
+    }
+    module = is_loadfromfile[1].replace("-", "_");
+
+    if (module != "google_protobuf") {  // We unconditionally require this in the header.
+      console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
+    }
+  } else {
+    console.log(line);
+  }
+});
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc
index 31938b11a..2dea60fa8 100755
--- a/src/google/protobuf/compiler/js/js_generator.cc
+++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -2516,6 +2516,10 @@ void Generator::GenerateFile(const GeneratorOptions& options,
 
   // Generate "require" statements.
   if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+    printer->Print("var jspb = require('google-protobuf');\n");
+    printer->Print("var goog = jspb;\n");
+    printer->Print("var global = Function('return this')();\n\n");
+
     for (int i = 0; i < file->dependency_count(); i++) {
       const std::string& name = file->dependency(i)->name();
       printer->Print(
@@ -2537,7 +2541,7 @@ void Generator::GenerateFile(const GeneratorOptions& options,
   //FindProvidesForFields(options, printer, extensions, &provided);
   for (std::set<string>::iterator it = provided.begin();
        it != provided.end(); ++it) {
-    printer->Print("goog.exportSymbol('$name$', null, this);\n",
+    printer->Print("goog.exportSymbol('$name$', null, global);\n",
                    "name", *it);
   }
 

From 9e60036c1b1db947d29cbaaa668aeae19e7d2068 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 4 Feb 2016 10:38:06 -0800
Subject: [PATCH 03/13] Moved CommonJS-specific files to commonjs/.

---
 js/{commonjs_export.js => commonjs/export.js}             | 0
 .../export_asserts.js}                                    | 0
 js/{jasmine_commonjs.json => commonjs/jasmine.json}       | 0
 js/{ => commonjs}/rewrite_tests_for_commonjs.js           | 0
 js/gulpfile.js                                            | 8 ++++----
 5 files changed, 4 insertions(+), 4 deletions(-)
 rename js/{commonjs_export.js => commonjs/export.js} (100%)
 rename js/{commonjs_export_asserts.js => commonjs/export_asserts.js} (100%)
 rename js/{jasmine_commonjs.json => commonjs/jasmine.json} (100%)
 rename js/{ => commonjs}/rewrite_tests_for_commonjs.js (100%)

diff --git a/js/commonjs_export.js b/js/commonjs/export.js
similarity index 100%
rename from js/commonjs_export.js
rename to js/commonjs/export.js
diff --git a/js/commonjs_export_asserts.js b/js/commonjs/export_asserts.js
similarity index 100%
rename from js/commonjs_export_asserts.js
rename to js/commonjs/export_asserts.js
diff --git a/js/jasmine_commonjs.json b/js/commonjs/jasmine.json
similarity index 100%
rename from js/jasmine_commonjs.json
rename to js/commonjs/jasmine.json
diff --git a/js/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
similarity index 100%
rename from js/rewrite_tests_for_commonjs.js
rename to js/commonjs/rewrite_tests_for_commonjs.js
diff --git a/js/gulpfile.js b/js/gulpfile.js
index 22f8f3fd5..bdc4212b2 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -23,7 +23,7 @@ gulp.task('genproto_commonjs', function (cb) {
 gulp.task('dist', function (cb) {
   // TODO(haberman): minify this more aggressively.
   // Will require proper externs/exports.
-  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -i commonjs_export.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -i commonjs/export.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -32,7 +32,7 @@ gulp.task('dist', function (cb) {
 });
 
 gulp.task('commonjs_asserts', function (cb) {
-  exec('mkdir -p commonjs_out && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs_export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/closure_asserts_commonjs.js',
+  exec('mkdir -p commonjs_out && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs/export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/closure_asserts_commonjs.js',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -45,7 +45,7 @@ gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts']
   // Will require proper externs/exports.
   var cmd = "mkdir -p commonjs_out/binary && ";
   function addTestFile(file) {
-    cmd += 'nodejs rewrite_tests_for_commonjs.js < ' + file +
+    cmd += 'nodejs commonjs/rewrite_tests_for_commonjs.js < ' + file +
            ' > commonjs_out/' + file + '&& ';
   }
 
@@ -53,7 +53,7 @@ gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts']
   glob.sync('binary/*_test.js').forEach(addTestFile);
 
   exec(cmd +
-       'cp jasmine_commonjs.json commonjs_out/jasmine.json && ' +
+       'cp commonjs/jasmine.json commonjs_out/jasmine.json && ' +
        'cp google-protobuf.js commonjs_out',
        function (err, stdout, stderr) {
     console.log(stdout);

From d6a186a8f18c1b14979e0244c0434b5b89c0f8a9 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 4 Feb 2016 14:58:45 -0800
Subject: [PATCH 04/13] Added some documentation in comments.

---
 js/commonjs/export.js                     |  3 +++
 js/commonjs/export_asserts.js             |  8 +++++++-
 js/commonjs/rewrite_tests_for_commonjs.js | 22 +++++++++++++++++++++-
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/js/commonjs/export.js b/js/commonjs/export.js
index ffbeb9dba..89ded3303 100644
--- a/js/commonjs/export.js
+++ b/js/commonjs/export.js
@@ -1,5 +1,8 @@
 /**
  * @fileoverview Export symbols needed by generated code in CommonJS style.
+ *
+ * This effectively is our canonical list of what we publicly export from
+ * the google-protobuf.js file that we build at distribution time.
  */
 
 exports.Message = jspb.Message;
diff --git a/js/commonjs/export_asserts.js b/js/commonjs/export_asserts.js
index 16abdd7de..9b823d3c5 100644
--- a/js/commonjs/export_asserts.js
+++ b/js/commonjs/export_asserts.js
@@ -1,11 +1,17 @@
 /**
- * @fileoverview Description of this file.
+ * @fileoverview Exports symbols needed only by tests.
+ *
+ * This file exports several Closure Library symbols that are only
+ * used by tests.  It is used to generate a file
+ * closure_asserts_commonjs.js that is only used at testing time.
  */
 
 goog.require('goog.testing.asserts');
 
 var global = Function('return this')();
 
+// All of the closure "assert" functions are exported at the global level.
+//
 // The Google Closure assert functions start with assert, eg.
 //   assertThrows
 //   assertNotThrows
diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
index 4dc0a22d0..d49f8a93e 100644
--- a/js/commonjs/rewrite_tests_for_commonjs.js
+++ b/js/commonjs/rewrite_tests_for_commonjs.js
@@ -1,5 +1,25 @@
 /**
- * @fileoverview Description of this file.
+ * @fileoverview Utility to translate test files to CommonJS imports.
+ *
+ * This is a somewhat hacky tool designed to do one very specific thing.
+ * All of the test files in *_test.js are written with Closure-style
+ * imports (goog.require()).  This works great for running the tests
+ * against Closure-style generated code, but we also want to run the
+ * tests against CommonJS-style generated code without having to fork
+ * the tests.
+ *
+ * Closure-style imports import each individual type by name.  This is
+ * very different than CommonJS imports which are by file.  So we put
+ * special comments in these tests like:
+ *
+ * // CommonJS-LoadFromFile: test_pb
+ * goog.require('proto.jspb.test.CloneExtension');
+ * goog.require('proto.jspb.test.Complex');
+ * goog.require('proto.jspb.test.DefaultValues');
+ *
+ * This script parses that special comment and uses it to generate proper
+ * CommonJS require() statements so that the tests can run and pass using
+ * CommonJS imports.
  */
 
 var lineReader = require('readline').createInterface({

From 77af5d04b1897baeda2ebd753d138c99afe72c50 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 4 Feb 2016 16:11:07 -0800
Subject: [PATCH 05/13] Fixed nested message scopes for CommonJS.

---
 js/binary/proto_test.js                       |  2 +-
 js/commonjs/export.js                         |  2 ++
 js/commonjs/rewrite_tests_for_commonjs.js     | 28 +++++++++++--------
 js/message_test.js                            | 19 +++++++++----
 js/proto3_test.js                             |  4 +--
 js/test.proto                                 |  7 +++++
 .../protobuf/compiler/js/js_generator.cc      | 12 ++++----
 7 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js
index 106ee243d..817f8a79f 100644
--- a/js/binary/proto_test.js
+++ b/js/binary/proto_test.js
@@ -32,7 +32,7 @@
 
 goog.require('goog.testing.asserts');
 
-// CommonJS-LoadFromFile: testbinary_pb
+// CommonJS-LoadFromFile: testbinary_pb proto.jspb.test
 goog.require('proto.jspb.test.ExtendsWithMessage');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignMessage');
diff --git a/js/commonjs/export.js b/js/commonjs/export.js
index 89ded3303..2fc2fcbd5 100644
--- a/js/commonjs/export.js
+++ b/js/commonjs/export.js
@@ -10,5 +10,7 @@ exports.BinaryReader = jspb.BinaryReader;
 exports.BinaryWriter = jspb.BinaryWriter;
 exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo;
 
+// These are used by generated code but should not be used directly by clients.
 exports.exportSymbol = goog.exportSymbol;
 exports.inherits = goog.inherits;
+exports.object = {extend: goog.object.extend};
diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
index d49f8a93e..6a655c1e5 100644
--- a/js/commonjs/rewrite_tests_for_commonjs.js
+++ b/js/commonjs/rewrite_tests_for_commonjs.js
@@ -27,34 +27,38 @@ var lineReader = require('readline').createInterface({
   output: process.stdout
 });
 
+function tryStripPrefix(str, prefix) {
+  if (str.lastIndexOf(prefix) !== 0) {
+    throw "String: " + str + " didn't start with: " + prefix;
+  }
+  return str.substr(prefix.length);
+}
+
 var module = null;
+var pkg = null;
 lineReader.on('line', function(line) {
-  var is_require = line.match(/goog\.require\('([^']*\.)([^'.]+)'\)/);
-  var is_loadfromfile = line.match(/CommonJS-LoadFromFile: (.*)/);
+  var is_require = line.match(/goog\.require\('([^']*)'\)/);
+  var is_loadfromfile = line.match(/CommonJS-LoadFromFile: ([^ ]*) (.*)/);
   var is_settestonly = line.match(/goog.setTestOnly()/);
   if (is_settestonly) {
     // Remove this line.
   } else if (is_require) {
     if (module) {  // Skip goog.require() lines before the first directive.
-      var pkg = is_require[1];
-      var sym = is_require[2];
-      console.log("google_protobuf.exportSymbol('" + pkg + sym + "', " + module + "." + sym + ', global);');
+      var full_sym = is_require[1];
+      var sym = tryStripPrefix(full_sym, pkg);
+      console.log("google_protobuf.exportSymbol('" + full_sym + "', " + module + sym + ', global);');
     }
   } else if (is_loadfromfile) {
     if (!module) {
+      console.log("var google_protobuf = require('google-protobuf');");
       console.log("var asserts = require('closure_asserts_commonjs');");
       console.log("var global = Function('return this')();");
       console.log("");
       console.log("// Bring asserts into the global namespace.");
-      console.log("for (var key in asserts) {");
-      console.log("  if (asserts.hasOwnProperty(key)) {");
-      console.log("    global[key] = asserts[key];");
-      console.log("  }");
-      console.log("}");
-      console.log("");
-      console.log("var google_protobuf = require('google-protobuf');");
+      console.log("google_protobuf.object.extend(global, asserts);");
     }
     module = is_loadfromfile[1].replace("-", "_");
+    pkg = is_loadfromfile[2];
 
     if (module != "google_protobuf") {  // We unconditionally require this in the header.
       console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
diff --git a/js/message_test.js b/js/message_test.js
index 63da8532f..f572188e6 100644
--- a/js/message_test.js
+++ b/js/message_test.js
@@ -35,19 +35,19 @@ goog.setTestOnly();
 goog.require('goog.json');
 goog.require('goog.testing.asserts');
 
-// CommonJS-LoadFromFile: google-protobuf
+// CommonJS-LoadFromFile: google-protobuf jspb
 goog.require('jspb.Message');
 
-// CommonJS-LoadFromFile: test5_pb
+// CommonJS-LoadFromFile: test5_pb proto.jspb.exttest.beta
 goog.require('proto.jspb.exttest.beta.floatingStrField');
 
-// CommonJS-LoadFromFile: test3_pb
+// CommonJS-LoadFromFile: test3_pb proto.jspb.exttest
 goog.require('proto.jspb.exttest.floatingMsgField');
 
-// CommonJS-LoadFromFile: test4_pb
+// CommonJS-LoadFromFile: test4_pb proto.jspb.exttest
 goog.require('proto.jspb.exttest.floatingMsgFieldTwo');
 
-// CommonJS-LoadFromFile: test_pb
+// CommonJS-LoadFromFile: test_pb proto.jspb.test
 goog.require('proto.jspb.test.CloneExtension');
 goog.require('proto.jspb.test.Complex');
 goog.require('proto.jspb.test.DefaultValues');
@@ -59,6 +59,7 @@ goog.require('proto.jspb.test.IndirectExtension');
 goog.require('proto.jspb.test.IsExtension');
 goog.require('proto.jspb.test.OptionalFields');
 goog.require('proto.jspb.test.OuterEnum');
+goog.require('proto.jspb.test.OuterMessage.Complex');
 goog.require('proto.jspb.test.simple1');
 goog.require('proto.jspb.test.Simple1');
 goog.require('proto.jspb.test.Simple2');
@@ -70,7 +71,7 @@ goog.require('proto.jspb.test.TestMessageWithOneof');
 goog.require('proto.jspb.test.TestReservedNames');
 goog.require('proto.jspb.test.TestReservedNamesExtension');
 
-// CommonJS-LoadFromFile: test2_pb
+// CommonJS-LoadFromFile: test2_pb proto.jspb.test
 goog.require('proto.jspb.test.ExtensionMessage');
 goog.require('proto.jspb.test.TestExtensionsMessage');
 goog.require('proto.jspb.test.floatingMsgField');
@@ -97,6 +98,12 @@ describe('Message test suite', function() {
     assertEquals('some_bytes', data.getBytesField());
   });
 
+  it('testNestedMessage', function() {
+    var msg = new proto.jspb.test.OuterMessage.Complex();
+    msg.setInnerComplexField(5);
+    assertObjectEquals({innerComplexField: 5}, msg.toObject());
+  });
+
   it('testComplexConversion', function() {
     var data1 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];
     var data2 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];
diff --git a/js/proto3_test.js b/js/proto3_test.js
index e4116cb9c..f8868716d 100644
--- a/js/proto3_test.js
+++ b/js/proto3_test.js
@@ -30,10 +30,10 @@
 
 goog.require('goog.testing.asserts');
 
-// CommonJS-LoadFromFile: testbinary_pb
+// CommonJS-LoadFromFile: testbinary_pb proto.jspb.test
 goog.require('proto.jspb.test.ForeignMessage');
 
-// CommonJS-LoadFromFile: proto3_test_pb
+// CommonJS-LoadFromFile: proto3_test_pb proto.jspb.test
 goog.require('proto.jspb.test.Proto3Enum');
 goog.require('proto.jspb.test.TestProto3');
 
diff --git a/js/test.proto b/js/test.proto
index 5f9078ef3..3cea5f374 100644
--- a/js/test.proto
+++ b/js/test.proto
@@ -100,6 +100,13 @@ message Complex {
   repeated string a_repeated_string = 7;
 }
 
+message OuterMessage {
+  // Make sure this doesn't conflict with the other Complex message.
+  message Complex {
+    optional int32 inner_complex_field = 1;
+  }
+}
+
 message IsExtension {
   extend HasExtensions {
     optional IsExtension ext_field = 100;
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc
index 2dea60fa8..7ebb9b122 100755
--- a/src/google/protobuf/compiler/js/js_generator.cc
+++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -1466,13 +1466,6 @@ void Generator::GenerateClass(const GeneratorOptions& options,
       GenerateClassExtensionFieldInfo(options, printer, desc);
     }
 
-    if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
-      printer->Print(
-          "exports.$name$ = $fullName$;\n",
-          "name", desc->name(),
-          "fullName", GetPath(options, desc));
-    }
-
     if (options.import_style != GeneratorOptions:: IMPORT_CLOSURE) {
       for (int i = 0; i < desc->extension_count(); i++) {
         GenerateExtension(options, printer, desc->extension(i));
@@ -2552,6 +2545,11 @@ void Generator::GenerateFile(const GeneratorOptions& options,
   for (int i = 0; i < file->extension_count(); i++) {
     GenerateExtension(options, printer, file->extension(i));
   }
+
+  if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+    printer->Print("goog.object.extend(exports, $package$);\n",
+                   "package", GetPath(options, file));
+  }
 }
 
 bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,

From 35298f97793d2b875a349db852b17ba979cf5e13 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 4 Feb 2016 17:01:33 -0800
Subject: [PATCH 06/13] Fixed definition of extensions, and added CommonJS
 tests to Travis.

---
 js/gulpfile.js                                  | 4 ++++
 js/package.json                                 | 2 +-
 src/google/protobuf/compiler/js/js_generator.cc | 9 ++++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/js/gulpfile.js b/js/gulpfile.js
index bdc4212b2..d8f8ef4ad 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -88,3 +88,7 @@ gulp.task('test_commonjs', ['make_commonjs_out'], function (cb) {
     cb(err);
   });
 });
+
+gulp.task('test', ['test_closure', 'test_commonjs'], function(cb) {
+  cb();
+});
diff --git a/js/package.json b/js/package.json
index d37da1ddc..6418e5075 100644
--- a/js/package.json
+++ b/js/package.json
@@ -13,7 +13,7 @@
     "glob": "~6.0.4"
   },
   "scripts": {
-    "test": "./node_modules/gulp/bin/gulp.js test_closure"
+    "test": "./node_modules/gulp/bin/gulp.js test"
   },
   "repository": {
     "type": "git",
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc
index 7ebb9b122..351c39660 100755
--- a/src/google/protobuf/compiler/js/js_generator.cc
+++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -2530,8 +2530,15 @@ void Generator::GenerateFile(const GeneratorOptions& options,
   //   // Later generated code expects foo.bar = {} to exist:
   //   foo.bar.Baz = function() { /* ... */ }
   std::set<std::string> provided;
+
+  // Cover the case where this file declares extensions but no messages.
+  // This will ensure that the file-level object will be declared to hold
+  // the extensions.
+  for (int i = 0; i < file->extension_count(); i++) {
+    provided.insert(file->extension(i)->full_name());
+  }
+
   FindProvidesForFile(options, printer, file, &provided);
-  //FindProvidesForFields(options, printer, extensions, &provided);
   for (std::set<string>::iterator it = provided.begin();
        it != provided.end(); ++it) {
     printer->Print("goog.exportSymbol('$name$', null, global);\n",

From 59ea5000bb07f09635229752361004ffe30dc85f Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Fri, 5 Feb 2016 15:07:15 -0800
Subject: [PATCH 07/13] Use "node" as binary instead of "nodejs".

"nodejs" does not exist on Travis, it appears.
---
 js/gulpfile.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/js/gulpfile.js b/js/gulpfile.js
index d8f8ef4ad..88bb0022b 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -45,7 +45,7 @@ gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts']
   // Will require proper externs/exports.
   var cmd = "mkdir -p commonjs_out/binary && ";
   function addTestFile(file) {
-    cmd += 'nodejs commonjs/rewrite_tests_for_commonjs.js < ' + file +
+    cmd += 'node commonjs/rewrite_tests_for_commonjs.js < ' + file +
            ' > commonjs_out/' + file + '&& ';
   }
 

From 5195b7f296724cde9a646b76603ef81cd41a22c2 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Fri, 5 Feb 2016 18:05:11 -0800
Subject: [PATCH 08/13] Greatly expanded README.md.

---
 js/README.md | 140 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 130 insertions(+), 10 deletions(-)

diff --git a/js/README.md b/js/README.md
index fc144a3d1..4b1ec8a15 100644
--- a/js/README.md
+++ b/js/README.md
@@ -1,14 +1,134 @@
-This directory contains Protocol Buffer support for JavaScript.  This code works
-in browsers and in Node.js.
+Protocol Buffers - Google's data interchange format
+===================================================
 
-The packaging work for this is still in-progress.  For now you can just run the
-tests.  First you need to build the main C++ distribution because the code
-generator for JavaScript is written in C++:
+[![Build Status](https://travis-ci.org/google/protobuf.svg?branch=master)](https://travis-ci.org/google/protobuf)
 
-   $ ./autogen.sh
-   $ ./configure
-   $ make
+Copyright 2008 Google Inc.
 
-Then you can run the JavaScript tests in this directory:
+This directory contains the JavaScript Protocol Buffers runtime library.
 
-   $ cd js && gulp test
+The library is currently compatible with:
+
+1. CommonJS-style imports (eg. `var protos = require('my-protos');`)
+2. Closure-style imports (eg. `goog.require('my.package.MyProto');`)
+
+Support for ES6-style imports is not implemented yet.  Browsers can
+be supported by using Browserify, webpack, Closure Compiler, etc. to
+resolve imports at compile time.
+
+To use Protocol Buffers with JavaScript, you need two main components:
+
+1. The protobuf runtime library.  You can install this with
+   `npm install google-protobuf`, or use the files in this directory.
+2. The Protocol Compiler `protoc`.  This translates `.proto` files
+   into `.js` files.  The compiler is not currently available via
+   npm -- you must download and compile it from GitHub or a tarball.
+
+
+Setup
+=====
+
+First, compile the Protocol Compiler.
+You can compile `protoc` from GitHub or a source tarball.  From the
+top level directory type:
+
+    $ ./autogen.sh  (only necessary for GitHub)
+    $ ./configure
+    $ make
+
+
+Once you have `protoc` compiled, you can run the tests by typing:
+
+    $ npm install
+    $ npm test
+
+This will run two separate copies of the tests: one that uses
+Closure Compiler style imports and one that uses CommonJS imports.
+You can see all the CommonJS files in `commonjs_out/`.
+If all of these tests pass, you know you have a working setup.
+
+
+Using Protocol Buffers in your own project
+==========================================
+
+To use Protocol Buffers in your own project, you need to integrate
+the Protocol Compiler into your build system.  The details are a
+little different depending on whether you are using Closure imports
+or CommonJS imports:
+
+Closure Imports
+---------------
+
+If you want to use Closure imports, your build should run a command
+like this:
+
+    $ protoc --js_out=library=myproto_libs,binary:. messages.proto base.proto
+
+For Closure imports, `protoc` will generate a single output file
+(`myproto_libs.js` in this example).  The generated file will `goog.provide()`
+all of the types defined in your .proto files.  For example, for the unit
+tests the generated files contain many `goog.provide` statements like:
+
+    goog.provide('proto.google.protobuf.DescriptorProto');
+    goog.provide('proto.google.protobuf.DescriptorProto.ExtensionRange');
+    goog.provide('proto.google.protobuf.DescriptorProto.ReservedRange');
+    goog.provide('proto.google.protobuf.EnumDescriptorProto');
+    goog.provide('proto.google.protobuf.EnumOptions');
+
+The generated code will also `goog.require()` many types in the core library,
+and they will require many types in the Google Closure library.  So make sure
+that your `goog.provide()` / `goog.require()` setup can find all of your
+generated code, the core library `.js` files in this directory, and the
+Google Closure library itself.
+
+Once you've done this, you should be able to import your types with
+statements like:
+
+    goog.require('proto.my.package.MyMessage');
+
+    var message = proto.my.package.MyMessage();
+
+CommonJS imports
+----------------
+
+If you want to use CommonJS imports, your build should run a command
+like this:
+
+    $ protoc --js_out=import_style=commonjs,binary:. messages.proto base.proto
+
+For CommonJS imports, `protoc` will spit out one file per input file
+(so `messages_pb.js` and `base_pb.js` in this example).  The generated
+code will depend on the core runtime, which should be in a file called
+`google-protobuf.js`.  If you are installing from `npm`, this file should
+already be built and available.  If you are running from GitHub, you need
+to build it first by running:
+
+    $ gulp dist
+
+Once you've done this, you should be able to import your types with
+statements like:
+
+    var messages = require('./messages_pb');
+
+    var message = new messages.MyMessage();
+
+API
+===
+
+The API is not well-documented yet.  Here is a quick example to give you an
+idea of how the library generally works:
+
+    var message = new MyMessage();
+
+    message.setName("John Doe");
+    message.setAge(25);
+    message.setPhoneNumbers(["800-555-1212", "800-555-0000"]);
+
+    // Serializes to a UInt8Array.
+    bytes = message.serializeBinary();
+
+    var message2 = new MyMessage();
+    message2.deserializeBinary(bytes);
+
+For more examples, see the tests.  You can also look at the generated code
+to see what methods are defined for your generated messages.

From 7726cd207c66f2595f647e79c5c2c499eed7f8db Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Tue, 16 Feb 2016 15:29:49 -0800
Subject: [PATCH 09/13] Integrate review comments.

---
 js/README.md                              | 4 +++-
 js/binary/utils_test.js                   | 2 +-
 js/commonjs/export.js                     | 6 ++++++
 js/commonjs/export_asserts.js             | 4 ++++
 js/commonjs/rewrite_tests_for_commonjs.js | 8 ++++----
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/js/README.md b/js/README.md
index 4b1ec8a15..9d63559f9 100644
--- a/js/README.md
+++ b/js/README.md
@@ -22,7 +22,9 @@ To use Protocol Buffers with JavaScript, you need two main components:
    `npm install google-protobuf`, or use the files in this directory.
 2. The Protocol Compiler `protoc`.  This translates `.proto` files
    into `.js` files.  The compiler is not currently available via
-   npm -- you must download and compile it from GitHub or a tarball.
+   npm, but you can download a pre-built binary
+   [on GitHub](https://github.com/google/protobuf/releases)
+   (look for the `protoc-*.zip` files under **Downloads**).
 
 
 Setup
diff --git a/js/binary/utils_test.js b/js/binary/utils_test.js
index 37bab080c..1b90855d1 100644
--- a/js/binary/utils_test.js
+++ b/js/binary/utils_test.js
@@ -39,7 +39,7 @@
 goog.require('goog.crypt.base64');
 goog.require('goog.testing.asserts');
 
-// CommonJS-LoadFromFile: google_protobuf
+// CommonJS-LoadFromFile: googleProtobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryWriter');
 goog.require('jspb.utils');
diff --git a/js/commonjs/export.js b/js/commonjs/export.js
index 2fc2fcbd5..a3cfbd6f1 100644
--- a/js/commonjs/export.js
+++ b/js/commonjs/export.js
@@ -5,6 +5,12 @@
  * the google-protobuf.js file that we build at distribution time.
  */
 
+goog.require('goog.object');
+goog.require('jspb.BinaryReader');
+goog.require('jspb.BinaryWriter');
+goog.require('jspb.ExtensionFieldInfo');
+goog.require('jspb.Message');
+
 exports.Message = jspb.Message;
 exports.BinaryReader = jspb.BinaryReader;
 exports.BinaryWriter = jspb.BinaryWriter;
diff --git a/js/commonjs/export_asserts.js b/js/commonjs/export_asserts.js
index 9b823d3c5..5219d120c 100644
--- a/js/commonjs/export_asserts.js
+++ b/js/commonjs/export_asserts.js
@@ -30,4 +30,8 @@ for (var key in global) {
   }
 }
 
+// The COMPILED variable is set by Closure compiler to "true" when it compiles
+// JavaScript, so in practice this is equivalent to "exports.COMPILED = true".
+// This will disable some debugging functionality in debug.js.  We could
+// investigate whether this can/should be enabled in CommonJS builds.
 exports.COMPILED = COMPILED
diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
index 6a655c1e5..2ab7b2c15 100644
--- a/js/commonjs/rewrite_tests_for_commonjs.js
+++ b/js/commonjs/rewrite_tests_for_commonjs.js
@@ -46,21 +46,21 @@ lineReader.on('line', function(line) {
     if (module) {  // Skip goog.require() lines before the first directive.
       var full_sym = is_require[1];
       var sym = tryStripPrefix(full_sym, pkg);
-      console.log("google_protobuf.exportSymbol('" + full_sym + "', " + module + sym + ', global);');
+      console.log("googleProtobuf.exportSymbol('" + full_sym + "', " + module + sym + ', global);');
     }
   } else if (is_loadfromfile) {
     if (!module) {
-      console.log("var google_protobuf = require('google-protobuf');");
+      console.log("var googleProtobuf = require('google-protobuf');");
       console.log("var asserts = require('closure_asserts_commonjs');");
       console.log("var global = Function('return this')();");
       console.log("");
       console.log("// Bring asserts into the global namespace.");
-      console.log("google_protobuf.object.extend(global, asserts);");
+      console.log("googleProtobuf.object.extend(global, asserts);");
     }
     module = is_loadfromfile[1].replace("-", "_");
     pkg = is_loadfromfile[2];
 
-    if (module != "google_protobuf") {  // We unconditionally require this in the header.
+    if (module != "googleProtobuf") {  // We unconditionally require this in the header.
       console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
     }
   } else {

From c348af2fc0da16d8bd94992f7db969be90d2b115 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Wed, 17 Feb 2016 17:06:46 -0800
Subject: [PATCH 10/13] Addressed more code review comments.

---
 js/README.md                              | 13 ++++----
 js/commonjs/rewrite_tests_for_commonjs.js | 37 ++++++++++++++---------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/js/README.md b/js/README.md
index 9d63559f9..2c33fb79d 100644
--- a/js/README.md
+++ b/js/README.md
@@ -30,17 +30,16 @@ To use Protocol Buffers with JavaScript, you need two main components:
 Setup
 =====
 
-First, compile the Protocol Compiler.
-You can compile `protoc` from GitHub or a source tarball.  From the
-top level directory type:
-
-    $ ./autogen.sh  (only necessary for GitHub)
-    $ ./configure
-    $ make
+First, obtain the Protocol Compiler.  The easiest way is to download
+a pre-built binary from [https://github.com/google/protobuf/releases](https://github.com/google/protobuf/releases).
 
+If you want, you can compile `protoc` from source instead.  To do this
+follow the instructions in [the top-level
+README](https://github.com/google/protobuf/blob/master/src/README.md).
 
 Once you have `protoc` compiled, you can run the tests by typing:
 
+    $ cd js
     $ npm install
     $ npm test
 
diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
index 2ab7b2c15..397320b2b 100644
--- a/js/commonjs/rewrite_tests_for_commonjs.js
+++ b/js/commonjs/rewrite_tests_for_commonjs.js
@@ -19,7 +19,16 @@
  *
  * This script parses that special comment and uses it to generate proper
  * CommonJS require() statements so that the tests can run and pass using
- * CommonJS imports.
+ * CommonJS imports.  The script will change the above statements into:
+ *
+ *   var test_pb = require('test_pb');
+ *   googleProtobuf.exportSymbol('proto.jspb.test.CloneExtension', test_pb.CloneExtension, global);
+ *   googleProtobuf.exportSymbol('proto.jspb.test.Complex', test_pb.Complex, global);
+ *   googleProtobuf.exportSymbol('proto.jspb.test.DefaultValues', test_pb.DefaultValues, global);
+ *
+ * (The "exportSymbol" function will define the given names in the global
+ * namespace, taking care not to overwrite any previous value for
+ * "proto.jspb.test").
  */
 
 var lineReader = require('readline').createInterface({
@@ -37,18 +46,16 @@ function tryStripPrefix(str, prefix) {
 var module = null;
 var pkg = null;
 lineReader.on('line', function(line) {
-  var is_require = line.match(/goog\.require\('([^']*)'\)/);
-  var is_loadfromfile = line.match(/CommonJS-LoadFromFile: ([^ ]*) (.*)/);
-  var is_settestonly = line.match(/goog.setTestOnly()/);
-  if (is_settestonly) {
-    // Remove this line.
-  } else if (is_require) {
+  var isRequire = line.match(/goog\.require\('([^']*)'\)/);
+  var isLoadFromFile = line.match(/CommonJS-LoadFromFile: (\S*) (.*)/);
+  var isSetTestOnly = line.match(/goog.setTestOnly()/);
+  if (isRequire) {
     if (module) {  // Skip goog.require() lines before the first directive.
-      var full_sym = is_require[1];
-      var sym = tryStripPrefix(full_sym, pkg);
-      console.log("googleProtobuf.exportSymbol('" + full_sym + "', " + module + sym + ', global);');
+      var fullSym = isRequire[1];
+      var sym = tryStripPrefix(fullSym, pkg);
+      console.log("googleProtobuf.exportSymbol('" + fullSym + "', " + module + sym + ', global);');
     }
-  } else if (is_loadfromfile) {
+  } else if (isLoadFromFile) {
     if (!module) {
       console.log("var googleProtobuf = require('google-protobuf');");
       console.log("var asserts = require('closure_asserts_commonjs');");
@@ -57,13 +64,13 @@ lineReader.on('line', function(line) {
       console.log("// Bring asserts into the global namespace.");
       console.log("googleProtobuf.object.extend(global, asserts);");
     }
-    module = is_loadfromfile[1].replace("-", "_");
-    pkg = is_loadfromfile[2];
+    module = isLoadFromFile[1].replace("-", "_");
+    pkg = isLoadFromFile[2];
 
     if (module != "googleProtobuf") {  // We unconditionally require this in the header.
-      console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
+      console.log("var " + module + " = require('" + isLoadFromFile[1] + "');");
     }
-  } else {
+  } else if (!isSetTestOnly) {  // Remove goog.setTestOnly() lines.
     console.log(line);
   }
 });

From 29d58d3392337228e05c5883f2ffdc06ac8cc983 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 18 Feb 2016 10:40:07 -0800
Subject: [PATCH 11/13] Removed unused directives from tests that aren't run
 under CommonJS.

---
 js/binary/reader_test.js | 2 --
 js/binary/utils_test.js  | 2 --
 2 files changed, 4 deletions(-)

diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js
index c0f127021..a6482610e 100644
--- a/js/binary/reader_test.js
+++ b/js/binary/reader_test.js
@@ -42,8 +42,6 @@
  */
 
 goog.require('goog.testing.asserts');
-
-// CommonJS-LoadFromFile: google_protobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryDecoder');
 goog.require('jspb.BinaryReader');
diff --git a/js/binary/utils_test.js b/js/binary/utils_test.js
index 1b90855d1..5c330791a 100644
--- a/js/binary/utils_test.js
+++ b/js/binary/utils_test.js
@@ -38,8 +38,6 @@
 
 goog.require('goog.crypt.base64');
 goog.require('goog.testing.asserts');
-
-// CommonJS-LoadFromFile: googleProtobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryWriter');
 goog.require('jspb.utils');

From 907ad4a00467e7e39e97b7147040f2a83c61789d Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Thu, 18 Feb 2016 10:46:44 -0800
Subject: [PATCH 12/13] Properly camelCase when translating to CommonJS.

---
 js/commonjs/rewrite_tests_for_commonjs.js | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js
index 397320b2b..dc5effec5 100644
--- a/js/commonjs/rewrite_tests_for_commonjs.js
+++ b/js/commonjs/rewrite_tests_for_commonjs.js
@@ -43,6 +43,22 @@ function tryStripPrefix(str, prefix) {
   return str.substr(prefix.length);
 }
 
+function camelCase(str) {
+  var ret = '';
+  var ucaseNext = false;
+  for (var i = 0; i < str.length; i++) {
+    if (str[i] == '-') {
+      ucaseNext = true;
+    } else if (ucaseNext) {
+      ret += str[i].toUpperCase();
+      ucaseNext = false;
+    } else {
+      ret += str[i];
+    }
+  }
+  return ret;
+}
+
 var module = null;
 var pkg = null;
 lineReader.on('line', function(line) {
@@ -64,7 +80,7 @@ lineReader.on('line', function(line) {
       console.log("// Bring asserts into the global namespace.");
       console.log("googleProtobuf.object.extend(global, asserts);");
     }
-    module = isLoadFromFile[1].replace("-", "_");
+    module = camelCase(isLoadFromFile[1])
     pkg = isLoadFromFile[2];
 
     if (module != "googleProtobuf") {  // We unconditionally require this in the header.

From 24c5424be5b220d8d4575e4262ee5d6ad9417959 Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Fri, 19 Feb 2016 11:46:03 -0800
Subject: [PATCH 13/13] Added a bit more to README.md, and allowed custom
 PROTOC var in tests.

---
 js/README.md   | 24 ++++++++++++++++++++++++
 js/gulpfile.js |  6 ++++--
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/js/README.md b/js/README.md
index 2c33fb79d..15d48c87f 100644
--- a/js/README.md
+++ b/js/README.md
@@ -43,6 +43,10 @@ Once you have `protoc` compiled, you can run the tests by typing:
     $ npm install
     $ npm test
 
+    # If your protoc is somewhere else than ../src/protoc, instead do this.
+    # But make sure your protoc is the same version as this (or compatible)!
+    $ PROTOC=/usr/local/bin/protoc npm test
+
 This will run two separate copies of the tests: one that uses
 Closure Compiler style imports and one that uses CommonJS imports.
 You can see all the CommonJS files in `commonjs_out/`.
@@ -113,6 +117,26 @@ statements like:
 
     var message = new messages.MyMessage();
 
+The `--js_out` flag
+-------------------
+
+The syntax of the `--js_out` flag is:
+
+    --js_out=[OPTIONS:]output_dir
+
+Where `OPTIONS` are separated by commas.  Options are either `opt=val` or
+just `opt` (for options that don't take a value).  The available options
+are specified and documented in the `GeneratorOptions` struct in
+[src/google/protobuf/compiler/js/js_generator.h](https://github.com/google/protobuf/blob/master/src/google/protobuf/compiler/js/js_generator.h#L53).
+
+Some examples:
+
+- `--js_out=library=myprotos_lib.js,binary:.`: this contains the options
+  `library=myprotos.lib.js` and `binary` and outputs to the current directory.
+  The `import_style` option is left to the default, which is `closure`.
+- `--js_out=import_style=commonjs,binary:protos`: this contains the options
+  `import_style=commonjs` and `binary` and outputs to the directory `protos`.
+
 API
 ===
 
diff --git a/js/gulpfile.js b/js/gulpfile.js
index 88bb0022b..b0faed06a 100644
--- a/js/gulpfile.js
+++ b/js/gulpfile.js
@@ -2,8 +2,10 @@ var gulp = require('gulp');
 var exec = require('child_process').exec;
 var glob = require('glob');
 
+var protoc = process.env.PROTOC || '../src/protoc';
+
 gulp.task('genproto_closure', function (cb) {
-  exec('../src/protoc --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
+  exec(protoc + ' --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -12,7 +14,7 @@ gulp.task('genproto_closure', function (cb) {
 });
 
 gulp.task('genproto_commonjs', function (cb) {
-  exec('mkdir -p commonjs_out && ../src/protoc --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
+  exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);