Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why package are ignored for CommonJS importation #188

Closed
benallamar opened this issue Feb 5, 2024 · 1 comment
Closed

Why package are ignored for CommonJS importation #188

benallamar opened this issue Feb 5, 2024 · 1 comment

Comments

@benallamar
Copy link

benallamar commented Feb 5, 2024

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 dibenede reopened this Feb 8, 2024
@dibenede
Copy link
Contributor

dibenede commented Feb 9, 2024

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.

@dibenede dibenede closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants