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

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

Open
brian-petersen opened this issue Jul 12, 2024 · 1 comment
Open

Comments

@brian-petersen
Copy link

brian-petersen commented Jul 12, 2024

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.

@brian-petersen brian-petersen changed the title nestJs=true and useDate=true interferes with @google-cloud/firestore nestJs=true and useDate=true interferes with @google-cloud/firestore Jul 12, 2024
@stephenh
Copy link
Owner

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.

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