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

[whtswrng]

There are multiple issues with commonjs and commonjs_strict exports.

import_style="commonjs"

Polutes the global "proto" which introduces many unexpected bugs during runtime (this is our current problem we've been dealing with for a while).

import_style="commonjs_strict"

Fixed the "proto" polution, BUT introduced breaking change by exporting the nested object "package.nested.name.SpecificMessage" instead of "SpecificMessage". Additionally introduces a serious bug

---

Assume a proto file

import "common/v1/command.proto";

message SomeRequest {
  repeated nested.pkg.Command commands = 2; 
  nested.pkg.Command command = 3; 
}

The JS output of the above proto file (via commonjs or commonjs_strict - both outputs are the same)

proto.some.pkg.CommandRequest.prototype.getCommandsList = function() {
  return /** @type{!Array<!proto.nested.pkg.v1.Command>} */ (
    jspb.Message.getRepeatedWrapperField(this, nested_pkg_command_pb.Command, 2));
};

proto.some.pkg.CommandRequest.prototype.addCommands = function(opt_value, opt_index) {
  return jspb.Message.addToRepeatedWrapperField(this, 2, opt_value, proto.nested.pkg.v1.Command, opt_index);
};

If we opt in for “commonjs” or “commonjs_strict” import style, the final js output consist of multiple approaches of how the external files are imported.

1. Direct access to declared variable nested_pkg_command_pb.Command in the “getCommandsList”

2. Access via “proto” variable proto.nested.pkg.v1.Command in the “addCommands”.

Why import style “commonjs” works

Cause it pollutes the global proto variable while declaring the fields on it → proto variable have the nested fields “nested.pkg.v1”

It exports the final object without nesting → goog.object.extend(exports, proto.nested.pkg.v1);

Then even though the final output uses multiple types of imports it works as the proto variable is polluted with the nested fields (proto.nested.pkg.v1.Command works) and the declared variable of the import exports the object without nesting (nested_pkg_command_pb.Command works)

Why import style “commonjs_strict” does not work

It supports only “nested approach”. So when JS runtime access the nested_pkg_command_pb.Command it fails as the variable nested_pkg_command_pb exports only the nested object nested_pkg_command_pb.nested.pkg.Command

var nested_pkg_command_pb = require('../protos/command_pb.js');  // exports with nesting "foo.bar.baz.Command"
goog.object.extend(proto, nested_pkg_command_pb);


proto.some.pkg.CommandRequest.prototype.getCommandsList = function() {
  return /** @type{!Array<!proto.nested.pkg.v1.Command>} */ (
    jspb.Message.getRepeatedWrapperField(this, nested_pkg_command_pb.Command, 2)); // runtime ERROR as there is no "Command" defined
};

proto.some.pkg.CommandRequest.prototype.addCommands = function(opt_value, opt_index) {
  return jspb.Message.addToRepeatedWrapperField(this, 2, opt_value, proto.nested.pkg.v1.Command, opt_index);
};

FAQ

Why another new import style for the same thing over and over?

I don't like it either, but if I modify the "commonjs_strict" I introduce breaking change for this option. On the other hand, both types of imports are kind of broken right now anyway...

Why did you introduce another separate test suite with new test framework?

I spent so many hours trying to make the old one work (via gulp) without any success. I don't really understand why it's even that complicated, it felt like trying to debug & run code of some kind of nuclear power plant.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[garetman]

There are multiple issues with commonjs and commonjs_strict exports.

import_style="commonjs"

Polutes the global "proto" which introduces many unexpected bugs during runtime (this is our current problem we've been dealing with for a while).

import_style="commonjs_strict"

Fixed the "proto" polution, BUT introduced breaking change by exporting the nested object "package.nested.name.SpecificMessage" instead of "SpecificMessage". Additionally introduces a serious bug

Assume a proto file

import "common/v1/command.proto";

message SomeRequest {
  repeated nested.pkg.Command commands = 2; 
  nested.pkg.Command command = 3; 
}

The JS output of the above proto file (via commonjs or commonjs_strict - both outputs are the same)

proto.some.pkg.CommandRequest.prototype.getCommandsList = function() {
  return /** @type{!Array<!proto.nested.pkg.v1.Command>} */ (
    jspb.Message.getRepeatedWrapperField(this, nested_pkg_command_pb.Command, 2));
};

proto.some.pkg.CommandRequest.prototype.addCommands = function(opt_value, opt_index) {
  return jspb.Message.addToRepeatedWrapperField(this, 2, opt_value, proto.nested.pkg.v1.Command, opt_index);
};

If we opt in for “commonjs” or “commonjs_strict” import style, the final js output consist of multiple approaches of how the external files are imported.

1. Direct access to declared variable nested_pkg_command_pb.Command in the “getCommandsList”
2. Access via “proto” variable proto.nested.pkg.v1.Command in the “addCommands”.

Why import style “commonjs” works

Cause it pollutes the global proto variable while declaring the fields on it → proto variable have the nested fields “nested.pkg.v1”

It exports the final object without nesting → goog.object.extend(exports, proto.nested.pkg.v1);

Then even though the final output uses multiple types of imports it works as the proto variable is polluted with the nested fields (proto.nested.pkg.v1.Command works) and the declared variable of the import exports the object without nesting (nested_pkg_command_pb.Command works)

Why import style “commonjs_strict” does not work

It supports only “nested approach”. So when JS runtime access the nested_pkg_command_pb.Command it fails as the variable nested_pkg_command_pb exports only the nested object nested_pkg_command_pb.nested.pkg.Command

var nested_pkg_command_pb = require('../protos/command_pb.js');  // exports with nesting "foo.bar.baz.Command"
goog.object.extend(proto, nested_pkg_command_pb);


proto.some.pkg.CommandRequest.prototype.getCommandsList = function() {
  return /** @type{!Array<!proto.nested.pkg.v1.Command>} */ (
    jspb.Message.getRepeatedWrapperField(this, nested_pkg_command_pb.Command, 2)); // runtime ERROR as there is no "Command" defined
};

proto.some.pkg.CommandRequest.prototype.addCommands = function(opt_value, opt_index) {
  return jspb.Message.addToRepeatedWrapperField(this, 2, opt_value, proto.nested.pkg.v1.Command, opt_index);
};

FAQ

Why another new import style for the same thing over and over?

I don't like it either, but if I modify the "commonjs_strict" I introduce breaking change for this option. On the other hand, both types of imports are kind of broken right now anyway...

Why did you introduce another separate test suite with new test framework?

I spent so many hours trying to make the old one work (via gulp) without any success. I don't really understand why it's even that complicated, it felt like trying to debug & run code of some kind of nuclear power plant.

[!NOTE]