From 55cc3aa987159bcdb491550d864115c1e8daeebb Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Tue, 2 Feb 2016 15:18:34 -0800 Subject: WIP. --- js/commonjs_export.js | 10 + js/gulpfile.js | 39 +++- js/jasmine_commonjs.json | 11 + js/package.json | 6 +- src/google/protobuf/compiler/js/js_generator.cc | 266 +++++++++++++++++++----- src/google/protobuf/compiler/js/js_generator.h | 18 +- 6 files changed, 286 insertions(+), 64 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 00000000..44e63296 --- /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 79095d65..a548a826 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('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('deps', ['genproto'], 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', + 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 00000000..05444550 --- /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 be93286f..8c8f434b 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 e6c3b36a..31938b11 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* 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& files, std::set* 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& files, std::set* provided) const { - std::set required; - std::set 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; - } + 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 required; + std::set forwards; + bool have_extensions = false; + bool have_message = false; - for (int j = 0; j < files[i]->extension_count(); j++) { - const FieldDescriptor* extension = files[i]->extension(j); - if (IgnoreField(extension)) { - continue; + 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 (extension->containing_type()->full_name() != - "google.protobuf.bridge.MessageSet") { - required.insert(GetPath(options, extension->containing_type())); + 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; } - FindRequiresForField(options, extension, &required, &forwards); - have_extensions = true; } - } - GenerateRequiresImpl(options, printer, &required, &forwards, provided, - /* require_jspb = */ have_message, - /* require_extension = */ have_extensions); + 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 + } } 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 provided; + FindProvidesForFile(options, printer, file, &provided); + //FindProvidesForFields(options, printer, extensions, &provided); + for (std::set::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& files, const string& parameter, GeneratorContext* context, @@ -2430,10 +2562,14 @@ bool Generator::GenerateAll(const vector& 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 output(context->Open(filename)); GOOGLE_CHECK(output.get()); @@ -2469,7 +2605,7 @@ bool Generator::GenerateAll(const vector& 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 > extensions_by_namespace; @@ -2611,6 +2747,24 @@ bool Generator::GenerateAll(const vector& 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 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 db2dceb3..db9178d3 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& file, std::set* provided) const; + void FindProvidesForFile(const GeneratorOptions& options, + io::Printer* printer, + const FileDescriptor* file, + std::set* 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* required, std::set* 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, -- cgit v1.2.3 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 ++ 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 +++++++++++++++++++++++++ src/google/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 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'); diff --git a/js/commonjs_export.js b/js/commonjs_export.js index 44e63296..ffbeb9db 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 00000000..16abdd7d --- /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 615fc7c6..d7bf3768 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 a548a826..22f8f3fd 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 05444550..666b8edb 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 971ea4f4..63da8532 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 8c8f434b..d37da1dd 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 8102bab6..e4116cb9 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 00000000..4dc0a22d --- /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 31938b11..2dea60fa 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::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); } -- cgit v1.2.3 From 9e60036c1b1db947d29cbaaa668aeae19e7d2068 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Feb 2016 10:38:06 -0800 Subject: Moved CommonJS-specific files to commonjs/. --- js/commonjs/export.js | 11 ++++++++ js/commonjs/export_asserts.js | 27 +++++++++++++++++++ js/commonjs/jasmine.json | 9 +++++++ js/commonjs/rewrite_tests_for_commonjs.js | 45 +++++++++++++++++++++++++++++++ js/commonjs_export.js | 11 -------- js/commonjs_export_asserts.js | 27 ------------------- js/gulpfile.js | 8 +++--- js/jasmine_commonjs.json | 9 ------- js/rewrite_tests_for_commonjs.js | 45 ------------------------------- 9 files changed, 96 insertions(+), 96 deletions(-) create mode 100644 js/commonjs/export.js create mode 100644 js/commonjs/export_asserts.js create mode 100644 js/commonjs/jasmine.json create mode 100644 js/commonjs/rewrite_tests_for_commonjs.js delete mode 100644 js/commonjs_export.js delete mode 100644 js/commonjs_export_asserts.js delete mode 100644 js/jasmine_commonjs.json delete mode 100644 js/rewrite_tests_for_commonjs.js diff --git a/js/commonjs/export.js b/js/commonjs/export.js new file mode 100644 index 00000000..ffbeb9db --- /dev/null +++ b/js/commonjs/export.js @@ -0,0 +1,11 @@ +/** + * @fileoverview Export symbols needed by generated code in CommonJS style. + */ + +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 00000000..16abdd7d --- /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/commonjs/jasmine.json b/js/commonjs/jasmine.json new file mode 100644 index 00000000..666b8edb --- /dev/null +++ b/js/commonjs/jasmine.json @@ -0,0 +1,9 @@ +{ + "spec_dir": "", + "spec_files": [ + "*_test.js", + "binary/proto_test.js" + ], + "helpers": [ + ] +} diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js new file mode 100644 index 00000000..4dc0a22d --- /dev/null +++ b/js/commonjs/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/js/commonjs_export.js b/js/commonjs_export.js deleted file mode 100644 index ffbeb9db..00000000 --- a/js/commonjs_export.js +++ /dev/null @@ -1,11 +0,0 @@ -/** - * @fileoverview Export symbols needed by generated code in CommonJS style. - */ - -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 deleted file mode 100644 index 16abdd7d..00000000 --- a/js/commonjs_export_asserts.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * @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/gulpfile.js b/js/gulpfile.js index 22f8f3fd..bdc4212b 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); diff --git a/js/jasmine_commonjs.json b/js/jasmine_commonjs.json deleted file mode 100644 index 666b8edb..00000000 --- a/js/jasmine_commonjs.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "spec_dir": "", - "spec_files": [ - "*_test.js", - "binary/proto_test.js" - ], - "helpers": [ - ] -} diff --git a/js/rewrite_tests_for_commonjs.js b/js/rewrite_tests_for_commonjs.js deleted file mode 100644 index 4dc0a22d..00000000 --- a/js/rewrite_tests_for_commonjs.js +++ /dev/null @@ -1,45 +0,0 @@ -/** - * @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); - } -}); -- cgit v1.2.3 From d6a186a8f18c1b14979e0244c0434b5b89c0f8a9 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Feb 2016 14:58:45 -0800 Subject: 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 ffbeb9db..89ded330 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 16abdd7d..9b823d3c 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 4dc0a22d..d49f8a93 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({ -- 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(-) 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 35298f97793d2b875a349db852b17ba979cf5e13 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Feb 2016 17:01:33 -0800 Subject: 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 bdc4212b..d8f8ef4a 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 d37da1dd..6418e507 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 7ebb9b12..351c3966 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 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::iterator it = provided.begin(); it != provided.end(); ++it) { printer->Print("goog.exportSymbol('$name$', null, global);\n", -- cgit v1.2.3 From 59ea5000bb07f09635229752361004ffe30dc85f Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 5 Feb 2016 15:07:15 -0800 Subject: 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 d8f8ef4a..88bb0022 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 + '&& '; } -- cgit v1.2.3 From 5195b7f296724cde9a646b76603ef81cd41a22c2 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 5 Feb 2016 18:05:11 -0800 Subject: 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 fc144a3d..4b1ec8a1 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. -- 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(-) 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 c348af2fc0da16d8bd94992f7db969be90d2b115 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 17 Feb 2016 17:06:46 -0800 Subject: 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 9d63559f..2c33fb79 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 2ab7b2c1..397320b2 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); } }); -- 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(-) 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 From 907ad4a00467e7e39e97b7147040f2a83c61789d Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 18 Feb 2016 10:46:44 -0800 Subject: 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 397320b2..dc5effec 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. -- cgit v1.2.3 From 24c5424be5b220d8d4575e4262ee5d6ad9417959 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 19 Feb 2016 11:46:03 -0800 Subject: 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 2c33fb79..15d48c87 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 88bb0022..b0faed06 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); -- cgit v1.2.3