Too strict type checking during serialization

[rideg]

I started migrating my project to protobuf-es from protobuf-javascript.
I received an error message while calling Message.toBinary()

cannot unwrap field value, package.Message does not define a field wrapper

It happens because here. It uses an instanceof check to ensure that the object in the property is really an instance of the filed descriptor type.

However my situation is the following:

I have the proto definitions in a moduleA
I have moduleB that is compiled independently
I have moduleC that depends on moduleB and it receives proto objects from it the wraps them into a proto object instanciated locally, then tries to send them on the wire.

The code is something like this:

import {Wrapper} from "moduleA/wrapper_pb";
import {Service} from "moduleB/server";

function wrap(s: Service): Uint8Array {
   const wrap = new Wrapper();
   const msg = s.getMsg(); // this returns {Msg} from "moduleA/msg_pb"
   wrap.msg = msg;
  return wrap.toBinary(); // this is where I get the error
}

The reason for the error is that Msg the msg class has two definitions: one in moduleB and one in moduleC and however they are equivalent the instanceof operator will return false since the prototypes are different.

It probably should be enough to check if the object's type's name is equal to the field descriptor's type name or something.
This use case works fine with the protobuf-javascript.

Edit: replaced protobuf-ts with protobuf-javascript

[timostamm]

Thanks for the report, Sandor! To be fair, protobuf-ts doesn't have this feature 😄

To better understand how you got into this situation: I assume you generated code for google/protobuf/wrappers.proto?

[rideg]

Hey Timo!
Thanks for the quick reply!
Yes you're right. I did not use protobuf-ts, but protobuf-javascript (--js_out + -grpc-web_out params), I was confused. I'll update the issue.

So no I did not generate code for google/protobuf/wrappers.proto. All protos belong to my own project.
They look something like this:

syntax = "proto3";

package mypackage;

message Msg {
   string field = 1;
}

message Wrapper {
   string some_data = 1;
   Msg msg = 2;
}

The way it works in my project is that I create a js module (moduleA) for the protos. Then I have another js module (moduleB) that uses the proto module. Besides these I have another js module (moduleC) that depends on the previous two.
So the problem is that since moduleA gets compiled/minified twice the classes for the same proto message will be different. So Msg coming from moduleB is not the same as Msg coming from moduleC, so the instaceof check does not work here even though they are identical.

I have a workaround that makes it work:

import {Wrapper} from "moduleA/wrapper_pb";
import {Service} from "moduleB/server";

function wrap(s: Service): Uint8Array {
   const wrap = new Wrapper();
   const msg = s.getMsg(); // this returns {Msg} from "moduleA/msg_pb"
   
  // This way I update the fields type so that the instanceof returns true 
  // and the serializer doesn't fail anymore and produces the appropriate
  // binary.
  (Wrapper.fields.findJsonName("msg") as any)["T"] = msg.getType();

   wrap.msg = msg;
  return wrap.toBinary(); // this is where I get the error
}

Hope this explains.

[timostamm]

Thanks for the details, Sandor. I think it's advisable to avoid a situation with duplicate generated messages, but I agree that the code path should be more tolerant.