Merge pull request #4299 from hekike/feat/add-commonjs-strict-import-style

Feat: add import-style=commonjs_strict option to the compiler
pull/4360/merge
Joshua Haberman 6 years ago committed by GitHub
commit 0ea3d74c3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      Makefile.am
  2. 1
      js/README.md
  3. 67
      js/commonjs/strict_test.js
  4. 18
      js/gulpfile.js
  5. 39
      js/test10.proto
  6. 39
      js/test9.proto
  7. 37
      src/google/protobuf/compiler/js/js_generator.cc
  8. 9
      src/google/protobuf/compiler/js/js_generator.h

@ -935,6 +935,7 @@ js_EXTRA_DIST= \
js/commonjs/import_test.js \
js/commonjs/jasmine.json \
js/commonjs/rewrite_tests_for_commonjs.js \
js/commonjs/strict_test.js \
js/commonjs/test6/test6.proto \
js/commonjs/test7/test7.proto \
js/compatibility_tests/v3.0.0/binary/arith_test.js \
@ -1006,6 +1007,8 @@ js_EXTRA_DIST= \
js/test4.proto \
js/test5.proto \
js/test8.proto \
js/test9.proto \
js/test10.proto \
js/test_bootstrap.js \
js/testbinary.proto \
js/testempty.proto

