[IMPORTANT] Fix bugs introduced by "commonjs" exports (introduce flat strict export) #205
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.