Skip to content

Commit

Permalink
js: use commonjs_strict for well-known types
Browse files Browse the repository at this point in the history
When generating JS code for .proto files which included well-known types
with the commonjs_strict import style, the generated code would import
generated JS code for the well-known types with the commonjs import
style (located in the google-protobuf NPM package). This causes several
issues as discussed in #7778.

This CL changes this so when using commonjs_strict, the imported
generated JS code for well-known types also uses commonjs_strict.

Fixes #7778
  • Loading branch information
avm99963 committed Sep 6, 2021
1 parent 10d0fae commit a6ed2cf
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 3 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -166,6 +166,7 @@ vendor/

# JavaScript artifacts
js/commonjs_out/
js/commonjs_strict/
js/compatibility_tests/v3.0.0/commonjs_out*
js/compatibility_tests/v3.0.0/protoc
js/compatibility_tests/v3.0.0/testproto_libs1.js
Expand Down
12 changes: 11 additions & 1 deletion js/gulpfile.js
Expand Up @@ -130,6 +130,16 @@ gulp.task('genproto_wellknowntypes', function (cb) {
cb(err);
});
});

gulp.task('genproto_wellknowntypes_commonjs_strict', function (cb) {
exec('mkdir -p commonjs_strict && ' + protoc + ' --js_out=import_style=commonjs_strict,binary:./commonjs_strict -I ../src ' + wellKnownTypes.join(' '),
function (err, stdout, stderr) {
console.log(stdout);
console.log(stderr);
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) {
Expand All @@ -153,7 +163,7 @@ function getClosureCompilerCommand(exportsFile, outputFile) {
].join(' ');
}

gulp.task('dist', gulp.series(['genproto_wellknowntypes'], function(cb) {
gulp.task('dist', gulp.series(['genproto_wellknowntypes', 'genproto_wellknowntypes_commonjs_strict'], function(cb) {
// TODO(haberman): minify this more aggressively.
// Will require proper externs/exports.
exec(
Expand Down
9 changes: 7 additions & 2 deletions src/google/protobuf/compiler/js/js_generator.cc
Expand Up @@ -110,12 +110,17 @@ std::string GetJSFilename(const GeneratorOptions& options,
// Given a filename like foo/bar/baz.proto, returns the root directory
// path ../../
std::string GetRootPath(const std::string& from_filename,
const std::string& to_filename) {
const std::string& to_filename,
const GeneratorOptions& options) {
if (to_filename.find("google/protobuf") == 0) {
// Well-known types (.proto files in the google/protobuf directory) are
// assumed to come from the 'google-protobuf' npm package. We may want to
// generalize this exception later by letting others put generated code in
// their own npm packages.

if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
return "google-protobuf/commonjs_strict/";
}
return "google-protobuf/";
}

Expand Down Expand Up @@ -3634,7 +3639,7 @@ void Generator::GenerateFile(const GeneratorOptions& options,
"var $alias$ = require('$file$');\n"
"goog.object.extend(proto, $alias$);\n",
"alias", ModuleAlias(name), "file",
GetRootPath(file->name(), name) + GetJSFilename(options, name));
GetRootPath(file->name(), name, options) + GetJSFilename(options, name));
}
}

Expand Down

0 comments on commit a6ed2cf

Please sign in to comment.