@ -144,6 +144,7 @@ Some examples:
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`.
`import_style=commonjs_strict` doesn't expose the output on the global scope.
API
===

@ -0,0 +1,67 @@
// Protocol Buffers - Google's data interchange format
// Copyright 2016 Google Inc. All rights reserved.
// https://developers.google.com/protocol-buffers/
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Test suite is written using Jasmine -- see http://jasmine.github.io/
var googleProtobuf = require('google-protobuf');
var asserts = require('closure_asserts_commonjs');
var global = Function('return this')();
// Bring asserts into the global namespace.
googleProtobuf.object.extend(global, asserts);
var test9_pb = require('./test9_pb');
var test10_pb = require('./test10_pb');
describe('Strict test suite', function() {
it('testImportedMessage', function() {
var simple1 = new test9_pb.jspb.exttest.strict.nine.Simple9()
var simple2 = new test9_pb.jspb.exttest.strict.nine.Simple9()
assertObjectEquals(simple1.toObject(), simple2.toObject());
});
it('testGlobalScopePollution', function() {
assertObjectEquals(global.jspb.exttest, undefined);
});
describe('with imports', function() {
it('testImportedMessage', function() {
var simple1 = new test10_pb.jspb.exttest.strict.ten.Simple10()
var simple2 = new test10_pb.jspb.exttest.strict.ten.Simple10()
assertObjectEquals(simple1.toObject(), simple2.toObject());
});
it('testGlobalScopePollution', function() {
assertObjectEquals(global.jspb.exttest, undefined);
});
});
});

@ -41,6 +41,12 @@ var group2Protos = [
'commonjs/test7/test7.proto',
];
var group3Protos = [
'test9.proto',
'test10.proto'
];
gulp.task('genproto_well_known_types_closure', function (cb) {
exec(protoc + ' --js_out=one_output_file_per_input_file,binary:. -I ../src -I . ' + wellKnownTypes.join(' '),
function (err, stdout, stderr) {
@ -112,6 +118,15 @@ gulp.task('genproto_wellknowntypes', function (cb) {
cb(err);
});
});
gulp.task('genproto_group3_commonjs_strict', function (cb) {
exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs_strict,binary:commonjs_out -I ../src -I commonjs -I . ' + group3Protos.join(' '),
function (err, stdout, stderr) {
console.log(stdout);
console.log(stderr);
cb(err);
});
});
function getClosureBuilderCommand(exportsFile, outputFile) {
return './node_modules/google-closure-library/closure/bin/build/closurebuilder.py ' +
@ -159,7 +174,7 @@ gulp.task('commonjs_testdeps', function (cb) {
});
});
gulp.task('make_commonjs_out', ['dist', 'genproto_well_known_types_commonjs', 'genproto_group1_commonjs', 'genproto_group2_commonjs', 'genproto_commonjs_wellknowntypes', 'commonjs_asserts', 'commonjs_testdeps'], function (cb) {
gulp.task('make_commonjs_out', ['dist', 'genproto_well_known_types_commonjs', 'genproto_group1_commonjs', 'genproto_group2_commonjs', 'genproto_commonjs_wellknowntypes', 'commonjs_asserts', 'commonjs_testdeps', 'genproto_group3_commonjs_strict'], function (cb) {
// TODO(haberman): minify this more aggressively.
// Will require proper externs/exports.
var cmd = "mkdir -p commonjs_out/binary && mkdir -p commonjs_out/test_node_modules && ";
@ -174,6 +189,7 @@ gulp.task('make_commonjs_out', ['dist', 'genproto_well_known_types_commonjs', 'g
exec(cmd +
'cp commonjs/jasmine.json commonjs_out/jasmine.json && ' +
'cp google-protobuf.js commonjs_out/test_node_modules && ' +
'cp commonjs/strict_test.js commonjs_out/strict_test.js &&' +
'cp commonjs/import_test.js commonjs_out/import_test.js',
function (err, stdout, stderr) {
console.log(stdout);

@ -0,0 +1,39 @@
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
// https://developers.google.com/protocol-buffers/
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
syntax = "proto3";
package jspb.exttest.strict.ten;
import "test9.proto";
message Simple10 {
jspb.exttest.strict.nine.Simple9 a = 1;
}

@ -0,0 +1,39 @@
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
// https://developers.google.com/protocol-buffers/
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
syntax = "proto2";
package jspb.exttest.strict.nine;
message Simple9 {
required string a_string = 1;
repeated string a_repeated_string = 2;
optional bool a_boolean = 3;
}

@ -276,7 +276,8 @@ string GetEnumPath(const GeneratorOptions& options,
string MaybeCrossFileRef(const GeneratorOptions& options,
const FileDescriptor* from_file,
const Descriptor* to_message) {
if (options.import_style == GeneratorOptions::kImportCommonJs &&
if ((options.import_style == GeneratorOptions::kImportCommonJs ||
options.import_style == GeneratorOptions::kImportCommonJsStrict) &&
from_file != to_message->file()) {
// Cross-file ref in CommonJS needs to use the module alias instead of
// the global name.
@ -1675,8 +1676,19 @@ void Generator::GenerateProvides(const GeneratorOptions& options,
//
// // Later generated code expects foo.bar = {} to exist:
// foo.bar.Baz = function() { /* ... */ }
printer->Print("goog.exportSymbol('$name$', null, global);\n", "name",
*it);
// Do not use global scope in strict mode
if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
string namespaceObject = *it;
// Remove "proto." from the namespace object
GOOGLE_CHECK(namespaceObject.compare(0, 6, "proto."));
namespaceObject.erase(0, 6);
printer->Print("goog.exportSymbol('$name$', null, proto);\n", "name",
namespaceObject);
} else {
printer->Print("goog.exportSymbol('$name$', null, global);\n", "name",
*it);
}
}
}
}
@ -3325,6 +3337,8 @@ bool GeneratorOptions::ParseFromOptions(
import_style = kImportClosure;
} else if (options[i].second == "commonjs") {
import_style = kImportCommonJs;
} else if (options[i].second == "commonjs_strict") {
import_style = kImportCommonJsStrict;
} else if (options[i].second == "browser") {
import_style = kImportBrowser;
} else if (options[i].second == "es6") {
@ -3434,15 +3448,23 @@ void Generator::GenerateFile(const GeneratorOptions& options,
GenerateHeader(options, printer);
// Generate "require" statements.
if (options.import_style == GeneratorOptions::kImportCommonJs) {
if ((options.import_style == GeneratorOptions::kImportCommonJs ||
options.import_style == GeneratorOptions::kImportCommonJsStrict)) {
printer->Print("var jspb = require('google-protobuf');\n");
printer->Print("var goog = jspb;\n");
printer->Print("var global = Function('return this')();\n\n");
// Do not use global scope in strict mode
if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
printer->Print("var proto = {};\n\n");
} else {
printer->Print("var global = Function('return this')();\n\n");
}
for (int i = 0; i < file->dependency_count(); i++) {
const string& name = file->dependency(i)->name();
printer->Print(
"var $alias$ = require('$file$');\n",
"var $alias$ = require('$file$');\n"
"goog.object.extend(proto, $alias$);\n",
"alias", ModuleAlias(name),
"file",
GetRootPath(file->name(), name) + GetJSFilename(options, name));
@ -3485,6 +3507,9 @@ void Generator::GenerateFile(const GeneratorOptions& options,
if (options.import_style == GeneratorOptions::kImportCommonJs && !provided.empty()) {
printer->Print("goog.object.extend(exports, $package$);\n",
"package", GetFilePath(options, file));
} else if(options.import_style == GeneratorOptions::kImportCommonJsStrict) {
printer->Print("goog.object.extend(exports, proto);\n",
"package", GetFilePath(options, file));
}
// Emit well-known type methods.

@ -63,10 +63,11 @@ struct GeneratorOptions {
bool binary;
// What style of imports should be used.
enum ImportStyle {
kImportClosure, // goog.require()
kImportCommonJs, // require()
kImportBrowser, // no import statements
kImportEs6, // import { member } from ''
kImportClosure, // goog.require()
kImportCommonJs, // require()
kImportCommonJsStrict, // require() with no global export
kImportBrowser, // no import statements
kImportEs6, // import { member } from ''
} import_style;
GeneratorOptions()

Loading…
Cancel
Save