Browse Source

Fixed nested message scopes for CommonJS.

Josh Haberman 9 years ago
parent
commit
77af5d04b1

+ 1 - 1
js/binary/proto_test.js

@@ -32,7 +32,7 @@
 
 
 goog.require('goog.testing.asserts');
 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.ExtendsWithMessage');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignMessage');
 goog.require('proto.jspb.test.ForeignMessage');

+ 2 - 0
js/commonjs/export.js

@@ -10,5 +10,7 @@ exports.BinaryReader = jspb.BinaryReader;
 exports.BinaryWriter = jspb.BinaryWriter;
 exports.BinaryWriter = jspb.BinaryWriter;
 exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo;
 exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo;
 
 
+// These are used by generated code but should not be used directly by clients.
 exports.exportSymbol = goog.exportSymbol;
 exports.exportSymbol = goog.exportSymbol;
 exports.inherits = goog.inherits;
 exports.inherits = goog.inherits;
+exports.object = {extend: goog.object.extend};

+ 16 - 12
js/commonjs/rewrite_tests_for_commonjs.js

@@ -27,34 +27,38 @@ var lineReader = require('readline').createInterface({
   output: process.stdout
   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 module = null;
+var pkg = null;
 lineReader.on('line', function(line) {
 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()/);
   var is_settestonly = line.match(/goog.setTestOnly()/);
   if (is_settestonly) {
   if (is_settestonly) {
     // Remove this line.
     // Remove this line.
   } else if (is_require) {
   } else if (is_require) {
     if (module) {  // Skip goog.require() lines before the first directive.
     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) {
   } else if (is_loadfromfile) {
     if (!module) {
     if (!module) {
+      console.log("var google_protobuf = require('google-protobuf');");
       console.log("var asserts = require('closure_asserts_commonjs');");
       console.log("var asserts = require('closure_asserts_commonjs');");
       console.log("var global = Function('return this')();");
       console.log("var global = Function('return this')();");
       console.log("");
       console.log("");
       console.log("// Bring asserts into the global namespace.");
       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("-", "_");
     module = is_loadfromfile[1].replace("-", "_");
+    pkg = is_loadfromfile[2];
 
 
     if (module != "google_protobuf") {  // We unconditionally require this in the header.
     if (module != "google_protobuf") {  // We unconditionally require this in the header.
       console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
       console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");

+ 13 - 6
js/message_test.js

@@ -35,19 +35,19 @@ goog.setTestOnly();
 goog.require('goog.json');
 goog.require('goog.json');
 goog.require('goog.testing.asserts');
 goog.require('goog.testing.asserts');
 
 
-// CommonJS-LoadFromFile: google-protobuf
+// CommonJS-LoadFromFile: google-protobuf jspb
 goog.require('jspb.Message');
 goog.require('jspb.Message');
 
 
-// CommonJS-LoadFromFile: test5_pb
+// CommonJS-LoadFromFile: test5_pb proto.jspb.exttest.beta
 goog.require('proto.jspb.exttest.beta.floatingStrField');
 goog.require('proto.jspb.exttest.beta.floatingStrField');
 
 
-// CommonJS-LoadFromFile: test3_pb
+// CommonJS-LoadFromFile: test3_pb proto.jspb.exttest
 goog.require('proto.jspb.exttest.floatingMsgField');
 goog.require('proto.jspb.exttest.floatingMsgField');
 
 
-// CommonJS-LoadFromFile: test4_pb
+// CommonJS-LoadFromFile: test4_pb proto.jspb.exttest
 goog.require('proto.jspb.exttest.floatingMsgFieldTwo');
 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.CloneExtension');
 goog.require('proto.jspb.test.Complex');
 goog.require('proto.jspb.test.Complex');
 goog.require('proto.jspb.test.DefaultValues');
 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.IsExtension');
 goog.require('proto.jspb.test.OptionalFields');
 goog.require('proto.jspb.test.OptionalFields');
 goog.require('proto.jspb.test.OuterEnum');
 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.Simple1');
 goog.require('proto.jspb.test.Simple1');
 goog.require('proto.jspb.test.Simple2');
 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.TestReservedNames');
 goog.require('proto.jspb.test.TestReservedNamesExtension');
 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.ExtensionMessage');
 goog.require('proto.jspb.test.TestExtensionsMessage');
 goog.require('proto.jspb.test.TestExtensionsMessage');
 goog.require('proto.jspb.test.floatingMsgField');
 goog.require('proto.jspb.test.floatingMsgField');
@@ -97,6 +98,12 @@ describe('Message test suite', function() {
     assertEquals('some_bytes', data.getBytesField());
     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() {
   it('testComplexConversion', function() {
     var data1 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];
     var data1 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];
     var data2 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];
     var data2 = ['a',,, [, 11], [[, 22], [, 33]],, ['s1', 's2'],, 1];

+ 2 - 2
js/proto3_test.js

@@ -30,10 +30,10 @@
 
 
 goog.require('goog.testing.asserts');
 goog.require('goog.testing.asserts');
 
 
-// CommonJS-LoadFromFile: testbinary_pb
+// CommonJS-LoadFromFile: testbinary_pb proto.jspb.test
 goog.require('proto.jspb.test.ForeignMessage');
 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.Proto3Enum');
 goog.require('proto.jspb.test.TestProto3');
 goog.require('proto.jspb.test.TestProto3');
 
 

+ 7 - 0
js/test.proto

@@ -100,6 +100,13 @@ message Complex {
   repeated string a_repeated_string = 7;
   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 {
 message IsExtension {
   extend HasExtensions {
   extend HasExtensions {
     optional IsExtension ext_field = 100;
     optional IsExtension ext_field = 100;

+ 5 - 7
src/google/protobuf/compiler/js/js_generator.cc

@@ -1466,13 +1466,6 @@ void Generator::GenerateClass(const GeneratorOptions& options,
       GenerateClassExtensionFieldInfo(options, printer, desc);
       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) {
     if (options.import_style != GeneratorOptions:: IMPORT_CLOSURE) {
       for (int i = 0; i < desc->extension_count(); i++) {
       for (int i = 0; i < desc->extension_count(); i++) {
         GenerateExtension(options, printer, desc->extension(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++) {
   for (int i = 0; i < file->extension_count(); i++) {
     GenerateExtension(options, printer, file->extension(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,
 bool Generator::GenerateAll(const vector<const FileDescriptor*>& files,