stephenh / ts-proto

An idiomatic protobuf generator for TypeScript
Apache License 2.0
2.17k stars 349 forks source link

`nestJs=true` and `useDate=true` interferes with `@google-cloud/firestore` #1074

Open brian-petersen opened 4 months ago

brian-petersen commented 4 months ago

When using those options to generate code, the timestamp wrapper code is added to the protobuf.wrappers array.

When trying to create or save a document with @google-cloud/firestore (which uses grpc/protobufjs under the hood), I get the following error: Request message serialization failure: value.getTime is not a function.

In digging into the @google-cloud/firestore code, it looks like it does it's own translation of Date to a JavaScript object. Then when protobufjs is later called, it sees that the target proto value is google.proto.Timestamp and will call the code that is generated by this lib:

protobufjs_1.wrappers[".google.protobuf.Timestamp"] = {
    fromObject(value) {
        return { seconds: value.getTime() / 1000, nanos: (value.getTime() % 1000) * 1e6 };
    },
    toObject(message) {
        return new Date(message.seconds * 1000 + message.nanos / 1e6);
    },
};

However, by this point value is no longer a Date object.

I find that this library overriding protobufjs's default wrappers to be quite intrusive. It pollutes a global object that may affect how other libs work.

The only workaround I can think of is to check for the type/shape of value and message to ensure that the transformation should actually happen.

Any tips or tricks would be welcomed.

stephenh commented 4 months ago

I find that this library overriding protobufjs's default wrappers to be quite intrusive.

Then don't use useDate=true. :-)

Well, or specifically useDate=true and NestJS together, because ts-proto, used directly/by itself, without NestJS, does not need the wrappers override--our encode / decode methods just do the right thing.

It's only because NestJS must* go through grpc/loader (not ts-proto's encode/decode methods directly), that the wrappers hack is the only way we know, so far, of injecting ts-proto-specific serde logic into NestJS.

(*I say "must" because I'm not actually a NestJS user, and our approach has been organically discovered by misc NestJS/ts-proto users over the last few years, it's a "must" in theory--perhaps that is a better way.)

If you know of a better way, I agree it would be great to use that instead. I'd love to accept a PR that had a different approach than the wrappers override.