Open whtswrng opened 1 month ago
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.
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
The JS output of the above proto file (via commonjs or commonjs_strict - both outputs are the same)
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.
Direct access to declared variable
nested_pkg_command_pb.Command
in the “getCommandsList”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 variablenested_pkg_command_pb
exports only the nested objectnested_pkg_command_pb.nested.pkg.Command
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.