feat: added nestJsTimestampTypeWrapper

[PhilipMantrov]

related issue #69

[stephenh]

@PhilipMantrov nice! Would be amazing to get this fixed.

I think as-is, the added code isn't going to actually get output? Or will it? Usually code within that makeUtils function is used for things we want conditionally generated, so I just assumed that would be the case here.

Can we nudge one of the existing integration-test examples around to show this code in it's output, and maybe even have a unit test that shows it works? Fwiw I'm fine w/o the unit test if it means just getting this landed, given a lot of users have been asking for it.

Thanks!

[PhilipMantrov]

Added integration test and change const wrappers = imp('wrappers@protobufjs/minimal'); to const wrappers = imp('wrapper@protobufjs'); because protobuf.js has custom wrapper only for Any (wrappers.js, and issue), and if we dont have Any in our proto, wrappers will be undefined

[PhilipMantrov]

And I commit changes in genfiles, if it isn't needed just revert this commit.

[PhilipMantrov]

@stephenh can we merge this pr asap? Or just tell me what we need this for accepting this pr, thx. :)

[stephenh]

@PhilipMantrov thanks for adding the test! I'm good to merge this once the build is passing.

[PhilipMantrov]

@PhilipMantrov thanks for adding the test! I'm good to merge this once the build is passing.

@stephenh you need to disable 'Diff check' for once because protobufjs was updated.

[n0isy]

Any updates?

[PhilipMantrov]

Something strange with tests, working well in local env, but failed in CI. @stephenh can you take a look?

[gusinacio]

I tried to run the assignment of ".google.protobuf.Timestamp" to wrappers with protobufjs 7.0.0 and it isn't working, I had to downgrade to 6.11.3.

It's worth to check if it's running in version 7 and what has changed.

[PhilipMantrov]

@flametuner yea, its not working on protobufjs: 7.0.0 and above. ts-proto was pinned protobufjs to 6.8.8 for some reason, and for protobufjs 7.0.0 need some work in ts-proto.

Just use for now

Add in main.ts:

// ToDo: Remove when https://github.com/stephenh/ts-proto/pull/567 has merged
registerWrapperTypes();

function:

import { wrappers } from 'protobufjs';

export function registerWrapperTypes() {
  wrappers['.google.protobuf.Timestamp'] = {
    fromObject(value) {
      return {
        seconds: value.getTime() / 1000,
        nanos: (value.getTime() % 1000) * 1e6,
      };
    },
    toObject(message: { seconds: number; nanos: number }) {
      return new Date(message.seconds * 1000 + message.nanos / 1e6);
    },
  } as any;
}

[TakeOver]

Whatsup guys when this will be merged? I need this feature ASAP

[TakeOver]

Is this repo alive? Seems like dead perhaps I should fork it

[stephenh]

@TakeOver happy to have you fork the repo; if you want to contribute here I'm happy to merge PRs as well.

For this one, it took a few iterations and I didn't realize it'd been rewritten quite a bit and the build was green. Thanks @PhilipMantrov !

Fwiw technically it is green-ish b/c as @PhilipMantrov mentioned the tests were failing in CI so they're currently being skipped:

  xit('should findOneHero', async () => {
    const hero = await heroService.findOneHero({ id: 1 }).toPromise();
    expect(hero).toEqual({ id: 1, name: 'Stephenh', birthDate: new Date("2000/01/01") });
  });

I didn't have time to debug those, so just fyi if you try using this feature, and it doesn't work, it would be interesting see if the failures are for the same issue as the xit-d tests. Or maybe it really is CI specific.

[stephenh]

:tada: This PR is included in version 1.128.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v1.128.0

Your semantic-release bot :package::rocket:

[TannerPerrien]

@TakeOver and @stephenh - Thanks for this contribution!

When generating output with --ts_proto_opt=useDate=date, I see the resulting protobufjs wrappers/fromObject/toObject. However, my nestjs application does not end up invoking this wrapper and my resulting Timestamp output is

{
  "seconds": "0",
  "nanos": 0
}

I have carefully reviewed the nestjs-simple-usedate integration test.

UPDATE: Leaving the above comment for reference to others.

I re-read this thread and saw the comment about this not working in protobufjs: 7.0.0. It turns out @grpc/proto-loader 0.7.1 and above uses protobufjs: 7.x.x. I downgraded @grpc/proto-loader to 0.6.13 and everything is now working.

[stephenh]

Ah nice! Thanks for both the self-diagnosis and also reporting back the "pin to @grpc/proto-loader 0.6.13" fix @TannerPerrien !