protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
330 stars 66 forks source link

Why package are ignored for CommonJS importation #188

Closed benallamar closed 4 months ago

benallamar commented 5 months ago

Ola Guys!

I've this line :

Packages and CommonJS Imports

If you are using CommonJS-style imports, any package declarations in your .proto files are ignored by the compiler.

in the documentation, and I was wondering what is the reason behind this decision ? I'm going with commonjs_strict type of file but my code is not working correctly (unable to find an object from another file) due to absence of package namespace.

dibenede commented 5 months ago

Hi,

I'm not particularly familiar with commonjs strict mode's requirements and not understanding the issue at play. I've tried putting together a repro scenario like this:

project/
  index.js
  protos/
    some/
      a.proto
      a_pb.js
    other/
      b.proto
      b_pb.js

My message in a.proto depends on b.proto:

// a.proto
syntax = "proto2";

import "protos/other/b.proto";

package some;

message A {
  optional string a1 = 1;
  optional other.B b = 2;
}

// b.proto
syntax = "proto2";

package other;

message B {
  optional string b1 = 1;
}

I generate my code with: protoc --plugin=node_modules/protoc-gen-js/bin/protoc-gen-js --js_out=import_style=commonjs_strict,binary:. protos/some/a.proto protos/other/b.proto

Now, digging into the produced gencode of a_pb.js, we see a couple of things:

(notably "use strict" is missing at the top. I'm assuming you are a manually editing it into the files, so I'll add it in myself)

.... 
var jspb = require('google-protobuf');
var goog = jspb;
var proto = {};

var protos_other_b_pb = require('../../protos/other/b_pb.js');
goog.object.extend(proto, protos_other_b_pb);
goog.exportSymbol('some.A', null, proto);

...

goog.object.extend(exports, proto);

So, we are importing the B message, stashing it into our proto Object, and then exporting all of that.

My index.js file looks like:

'use strict';

const apb = require('./protos/some/a_pb.js');
const bpb = require('./protos/other/b_pb.js');

const a = new apb.some.A();
a.setA1('hello');

const b = new bpb.other.B();
b.setB1('world!');

a.setB(b);

console.log(`a: ${a.getA1()}, b: ${a.getB().getB1()}`);

This runs ok and has the expected output. Can tell me a bit more about the scenario details where you're having a problem?

In your proposed change (#189), I think you are just changing the trailing goog.object.extend(exports, proto); in the gencode from proto to a package name. However, we aren't stashing anything in a variable named after the package name.