Js TypeError when generated with commonjs_strict

[Hunrik]

Version: v3.10.0
Language: Javascript
OS: OSX 10.14.6
Node v12.10.0

When js files generated with commonjs_strict the referrences to types defined in other files are not using the package name to access it from the exported object, so TypeError is thrown due to reading property of undefined.

How to reproduce

a.proto

syntax = "proto3";

package foo.v1;
import "b.proto";

message A {
  repeated foo.v1.B foo = 1;
}

b.proto

syntax = "proto3";

package foo.v1;
import "b.proto";

message A {
  repeated foo.v1.B foo = 1;
}

protoc \
  --js_out=import_style=commonjs_strict:./generated \
  a.proto b.proto

In the generated js files the protos_b_pb.B should be protos_b_foo.v1.pb.B

[kconwayinvision]

The issue seems to be rooted in a change to how file exports are written when switching to strict mode. It happens to any import of a file, in or out of the same package, that was rendered using commonjs_strict. For example, the following protobuf definitions:

/foo/bar/v1/messages.proto

syntax = "proto3";
package foo.bar.v1;

message A {
  string value = 1;
}

/foo/baz/v1/messages.proto

syntax = "proto3";
package foo.baz.v1;

import "foo/bar/v1";

message B {
  foo.bar.v1.A value = 1;
}

result in files like:

/foo/bar/v1/messages_pb.proto

var jspb = require('google-protobuf');
var goog = jspb;
var proto = {};
// ...
proto.foo.bar.v1.A = function(opt_data) {
  jspb.Message.initialize(this, opt_data, 0, -1, null, null);
};
// ...
goog.object.extend(exports, proto);

/foo/baz/v1/messages_pb.proto

var jspb = require('google-protobuf');
var goog = jspb;
var proto = {};
// ...
var foo_bar_v1_messages_pb = require('../../../foo/bar/v1/messages_pb.js');
// ...
proto.foo.baz.v1.B = function(opt_data) {
  jspb.Message.initialize(this, opt_data, 0, -1, null, null);
};
// ...
proto.foo.baz.v1.B.deserializeBinaryFromReader = function(msg, reader) {
  // ...
    switch (field) {
    case 1:
      var value = new foo_bar_v1_messages_pb.A;
      reader.readMessage(value,foo_bar_v1_messages_pb.A.deserializeBinaryFromReader);
// ...
goog.object.extend(exports, proto);

The issues is that foo_bar_v1_messages_pb.A in the deserialization is undefined because foo_bar_v1_messages_pb exports a set of nested namespaces. The actual path to A is foo_bar_v1_messages_pb.foo.bar.v1.A.

This is only an issue when generating code using commonjs_strict because using commonjs results in a different export behavior. For example, the export for /foo/bar/v1/messages_pb.proto changes from goog.object.extend(exports, proto); when in strict mode to goog.object.extend(exports, proto.foo.bar.v1); when only in commonjs mode. All generation of imports and usage of imported code, even under commonjs_strict, assumes the exports are not nested namespaces.

[dibenede]

We need to verify if this is still an issue (likely so given the dates). Thank you for the detailed repro instructions and explanation.

[niloc132]

I can confirm that this is still an issue - we're attempting to move from commonjs to commonjs_strict, but due to this issue, one .proto file cannot reference another and yield working JS.

We're working around this with some custom webpack aliases and a per-.proto shim file to return the un-namespaced contents. That said, there's a bit more context omitted by the report above - the foo_bar_v1_messages_pb ends up being goog.object.extend()'d into proto itself, which implies that at that point, the nested packages are still expected.

Note that this issue may be the cause of #25 - distributing timestamp_pb.js as commonjs output may mean that commonjs_strict files cannot import it due to these differing conventions.