[IMPORTANT] Fix bugs introduced by "commonjs" exports (introduce flat strict export)

.gitignore

@@ -7,3 +7,4 @@ bazel-*
 /testproto_libs1.js
 /testproto_libs2.js
 /google-protobuf.js
+test-jest/*_pb.js

generator/js_generator.cc

@@ -228,7 +228,8 @@ std::string MaybeCrossFileRef(const GeneratorOptions& options,
                               const FileDescriptor* from_file,
                               const Descriptor* to_message) {
   if ((options.import_style == GeneratorOptions::kImportCommonJs ||
-       options.import_style == GeneratorOptions::kImportCommonJsStrict) &&
+       options.import_style == GeneratorOptions::kImportCommonJsStrict ||
+       options.import_style == GeneratorOptions::kImportCommonJsFlatStrict) &&
       from_file != to_message->file()) {
     // Cross-file ref in CommonJS needs to use the module alias instead of
     // the global name.
@@ -1737,7 +1738,7 @@ void Generator::GenerateProvides(const GeneratorOptions& options,
       //   foo.bar.Baz = function() { /* ... */ }
 
       // Do not use global scope in strict mode
-      if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
+      if (options.import_style == GeneratorOptions::kImportCommonJsStrict || options.import_style == GeneratorOptions::kImportCommonJsFlatStrict) {
         std::string namespaceObject = *it;
         // Remove "proto." from the namespace object
         ABSL_CHECK_EQ(0, namespaceObject.compare(0, 6, "proto."));
@@ -2315,7 +2316,7 @@ void Generator::GenerateClassFieldToObject(const GeneratorOptions& options,
     std::string value_to_object;
     if (value_field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
       value_to_object =
-          GetMessagePath(options, value_field->message_type()) + ".toObject";
+          SubmessageTypeRef(options, value_field) + ".toObject";
     } else {
       value_to_object = "undefined";
     }
@@ -2450,7 +2451,7 @@ void Generator::GenerateClassFieldFromObject(
           "$fieldclass$.fromObject));\n",
           "name", JSObjectFieldName(options, field), "index",
           JSFieldIndex(field), "fieldclass",
-          GetMessagePath(options, value_field->message_type()));
+          SubmessageTypeRef(options, value_field));
     } else {
       // `msg` is a newly-constructed message object that has not yet built any
       // map containers wrapping underlying arrays, so we can simply directly
@@ -2584,7 +2585,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
       printer->Print(
           ",\n"
           "      $messageType$",
-          "messageType", GetMessagePath(options, value_field->message_type()));
+          "messageType", SubmessageTypeRef(options, value_field));
     } else {
       printer->Print(
           ",\n"
@@ -2961,7 +2962,7 @@ void Generator::GenerateRepeatedMessageHelperMethods(
       "\n",
       "index", JSFieldIndex(field), "oneofgroup",
       (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""), "ctor",
-      GetMessagePath(options, field->message_type()));
+      SubmessageTypeRef(options, field));
 }
 
 void Generator::GenerateClassExtensionFieldInfo(const GeneratorOptions& options,
@@ -3102,14 +3103,14 @@ void Generator::GenerateClassDeserializeBinaryField(
     if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
       printer->Print(", $messageType$.deserializeBinaryFromReader",
                      "messageType",
-                     GetMessagePath(options, value_field->message_type()));
+                     SubmessageTypeRef(options, value_field));
     } else {
       printer->Print(", null");
     }
     printer->Print(", $defaultKey$", "defaultKey", JSFieldDefault(key_field));
     if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
       printer->Print(", new $messageType$()", "messageType",
-                     GetMessagePath(options, value_field->message_type()));
+                     SubmessageTypeRef(options, value_field));
     } else {
       printer->Print(", $defaultValue$", "defaultValue",
                      JSFieldDefault(value_field));
@@ -3302,7 +3303,7 @@ void Generator::GenerateClassSerializeBinaryField(
 
     if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
       printer->Print(", $messageType$.serializeBinaryToWriter", "messageType",
-                     GetMessagePath(options, value_field->message_type()));
+                     SubmessageTypeRef(options, value_field));
     }
 
     printer->Print(");\n");
