Auto-generated JS code manipulates module state of "protobufjs/minimal"

[webmaster128]

protobuf.js version: 6.10.0

We use the following command to autogenerate a JS codec from .protos:

yarn pbjs \
  -t static-module \
  --es6 \
  -w commonjs \
  -o "$GENERATED_DIR/codecimpl.js" \
  --sparse \
  --no-beautify --no-delimited --no-verify --no-convert \
  "$COSMOS_PROTO_DIR/cosmos.proto" [...] "$GOOGLE_PROTO_DIR/protobuf/any.proto"

Now we have two of those codes in different packages. Let's call them codecimpl_full.js and codecimpl_minimal.js. Both use the same protobuf namespaces but have different symbols available.

As long as I only use one of them, everything is fine. But when I use both in the following way, the trouble beging:

const full = require("./codecimpl_full");
// here `full` works as expected 
const minimal = require("./codecimpl_minimal");
// here `full` is missing elements

The reason is that the auto-generated codec manipulate the object require("protobufjs/minimal").roots["default"], which is the same object for both imports as require("protobufjs/minimal") is created only once per Node.js process. This happens here (google/tendermint/cosmos are my namespaces):

"use strict";
var $protobuf = require("protobufjs/minimal");
const $Reader = $protobuf.Reader,
  $Writer = $protobuf.Writer,
  $util = $protobuf.util;
const $root = $protobuf.roots["default"] || ($protobuf.roots["default"] = {});
export const cosmos = ($root.cosmos = (() => {
  // ...

As you can see in the last line, $root which is require("protobufjs/minimal").roots["default"] is manipulated. This leads to one codec overriding the other one.

The easiest fix I see is:

diff --git a/packages/stargate/src/generated/codecimpl.js b/packages/stargate/src/generated/codecimpl.js
index 0b4f87cb..1ae1a36b 100644
--- a/packages/stargate/src/generated/codecimpl.js
+++ b/packages/stargate/src/generated/codecimpl.js
@@ -3,7 +3,7 @@ var $protobuf = require("protobufjs/minimal");
 const $Reader = $protobuf.Reader,
   $Writer = $protobuf.Writer,
   $util = $protobuf.util;
-const $root = $protobuf.roots["default"] || ($protobuf.roots["default"] = {});
+const $root = {};
 export const cosmos = ($root.cosmos = (() => {
   const cosmos = {};
   cosmos.Coin = (function () {

However, I wonder if state manipulation of protobufjs/minimal this is a feature I don't understand.

[thw0rted]

This doesn't look like a problem? The line you changed is equivalent to

let $root = $protobuf.roots["default"];
if (!$root) {
  $root  = {};
  $protobuf.roots["default"] = $root;
}

roots is a singleton that tracks all roots globally. This line provides a default value for the "default" named root, basically. If it runs twice, the first one should create the root, then the second one should use the root object created by the first pass.

I would wager that the problem here is that your namespace ("cosmos") has the same name in both places. If you're going to build two JS packages, they should use distinct namespaces.

[webmaster128]

Yes this is a problem because:

Now we have two of those codes in different packages. Let's call them codecimpl_full.js and codecimpl_minimal.js. Both use the same protobuf namespaces but have different symbols available.

And I disagree with

If you're going to build two JS packages, they should use distinct namespaces.

. The namespace is defined by the proto files and I cannot chose it. In one JS file I want a different subset of the namespace than in another JS file.

roots is a singleton that tracks all roots globally.

Exactly and this is a problem in the given use case.

[thw0rted]

So, you want a way to opt out of registering your module in the singleton so that you can have multiple definitions of the same namespace. How would the library know what to return when you call lookupType? It sounds like you're asking to use the project in an unsupported way.

[webmaster128]

This is a very good question. I never used lookupType, so I am not super familar with the singleton. In this case it seems to be a feature I did not understand.

Looking at the example https://github.com/protobufjs/protobuf.js/blob/v6.10.2/README.md#using-reflection-only it seems to be possible to create and maintain explicit root instances that do not rely on module-level state.

So my case might be unsupported right now, okay. But from an engineering perspective, isn't it an antipattern to rely in module-level state for singletons, which relies on module caching? Do we get guarantees that var $protobuf1 = require("protobufjs/minimal"); var $protobuf2 = require("protobufjs/minimal"); are always the same object? Does this work with ES6 modules in different environments?

[thw0rted]

I agree, I couldn't figure out how new Root() is actually supposed to work -- can I have more than one Root, and if so, how does the module know which root I meant to work with? I think there's a problem with the model. (Maybe that merits opening a different issue?)

[webmaster128]

think there's a problem with the model. (Maybe that merits opening a different issue?)

Personally I do not have a need for fix of this anmore as we migrated the codec generation to ts-proto for various reasons. I'm also fine if this gets closed as out of scope.

[bisol]

We are facing a similar issue: I have a lib providing a service compiled with pbjs, using the namespace "company.serviceA", and a project using this lib that uses pbjs to compile a proto file with the namespace "company.serviceB". The only way we got this to work was for the current project to compile it' proto files together with the lib' proto files.

[VagnerNico]

I've opened a pull request that should fix the problem.

[oliverrahner]

I have the same problem. My use case: A product I'm working on chose to create two distinct sets of proto files with partials of the same namespace in the past. To keep the same structure, I now want to turn each of these sets into a different node library which leads to the exact problem described here when importing both.