From e9f31ee3d7cf7c0f370607e54dbea01ba7240a77 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Feb 2016 10:29:27 -0800 Subject: CommonJS tests are now passing. --- js/binary/proto_test.js | 2 ++ js/binary/reader_test.js | 2 ++ js/binary/utils_test.js | 2 ++ 3 files changed, 6 insertions(+) (limited to 'js/binary') diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js index 1cb7ff0e..106ee243 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 a6482610..c0f12702 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 5c330791..37bab080 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'); -- cgit v1.2.3 From 77af5d04b1897baeda2ebd753d138c99afe72c50 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Feb 2016 16:11:07 -0800 Subject: 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 +++++++ src/google/protobuf/compiler/js/js_generator.cc | 12 +++++------ 7 files changed, 46 insertions(+), 28 deletions(-) (limited to 'js/binary') diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js index 106ee243..817f8a79 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 89ded330..2fc2fcbd 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 d49f8a93..6a655c1e 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 63da8532..f572188e 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 e4116cb9..f8868716 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 5f9078ef..3cea5f37 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 2dea60fa..7ebb9b12 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& files, -- cgit v1.2.3 From 7726cd207c66f2595f647e79c5c2c499eed7f8db Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Tue, 16 Feb 2016 15:29:49 -0800 Subject: 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(-) (limited to 'js/binary') diff --git a/js/README.md b/js/README.md index 4b1ec8a1..9d63559f 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 37bab080..1b90855d 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 2fc2fcbd..a3cfbd6f 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 9b823d3c..5219d120 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 6a655c1e..2ab7b2c1 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 { -- cgit v1.2.3 From 29d58d3392337228e05c5883f2ffdc06ac8cc983 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 18 Feb 2016 10:40:07 -0800 Subject: 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(-) (limited to 'js/binary') diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js index c0f12702..a6482610 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 1b90855d..5c330791 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'); -- cgit v1.2.3