@@ -3482,6 +3483,8 @@ bool GeneratorOptions::ParseFromOptions(
         import_style = kImportCommonJs;
       } else if (option.second == "commonjs_strict") {
         import_style = kImportCommonJsStrict;
+      } else if (option.second == "commonjs_flat_strict") {
+        import_style = kImportCommonJsFlatStrict;
       } else if (option.second == "browser") {
         import_style = kImportBrowser;
       } else if (option.second == "es6") {
@@ -3618,12 +3621,14 @@ void Generator::GenerateFile(const GeneratorOptions& options,
 
   // Generate "require" statements.
   if ((options.import_style == GeneratorOptions::kImportCommonJs ||
-       options.import_style == GeneratorOptions::kImportCommonJsStrict)) {
+       options.import_style == GeneratorOptions::kImportCommonJsStrict || 
+       options.import_style == GeneratorOptions::kImportCommonJsFlatStrict)
+      ) {
     printer->Print("var jspb = require('google-protobuf');\n");
     printer->Print("var goog = jspb;\n");
 
     // Do not use global scope in strict mode
-    if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
+    if (options.import_style == GeneratorOptions::kImportCommonJsStrict || options.import_style == GeneratorOptions::kImportCommonJsFlatStrict) {
       printer->Print("var proto = {};\n\n");
     } else {
       // To get the global object we call a function with .call(null), this will
@@ -3691,7 +3696,7 @@ void Generator::GenerateFile(const GeneratorOptions& options,
   }
 
   // if provided is empty, do not export anything
-  if (options.import_style == GeneratorOptions::kImportCommonJs &&
+  if ((options.import_style == GeneratorOptions::kImportCommonJs || options.import_style == GeneratorOptions::kImportCommonJsFlatStrict) &&
       !provided.empty()) {
     printer->Print("goog.object.extend(exports, $package$);\n", "package",
                    GetNamespace(options, file));

generator/js_generator.h

@@ -70,6 +70,7 @@ struct GeneratorOptions {
     kImportClosure,         // goog.require()
     kImportCommonJs,        // require()
     kImportCommonJsStrict,  // require() with no global export
+    kImportCommonJsFlatStrict,  // require() with no global export + flat export of the proto objects
     kImportBrowser,         // no import statements
     kImportEs6,             // import { member } from ''
   } import_style;

package.json

@@ -12,18 +12,20 @@
     "package.json",
     "README.md"
   ],
-  "dependencies": {},
   "devDependencies": {
     "glob": "~7.1.4",
     "google-closure-compiler": "~20190819.0.0",
     "google-closure-deps": "^20210406.0.0",
     "google-closure-library": "~20200315.0.0",
+    "google-protobuf": "^3.21.2",
     "gulp": "~4.0.2",
-    "jasmine": "~3.5.0"
+    "jasmine": "~3.5.0",
+    "jest": "29.7.0"
   },
   "scripts": {
     "build": "node ./node_modules/gulp/bin/gulp.js dist",
     "test": "node ./node_modules/gulp/bin/gulp.js test",
+    "test:jest": "jest ./test-jest --runInBand",
     "clean": "node ./node_modules/gulp/bin/gulp.js clean"
   },
   "repository": {

test-jest/README.md

@@ -0,0 +1,9 @@
+## Prerequisities
+
+- node version 20.0.0+
+
+## How to run
+
+You need to build the "protoc-gen-js" binary before running any test.
+1. Run `bazel build` in the root directory.
+2. Run `npm run test:jest`

test-jest/commonjs-strict/flat-strict.test.js

@@ -0,0 +1,34 @@
+const path = require("path");
+const util = require("util");
+const exec = util.promisify(require("node:child_process").exec);
+
+const protocJsPlugin = path.resolve(__dirname, "..", "..", "bazel-bin", "generator", "protoc-gen-js");
+const command = `protoc --plugin=protoc-gen-js=${protocJsPlugin} --js_out=import_style=commonjs_flat_strict,binary:.`;
+
+describe("When import_style is 'commonjs_flat_strict'", () => {
+  it("'proto' is exported directly without package nesting", async () => {
+    await exec(command + " ./protos/proto.proto ./protos/proto2.proto", { cwd: __dirname });
+    const r = require(path.resolve(__dirname, "./protos/proto_pb"));
+    expect(r.CommandRequest).toBeDefined();
+  });
+
+  it("global 'proto' is not polluted", async () => {
+    await exec(command + " ./protos/proto.proto ./protos/proto2.proto", { cwd: __dirname });
+    const r = require(path.resolve(__dirname, "./protos/proto_pb"));
+    expect(global.proto).toBe(undefined);
+  });
+
+  it("addCommands resolves correctly without an error", async () => {
+    await exec(command + " ./protos/proto.proto ./protos/proto2.proto", { cwd: __dirname });
+    const r = require(path.resolve(__dirname, "./protos/proto_pb"));
+    const c = new r.CommandRequest();
+    c.addCommands();
+  });
+
+  it("getCommandsList resolves correctly without an error", async () => {
+    await exec(command + " ./protos/proto.proto ./protos/proto2.proto", { cwd: __dirname });
+    const r = require(path.resolve(__dirname, "./protos/proto_pb"));
+    const c = new r.CommandRequest();
+    c.getCommandsList();
+  });
+});

test-jest/commonjs-strict/protos/proto.proto

@@ -0,0 +1,20 @@
+syntax = "proto3";
+
+package foo.bar.baz.v2;
+
+import "protos/proto2.proto";
+
+service FooService {
+  rpc Commands(CommandRequest) returns (CommandResponse) {}
+}
+
+message CommandRequest {
+  string session_id = 1;
+  repeated command.pkg.nesting.Command commands = 2; 
+  command.pkg.nesting.Command command = 3; 
+  map<string, command.pkg.nesting.Command> more_commands = 4;
+}
+
+message CommandResponse {
+  string result = 1;
+}

test-jest/commonjs-strict/protos/proto2.proto

@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+package command.pkg.nesting;
+
+message Command {
+  string id = 1;
+  string command = 2;
+}